Make sure that changes to the "span" attribute on a colgroup actually work. Bug 404309, r=bernd, sr=dbaron, a=beltzner
authorbzbarsky@mit.edu
Thu, 14 Feb 2008 20:19:28 -0800
changeset 11746 07f7ab62a14130d7815dcabe2d921bbb322b0b7f
parent 11745 2e36ddcd128937218da02a786f680de3229a94af
child 11747 14a6da8ce102b71a12be96176ecf3343644a1778
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbernd, dbaron, beltzner
bugs404309
milestone1.9b4pre
Make sure that changes to the "span" attribute on a colgroup actually work. Bug 404309, r=bernd, sr=dbaron, a=beltzner
content/html/content/src/nsHTMLTableColElement.cpp
layout/reftests/bugs/404309-1-ref.html
layout/reftests/bugs/404309-1a.html
layout/reftests/bugs/404309-1b.html
layout/reftests/bugs/reftest.list
layout/tables/nsTableColGroupFrame.cpp
--- a/content/html/content/src/nsHTMLTableColElement.cpp
+++ b/content/html/content/src/nsHTMLTableColElement.cpp
@@ -143,16 +143,31 @@ nsHTMLTableColElement::ParseAttribute(PR
 
   return nsGenericHTMLElement::ParseAttribute(aNamespaceID, aAttribute, aValue,
                                               aResult);
 }
 
 static 
 void MapAttributesIntoRule(const nsMappedAttributes* aAttributes, nsRuleData* aData)
 {
+  if (aData->mSIDs & NS_STYLE_INHERIT_BIT(Table) && 
+      aData->mTableData->mSpan.GetUnit() == eCSSUnit_Null) {
+    // span: int
+    const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::span);
+    if (value && value->Type() == nsAttrValue::eInteger) {
+      PRInt32 val = value->GetIntegerValue();
+      // Note: Do NOT use this code for table cells!  The value "0"
+      // means something special for colspan and rowspan, but for <col
+      // span> and <colgroup span> it's just disallowed.
+      if (val > 0) {
+        aData->mTableData->mSpan.SetIntValue(value->GetIntegerValue(),
+                                             eCSSUnit_Integer);
+      }
+    }
+  }
   if ((aData->mSIDs & NS_STYLE_INHERIT_BIT(Position)) &&
       aData->mPositionData->mWidth.GetUnit() == eCSSUnit_Null) {
     // width
     const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::width);
     if (value) {
       switch (value->Type()) {
       case nsAttrValue::ePercent: {
         aData->mPositionData->mWidth.SetPercentValue(value->GetPercentValue());
@@ -182,72 +197,33 @@ void MapAttributesIntoRule(const nsMappe
       if (value && value->Type() == nsAttrValue::eEnum)
         aData->mTextData->mVerticalAlign.SetIntValue(value->GetEnumValue(), eCSSUnit_Enumerated);
     }
   }
 
   nsGenericHTMLElement::MapCommonAttributesInto(aAttributes, aData);
 }
 
-static 
-void ColMapAttributesIntoRule(const nsMappedAttributes* aAttributes,
-                              nsRuleData* aData)
-{
-  if (aData->mSIDs & NS_STYLE_INHERIT_BIT(Table) && 
-      aData->mTableData->mSpan.GetUnit() == eCSSUnit_Null) {
-    // span: int
-    const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::span);
-    if (value && value->Type() == nsAttrValue::eInteger) {
-      PRInt32 val = value->GetIntegerValue();
-      if (val >= 1) {
-        aData->mTableData->mSpan.SetIntValue(value->GetIntegerValue(),
-                                             eCSSUnit_Integer);
-      }
-    }
-  }
-
-  MapAttributesIntoRule(aAttributes, aData);
-}
-
 NS_IMETHODIMP_(PRBool)
 nsHTMLTableColElement::IsAttributeMapped(const nsIAtom* aAttribute) const
 {
   static const MappedAttributeEntry attributes[] = {
     { &nsGkAtoms::width },
     { &nsGkAtoms::align },
     { &nsGkAtoms::valign },
-    { nsnull }
-  };
-
-  static const MappedAttributeEntry span_attribute[] = {
     { &nsGkAtoms::span },
     { nsnull }
   };
 
-  static const MappedAttributeEntry* const col_map[] = {
-    attributes,
-    span_attribute,
-    sCommonAttributeMap,
-  };
-
-  static const MappedAttributeEntry* const colspan_map[] = {
+  static const MappedAttributeEntry* const map[] = {
     attributes,
     sCommonAttributeMap,
   };
 
-  // we only match "span" if we're a <col>
-  if (mNodeInfo->Equals(nsGkAtoms::col))
-    return FindAttributeDependence(aAttribute, col_map,
-                                   NS_ARRAY_LENGTH(col_map));
-  return FindAttributeDependence(aAttribute, colspan_map,
-                                 NS_ARRAY_LENGTH(colspan_map));
+  return FindAttributeDependence(aAttribute, map, NS_ARRAY_LENGTH(map));
 }
 
 
 nsMapRuleToAttributesFunc
 nsHTMLTableColElement::GetAttributeMappingFunction() const
 {
-  if (mNodeInfo->Equals(nsGkAtoms::col)) {
-    return &ColMapAttributesIntoRule;
-  }
-
   return &MapAttributesIntoRule;
 }
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404309-1-ref.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table { background: white }
+colgroup { background: green }
+td { color: white }
+</style>
+</head>
+<body>
+ <table>
+   <colgroup id="x" span="2">
+   <tr>
+     <td>One</td>
+     <td>Two</td>
+     <td>Three</td>
+   </tr>
+ </table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404309-1a.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table { background: white }
+colgroup { background: green }
+td { color: white }
+</style>
+</head>
+<body onload="runTest()">
+ <table>
+   <colgroup id="x" span="1">
+   <tr>
+     <td>One</td>
+     <td>Two</td>
+     <td>Three</td>
+   </tr>
+ </table>
+
+ <script>
+   function runTest() {
+     document.body.offsetWidth;
+     document.getElementById("x").setAttribute("span", 2);
+   }
+ </script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404309-1b.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table { background: white }
+colgroup { background: green }
+td { color: white }
+</style>
+</head>
+<body onload="runTest()">
+ <table>
+   <colgroup id="x" span="3">
+   <tr>
+     <td>One</td>
+     <td>Two</td>
+     <td>Three</td>
+   </tr>
+ </table>
+
+ <script>
+   function runTest() {
+     document.body.offsetWidth;
+     document.getElementById("x").setAttribute("span", 2);
+   }
+ </script>
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -647,16 +647,18 @@ random == 403134-1.html 403134-1-ref.htm
 #== 403657-1.html 403657-1-ref.html  # Fails depending on the fonts...
 == 403733-1.html 403733-1-ref.html
 == 403962-1.xhtml 403962-1-ref.xhtml
 == 404030-1.html 404030-1-ref.html
 != 404030-1-notref.html 404030-1.html
 != 404030-1-notref2.html 404030-1.html
 == 404180-1.html 404180-1-ref.html
 == 404301-1.html 404301-1-ref.html
+== 404309-1a.html 404309-1-ref.html
+== 404309-1b.html 404309-1-ref.html
 != data:application/xml,<foo/> data:text/plain, # bug 404419
 == 404553-1.html 404553-1-ref.html  # assertion test, also tests that marquee binding is applied correctly
 == 404666-1.html 404666-1-ref.html
 == 404666-2.html 404666-2-ref.html
 == 405186-1.xhtml about:blank
 == 405305-1.html 405305-1-ref.html
 == 405380-1.html 405380-1-ref.html
 == 405517-1.xhtml 405517-1-ref.xhtml
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -445,33 +445,17 @@ nsTableColFrame * nsTableColGroupFrame::
     }
     childFrame = childFrame->GetNextSibling();
   }
   return result;
 }
 
 PRInt32 nsTableColGroupFrame::GetSpan()
 {
-  PRInt32 span = 1;
-  nsIContent* iContent = GetContent();
-  if (!iContent) return NS_OK;
-
-  // col group element derives from col element
-  nsIDOMHTMLTableColElement* cgContent = nsnull;
-  nsresult rv = iContent->QueryInterface(NS_GET_IID(nsIDOMHTMLTableColElement), 
-                                         (void **)&cgContent);
-  if (cgContent && NS_SUCCEEDED(rv)) { 
-    cgContent->GetSpan(&span);
-    // XXX why does this work!!
-    if (span == -1) {
-      span = 1;
-    }
-    NS_RELEASE(cgContent);
-  }
-  return span;
+  return GetStyleTable()->mSpan;
 }
 
 void nsTableColGroupFrame::SetContinuousBCBorderWidth(PRUint8     aForSide,
                                                       BCPixelSize aPixelValue)
 {
   switch (aForSide) {
     case NS_SIDE_TOP:
       mTopContBorderWidth = aPixelValue;