Issue #1384 - Match standard for colSpan/rowSpan

HTML standardizes proper behavior of colSpan and rowSpan:

The main thing is that getting the .rowSpan and .colSpan IDL properties will now return the actual clamped value that we use.

6 files changed, 59 insertions, 19 deletions

diff --git a/dom/base/nsAttrValue.cpp b/dom/base/nsAttrValue.cpp
index ebddcb7ed..2418fb501 100644
--- a/dom/base/nsAttrValue.cpp
+++ b/dom/base/nsAttrValue.cpp
@@ -1526,6 +1526,40 @@ nsAttrValue::ParseIntWithFallback(const nsAString& aString, int32_t aDefault,
   SetIntValueAndType(val, eInteger, nonStrict ? &aString : nullptr);
 }
 
+void
+nsAttrValue::ParseClampedNonNegativeInt(const nsAString& aString,
+                                        int32_t aDefault, int32_t aMin,
+                                        int32_t aMax)
+{
+  ResetIfSet();
+
+  nsContentUtils::ParseHTMLIntegerResultFlags result;
+  int32_t val = nsContentUtils::ParseHTMLInteger(aString, &result);
+  bool nonStrict = (result & nsContentUtils::eParseHTMLInteger_IsPercent) ||
+                   (result & nsContentUtils::eParseHTMLInteger_NonStandard) ||
+                   (result & nsContentUtils::eParseHTMLInteger_DidNotConsumeAllInput);
+
+  if (result & nsContentUtils::eParseHTMLInteger_ErrorOverflow) {
+    if (result & nsContentUtils::eParseHTMLInteger_Negative) {
+      val = aDefault;
+    } else {
+      val = aMax;
+    }
+    nonStrict = true;
+  } else if ((result & nsContentUtils::eParseHTMLInteger_Error) || val < 0) {
+    val = aDefault;
+    nonStrict = true;
+  } else if (val < aMin) {
+    val = aMin;
+    nonStrict = true;
+  } else if (val > aMax) {
+    val = aMax;
+    nonStrict = true;
+  }
+
+  SetIntValueAndType(val, eInteger, nonStrict ? &aString : nullptr);
+}
+
 bool
 nsAttrValue::ParseNonNegativeIntValue(const nsAString& aString)
 {
diff --git a/dom/base/nsAttrValue.h b/dom/base/nsAttrValue.h
index 655e4ca61..23f61a614 100644
--- a/dom/base/nsAttrValue.h
+++ b/dom/base/nsAttrValue.h
@@ -362,6 +362,19 @@ public:
    */
   bool ParseNonNegativeIntValue(const nsAString& aString);
 
+   /**
+   * Parse a string value into a clamped non-negative integer.
+   * This method follows the rules for parsing non-negative integer from:
+   * https://html.spec.whatwg.org/multipage/infrastructure.html#clamped-to-the-range
+   *
+   * @param aString the string to parse
+   * @param aDefault value to return for negative or invalid values
+   * @param aMin minimum value
+   * @param aMax maximum value
+   */
+  void ParseClampedNonNegativeInt(const nsAString& aString, int32_t aDefault,
+                                  int32_t aMin, int32_t aMax);
+
   /**
    * Parse a string value into a positive integer.
    * This method follows the rules for parsing non-negative integer from:
diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
index b6cbbbace..4f7b71b19 100644
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -1088,6 +1088,7 @@ nsContentUtils::ParseHTMLInteger(const nsAString& aValue,
   int sign = 1;
   if (*iter == char16_t('-')) {
     sign = -1;
+    result |= eParseHTMLInteger_Negative;
     ++iter;
   } else if (*iter == char16_t('+')) {
     result |= eParseHTMLInteger_NonStandard;
diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h
index bf6a59dcd..64f7485cb 100644
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -445,7 +445,9 @@ public:
     // Set if one or more error flags were set.
     eParseHTMLInteger_Error                 = 1 << 3,
     eParseHTMLInteger_ErrorNoValue          = 1 << 4,
-    eParseHTMLInteger_ErrorOverflow         = 1 << 5
+    eParseHTMLInteger_ErrorOverflow         = 1 << 5,
+    // Use this flag to detect the difference between overflow and underflow
+    eParseHTMLInteger_Negative              = 1 << 6,
   };
   static int32_t ParseHTMLInteger(const nsAString& aValue,
                                   ParseHTMLIntegerResultFlags *aResult);
diff --git a/dom/html/HTMLTableCellElement.cpp b/dom/html/HTMLTableCellElement.cpp
index d00d60400..1cf413413 100644
--- a/dom/html/HTMLTableCellElement.cpp
+++ b/dom/html/HTMLTableCellElement.cpp
@@ -390,26 +390,16 @@ HTMLTableCellElement::ParseAttribute(int32_t aNamespaceID,
       return aResult.ParseIntWithBounds(aValue, 0);
     }
     if (aAttribute == nsGkAtoms::colspan) {
-      bool res = aResult.ParseIntWithBounds(aValue, -1);
-      if (res) {
-        int32_t val = aResult.GetIntegerValue();
-        // reset large colspan values as IE and opera do
-        if (val > MAX_COLSPAN || val <= 0) {
-          aResult.SetTo(1, &aValue);
-        }
-      }
-      return res;
+      aResult.ParseClampedNonNegativeInt(aValue, 1, 1, MAX_COLSPAN);
+      return true;
     }
     if (aAttribute == nsGkAtoms::rowspan) {
-      bool res = aResult.ParseIntWithBounds(aValue, -1, MAX_ROWSPAN);
-      if (res) {
-        int32_t val = aResult.GetIntegerValue();
-        // quirks mode does not honor the special html 4 value of 0
-        if (val < 0 || (0 == val && InNavQuirksMode(OwnerDoc()))) {
+      aResult.ParseClampedNonNegativeInt(aValue, 1, 0, MAX_ROWSPAN);
+      // quirks mode does not honor the special html 4 value of 0
+      if (aResult.GetIntegerValue() == 0 && InNavQuirksMode(OwnerDoc())) {
           aResult.SetTo(1, &aValue);
-        }
       }
-      return res;
+      return true;
     }
     if (aAttribute == nsGkAtoms::height) {
       return aResult.ParseSpecialIntValue(aValue);
diff --git a/dom/html/HTMLTableCellElement.h b/dom/html/HTMLTableCellElement.h
index 916333510..ab7a918eb 100644
--- a/dom/html/HTMLTableCellElement.h
+++ b/dom/html/HTMLTableCellElement.h
@@ -37,7 +37,7 @@ public:
   }
   void SetColSpan(uint32_t aColSpan, ErrorResult& aError)
   {
-    SetHTMLIntAttr(nsGkAtoms::colspan, aColSpan, aError);
+    SetUnsignedIntAttr(nsGkAtoms::colspan, aColSpan, 1, aError);
   }
   uint32_t RowSpan() const
   {
@@ -45,7 +45,7 @@ public:
   }
   void SetRowSpan(uint32_t aRowSpan, ErrorResult& aError)
   {
-    SetHTMLIntAttr(nsGkAtoms::rowspan, aRowSpan, aError);
+    SetUnsignedIntAttr(nsGkAtoms::rowspan, aRowSpan, 1, aError);
   }
   //already_AddRefed<nsDOMTokenList> Headers() const;
   void GetHeaders(DOMString& aHeaders)