Bug 586786 - Fix legacy reflected attributes r=bz, mounir

2011-07-11 12:56:00 +02:00 · 2011-07-11 12:56:00 +02:00 · 0c00e974d0
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@ -2103,17 +2103,6 @@ nsGenericHTMLElement::SetAttrHelper(nsIAtom* aAttr, const nsAString& aValue)
  return SetAttr(kNameSpaceID_None, aAttr, aValue, PR_TRUE);
 }

-nsresult
-nsGenericHTMLElement::GetStringAttrWithDefault(nsIAtom* aAttr,
-                                               const char* aDefault,
-                                               nsAString& aResult)
-{
-  if (!GetAttr(kNameSpaceID_None, aAttr, aResult)) {
-    CopyASCIItoUTF16(aDefault, aResult);
-  }
-  return NS_OK;
-}
-
 nsresult
 nsGenericHTMLElement::SetBoolAttr(nsIAtom* aAttr, PRBool aValue)
 {
--- a/content/html/content/src/nsGenericHTMLElement.h
+++ b/content/html/content/src/nsGenericHTMLElement.h
@ -609,19 +609,6 @@ protected:
   */
  NS_HIDDEN_(nsresult) SetAttrHelper(nsIAtom* aAttr, const nsAString& aValue);

-  /**
-   * Helper method for NS_IMPL_STRING_ATTR_DEFAULT_VALUE macro.
-   * Gets the value of an attribute, returns specified default value if the
-   * attribute isn't set. Only works for attributes in null namespace.
-   *
-   * @param aAttr    name of attribute.
-   * @param aDefault default-value to return if attribute isn't set.
-   * @param aResult  result value [out]
-   */
-  NS_HIDDEN_(nsresult) GetStringAttrWithDefault(nsIAtom* aAttr,
-                                                const char* aDefault,
-                                                nsAString& aResult);
-
  /**
   * Helper method for NS_IMPL_BOOL_ATTR macro.
   * Gets value of boolean attribute. Only works for attributes in null
@ -1084,23 +1071,6 @@ protected:
    return SetAttrHelper(nsGkAtoms::_atom, aValue);                  \
  }

-/**
- * A macro to implement the getter and setter for a given string
- * valued content property with a default value.
- * The method uses the generic GetAttr and SetAttr methods.
- */
-#define NS_IMPL_STRING_ATTR_DEFAULT_VALUE(_class, _method, _atom, _default) \
-  NS_IMETHODIMP                                                      \
-  _class::Get##_method(nsAString& aValue)                            \
-  {                                                                  \
-    return GetStringAttrWithDefault(nsGkAtoms::_atom, _default, aValue);\
-  }                                                                  \
-  NS_IMETHODIMP                                                      \
-  _class::Set##_method(const nsAString& aValue)                      \
-  {                                                                  \
-    return SetAttrHelper(nsGkAtoms::_atom, aValue);                \
-  }
-
 /**
 * A macro to implement the getter and setter for a given boolean
 * valued content property. The method uses the generic GetAttr and
--- a/content/html/content/src/nsHTMLTableCellElement.cpp
+++ b/content/html/content/src/nsHTMLTableCellElement.cpp
@ -222,7 +222,7 @@ nsHTMLTableCellElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Abbr, abbr)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Axis, axis)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, BgColor, bgcolor)
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableCellElement, Ch, _char, ".")
+NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Ch, _char)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, ChOff, charoff)
 NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLTableCellElement, ColSpan, colspan, 1)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Headers, headers)
@ -230,7 +230,7 @@ NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Height, height)
 NS_IMPL_BOOL_ATTR(nsHTMLTableCellElement, NoWrap, nowrap)
 NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLTableCellElement, RowSpan, rowspan, 1)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Scope, scope)
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableCellElement, VAlign, valign, "middle")
+NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, VAlign, valign)
 NS_IMPL_STRING_ATTR(nsHTMLTableCellElement, Width, width)


--- a/content/html/content/src/nsHTMLTableColElement.cpp
+++ b/content/html/content/src/nsHTMLTableColElement.cpp
@ -111,11 +111,11 @@ NS_HTML_CONTENT_INTERFACE_TABLE_TAIL_CLASSINFO(HTMLTableColElement)
 NS_IMPL_ELEMENT_CLONE(nsHTMLTableColElement)


-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableColElement, Align, align, "left")
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableColElement, Ch, _char, ".")
+NS_IMPL_STRING_ATTR(nsHTMLTableColElement, Align, align)
+NS_IMPL_STRING_ATTR(nsHTMLTableColElement, Ch, _char)
 NS_IMPL_STRING_ATTR(nsHTMLTableColElement, ChOff, charoff)
 NS_IMPL_INT_ATTR_DEFAULT_VALUE(nsHTMLTableColElement, Span, span, 1)
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableColElement, VAlign, valign, "middle")
+NS_IMPL_STRING_ATTR(nsHTMLTableColElement, VAlign, valign)
 NS_IMPL_STRING_ATTR(nsHTMLTableColElement, Width, width)


--- a/content/html/content/src/nsHTMLTableElement.cpp
+++ b/content/html/content/src/nsHTMLTableElement.cpp
@ -965,17 +965,10 @@ nsHTMLTableElement::ParseAttribute(PRInt32 aNamespaceID,
        aAttribute == nsGkAtoms::cellpadding) {
      return aResult.ParseSpecialIntValue(aValue);
    }
-    if (aAttribute == nsGkAtoms::cols) {
+    if (aAttribute == nsGkAtoms::cols ||
+        aAttribute == nsGkAtoms::border) {
      return aResult.ParseIntWithBounds(aValue, 0);
    }
-    if (aAttribute == nsGkAtoms::border) {
-      if (!aResult.ParseIntWithBounds(aValue, 0)) {
-        // XXX this should really be NavQuirks only to allow non numeric value
-        aResult.SetTo(1);
-      }
-
-      return PR_TRUE;
-    }
    if (aAttribute == nsGkAtoms::height) {
      return aResult.ParseSpecialIntValue(aValue);
    }
@ -983,14 +976,12 @@ nsHTMLTableElement::ParseAttribute(PRInt32 aNamespaceID,
      if (aResult.ParseSpecialIntValue(aValue)) {
        // treat 0 width as auto
        nsAttrValue::ValueType type = aResult.Type();
-        if ((type == nsAttrValue::eInteger &&
-             aResult.GetIntegerValue() == 0) ||
-            (type == nsAttrValue::ePercent &&
-             aResult.GetPercentValue() == 0.0f)) {
-          return PR_FALSE;
-        }
+        return !((type == nsAttrValue::eInteger &&
+                  aResult.GetIntegerValue() == 0) ||
+                 (type == nsAttrValue::ePercent &&
+                  aResult.GetPercentValue() == 0.0f));
      }
-      return PR_TRUE;
+      return PR_FALSE;
    }
    
    if (aAttribute == nsGkAtoms::align) {
--- a/content/html/content/src/nsHTMLTableRowElement.cpp
+++ b/content/html/content/src/nsHTMLTableRowElement.cpp
@ -359,11 +359,11 @@ nsHTMLTableRowElement::DeleteCell(PRInt32 aValue)
  return RemoveChild(cell, getter_AddRefs(retChild));
 }

-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableRowElement, Align, align, "left")
+NS_IMPL_STRING_ATTR(nsHTMLTableRowElement, Align, align)
 NS_IMPL_STRING_ATTR(nsHTMLTableRowElement, BgColor, bgcolor)
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableRowElement, Ch, _char, ".")
+NS_IMPL_STRING_ATTR(nsHTMLTableRowElement, Ch, _char)
 NS_IMPL_STRING_ATTR(nsHTMLTableRowElement, ChOff, charoff)
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableRowElement, VAlign, valign, "middle")
+NS_IMPL_STRING_ATTR(nsHTMLTableRowElement, VAlign, valign)


 PRBool
--- a/content/html/content/src/nsHTMLTableSectionElement.cpp
+++ b/content/html/content/src/nsHTMLTableSectionElement.cpp
@ -120,9 +120,9 @@ NS_HTML_CONTENT_INTERFACE_TABLE_TAIL_CLASSINFO(HTMLTableSectionElement)
 NS_IMPL_ELEMENT_CLONE(nsHTMLTableSectionElement)


-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableSectionElement, Align, align, "left")
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableSectionElement, VAlign, valign, "middle")
-NS_IMPL_STRING_ATTR_DEFAULT_VALUE(nsHTMLTableSectionElement, Ch, _char, ".")
+NS_IMPL_STRING_ATTR(nsHTMLTableSectionElement, Align, align)
+NS_IMPL_STRING_ATTR(nsHTMLTableSectionElement, VAlign, valign)
+NS_IMPL_STRING_ATTR(nsHTMLTableSectionElement, Ch, _char)
 NS_IMPL_STRING_ATTR(nsHTMLTableSectionElement, ChOff, charoff)


--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@ -273,6 +273,7 @@ _TEST_FILES = \
 		test_bug659596.html \
 		test_bug659743.xml \
 		test_bug660663.html \
+		test_bug586786.html \
 		test_restore_from_parser_fragment.html \
 		$(NULL)

--- a/content/html/content/test/test_bug586786.html
+++ b/content/html/content/test/test_bug586786.html
@ -0,0 +1,58 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=586786
+-->
+<head>
+  <title>Test for Bug 586786</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="reflect.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=586786">Mozilla Bug 586786</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 586786 **/
+
+var elements = ["col", "colgroup", "tbody", "tfoot", "thead", "tr", "td", "th"];
+
+for(var i = 0; i < elements.length; i++)
+{
+  reflectString({
+    element: document.createElement(elements[i]), 
+    attribute: "align",
+    otherValues: [ "left", "right", "center", "justify", "char" ]
+  });
+  
+  reflectString({
+    element: document.createElement(elements[i]), 
+    attribute: "vAlign",
+    otherValues: [ "top", "middle", "bottom", "baseline" ]
+  });
+  
+  reflectString({
+    element: document.createElement(elements[i]), 
+    attribute: {idl: "ch", content: "char"}
+  });
+}
+
+// table.border, table.width
+reflectString({
+  element: document.createElement("table"), 
+  attribute: "border"
+});
+
+reflectString({
+  element: document.createElement("table"), 
+  attribute: "width"
+});
+</script>
+</pre>
+</body>
+</html>