Bug 1296929 - Match spec for col/colgroup.span, textarea.rows/cols; r=jst

The spec recently changed to match browsers better.  There's currently
not much interop in exact details of how this work.  This brings us in
line with the spec except for the limit of 1000 on the span attribute.

The added textarea failures are spurious, because I'm not updating our
local tests in this commit.  The new tests are submitted upstream at
<https://github.com/w3c/web-platform-tests/pull/3518>.

MozReview-Commit-ID: 1L8aUtF47Qi

dom/base/nsAttrValue.cpp

@@ -1554,6 +1554,34 @@ nsAttrValue::ParseIntWithBounds(const nsAString& aString,
   return true;
 }
 
+void
+nsAttrValue::ParseIntWithFallback(const nsAString& aString, int32_t aDefault,
+                                  int32_t aMax)
+{
+  ResetIfSet();
+
+  nsContentUtils::ParseHTMLIntegerResultFlags result;
+  int32_t val = nsContentUtils::ParseHTMLInteger(aString, &result);
+  bool nonStrict = false;
+  if ((result & nsContentUtils::eParseHTMLInteger_Error) || val < 1) {
+    val = aDefault;
+    nonStrict = true;
+  }
+
+  if (val > aMax) {
+    val = aMax;
+    nonStrict = true;
+  }
+
+  if ((result & nsContentUtils::eParseHTMLInteger_IsPercent) ||
+      (result & nsContentUtils::eParseHTMLInteger_NonStandard) ||
+      (result & nsContentUtils::eParseHTMLInteger_DidNotConsumeAllInput)) {
+    nonStrict = true;
+  }
+
+  SetIntValueAndType(val, eInteger, nonStrict ? &aString : nullptr);
+}
+
 bool
 nsAttrValue::ParseNonNegativeIntValue(const nsAString& aString)
 {

dom/base/nsAttrValue.h

@@ -321,6 +321,18 @@ public:
   bool ParseIntWithBounds(const nsAString& aString, int32_t aMin,
                             int32_t aMax = INT32_MAX);
 
+  /**
+   * Parse a string value into an integer with a fallback for invalid values.
+   * Also allows clamping to a maximum value to support col/colgroup.span (this
+   * is not per spec right now).
+   *
+   * @param aString the string to parse
+   * @param aDefault the default value
+   * @param aMax the maximum value (if value is greater it will be clamped)
+   */
+  void ParseIntWithFallback(const nsAString& aString, int32_t aDefault,
+                            int32_t aMax = INT32_MAX);
+
   /**
    * Parse a string value into a non-negative integer.
    * This method follows the rules for parsing non-negative integer from:

dom/html/HTMLTableColElement.cpp

@@ -44,7 +44,8 @@ HTMLTableColElement::ParseAttribute(int32_t aNamespaceID,
     }
     if (aAttribute == nsGkAtoms::span) {
       /* protection from unrealistic large colspan values */
-      return aResult.ParseIntWithBounds(aValue, 1, MAX_COLSPAN);
+      aResult.ParseIntWithFallback(aValue, 1, MAX_COLSPAN);
+      return true;
     }
     if (aAttribute == nsGkAtoms::width) {
       return aResult.ParseSpecialIntValue(aValue);

dom/html/HTMLTableColElement.h

@@ -27,7 +27,8 @@ public:
   }
   void SetSpan(uint32_t aSpan, ErrorResult& aError)
   {
-    SetHTMLIntAttr(nsGkAtoms::span, aSpan, aError);
+    uint32_t span = aSpan ? aSpan : 1;
+    SetUnsignedIntAttr(nsGkAtoms::span, span, 1, aError);
   }
 
   void GetAlign(DOMString& aAlign)

dom/html/HTMLTextAreaElement.cpp

@@ -402,9 +402,12 @@ HTMLTextAreaElement::ParseAttribute(int32_t aNamespaceID,
     if (aAttribute == nsGkAtoms::maxlength ||
         aAttribute == nsGkAtoms::minlength) {
       return aResult.ParseNonNegativeIntValue(aValue);
-    } else if (aAttribute == nsGkAtoms::cols ||
-               aAttribute == nsGkAtoms::rows) {
-      return aResult.ParsePositiveIntValue(aValue);
+    } else if (aAttribute == nsGkAtoms::cols) {
+      aResult.ParseIntWithFallback(aValue, DEFAULT_COLS);
+      return true;
+    } else if (aAttribute == nsGkAtoms::rows) {
+      aResult.ParseIntWithFallback(aValue, DEFAULT_ROWS_TEXTAREA);
+      return true;
     }
   }
   return nsGenericHTMLElement::ParseAttribute(aNamespaceID, aAttribute, aValue,

dom/html/HTMLTextAreaElement.h

@@ -180,11 +180,8 @@ public:
   }
   void SetCols(uint32_t aCols, ErrorResult& aError)
   {
-    if (aCols == 0) {
-      aError.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
-    } else {
-      SetUnsignedIntAttr(nsGkAtoms::cols, aCols, DEFAULT_COLS, aError);
-    }
+    uint32_t cols = aCols ? aCols : DEFAULT_COLS;
+    SetUnsignedIntAttr(nsGkAtoms::cols, cols, DEFAULT_COLS, aError);
   }
   bool Disabled()
   {
@@ -262,11 +259,8 @@ public:
   }
   void SetRows(uint32_t aRows, ErrorResult& aError)
   {
-    if (aRows == 0) {
-      aError.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
-    } else {
-      SetUnsignedIntAttr(nsGkAtoms::rows, aRows, DEFAULT_ROWS_TEXTAREA, aError);
-    }
+    uint32_t rows = aRows ? aRows : DEFAULT_ROWS_TEXTAREA;
+    SetUnsignedIntAttr(nsGkAtoms::rows, rows, DEFAULT_ROWS_TEXTAREA, aError);
   }
   // XPCOM GetWrap is fine
   void SetWrap(const nsAString& aWrap, ErrorResult& aError)

dom/html/test/forms/test_textarea_attributes_reflection.html

@@ -25,6 +25,7 @@ reflectUnsignedInt({
   attribute: "cols",
   nonZero: true,
   defaultValue: 20,
+  fallback: true,
 });
 
 todo("dirName" in document.createElement("textarea"),
@@ -77,6 +78,7 @@ reflectUnsignedInt({
   attribute: "rows",
   nonZero: true,
   defaultValue: 2,
+  fallback: true,
 });
 
 // .wrap

dom/html/test/reflect.js

@@ -140,6 +140,7 @@ function reflectUnsignedInt(aParameters)
   var attr = aParameters.attribute;
   var nonZero = aParameters.nonZero;
   var defaultValue = aParameters.defaultValue;
+  var fallback = aParameters.fallback;
 
   if (defaultValue === undefined) {
     if (nonZero) {
@@ -149,6 +150,10 @@ function reflectUnsignedInt(aParameters)
     }
   }
 
+  if (fallback === undefined) {
+    fallback = false;
+  }
+
   ok(attr in element, attr + " should be an IDL attribute of this element");
   is(typeof element[attr], "number", attr + " IDL attribute should be a number");
 
@@ -213,7 +218,7 @@ function reflectUnsignedInt(aParameters)
     is(e.code, DOMException.INDEX_SIZE_ERR, "exception code should be INDEX_SIZE_ERR");
   }
 
-  if (nonZero) {
+  if (nonZero && !fallback) {
     ok(caught, "an exception should have been caught");
   } else {
     ok(!caught, "no exception should have been caught");

testing/web-platform/meta/html/dom/reflection-forms.html.ini

@@ -655,6 +655,12 @@
   [textarea.tabIndex: setAttribute() to object "3" followed by IDL get]
     expected: FAIL
 
+  [textarea.cols: IDL set to 0 must throw INDEX_SIZE_ERR]
+    expected: FAIL
+
+  [textarea.rows: IDL set to 0 must throw INDEX_SIZE_ERR]
+    expected: FAIL
+
   [textarea.dirName: typeof IDL attribute]
     expected: FAIL
 

testing/web-platform/meta/html/dom/reflection-tabular.html.ini

@@ -339,18 +339,6 @@
   [th.sorted: IDL set to object "test-valueOf" followed by IDL get]
     expected: FAIL
 
-  [colgroup.span: IDL set to 2147483648 followed by getAttribute()]
-    expected: FAIL
-
-  [colgroup.span: IDL set to 4294967295 followed by getAttribute()]
-    expected: FAIL
-
-  [col.span: IDL set to 2147483648 followed by getAttribute()]
-    expected: FAIL
-
-  [col.span: IDL set to 4294967295 followed by getAttribute()]
-    expected: FAIL
-
   [td.colSpan: IDL set to 2147483648 followed by getAttribute()]
     expected: FAIL