When we do setAttribute("border", null) on a table we should create a border like...
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jan 2013 18:22:01 +0000 (18:22 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jan 2013 18:22:01 +0000 (18:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=102112

Reviewed by Ryosuke Niwa.

Source/WebCore:

http://www.whatwg.org/specs/web-apps/current-work/multipage/rendering.html#tables says:
"If the [table's border] attribute is present but parsing the attribute's value using the rules for parsing
non-negative integers generates an error, a default value of 1px is expected to be used for that property instead."

Match the spec and bring us into line with other browsers by observing the 'parsing non-negative integers' algorithm.

Tests: fast/dom/HTMLTableElement/table-with-invalid-border.html
       fast/table/table-with-borderattr-null.html
       fast/table/table-with-borderattr-set-to-null.html

* html/HTMLElement.cpp:
(WebCore::HTMLElement::parseBorderWidthAttribute):
(WebCore::HTMLElement::applyBorderAttributeToStyle):
* html/HTMLElement.h:
(HTMLElement):
* html/HTMLTableElement.cpp:
(WebCore::HTMLTableElement::collectStyleForPresentationAttribute):
(WebCore::HTMLTableElement::parseAttribute):

LayoutTests:

* fast/dom/HTMLTableElement/table-with-invalid-border-expected.txt: Added.
* fast/dom/HTMLTableElement/table-with-invalid-border.html: Added.
* fast/table/table-with-borderattr-null-expected.txt: Added.
* fast/table/table-with-borderattr-null.html: Added.
* fast/table/table-with-borderattr-set-to-null-expected.txt: Added.
* fast/table/table-with-borderattr-set-to-null.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140436 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html [new file with mode: 0644]
LayoutTests/fast/table/table-with-borderattr-null-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/table-with-borderattr-null.html [new file with mode: 0644]
LayoutTests/fast/table/table-with-borderattr-set-to-null-expected.txt [new file with mode: 0644]
LayoutTests/fast/table/table-with-borderattr-set-to-null.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLElement.cpp
Source/WebCore/html/HTMLElement.h
Source/WebCore/html/HTMLTableElement.cpp

index 5969cf0..9f4f776 100644 (file)
@@ -1,3 +1,17 @@
+2013-01-22  Robert Hogan  <robert@webkit.org>
+
+        When we do setAttribute("border", null) on a table we should create a border like every other browser
+        https://bugs.webkit.org/show_bug.cgi?id=102112
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/dom/HTMLTableElement/table-with-invalid-border-expected.txt: Added.
+        * fast/dom/HTMLTableElement/table-with-invalid-border.html: Added.
+        * fast/table/table-with-borderattr-null-expected.txt: Added.
+        * fast/table/table-with-borderattr-null.html: Added.
+        * fast/table/table-with-borderattr-set-to-null-expected.txt: Added.
+        * fast/table/table-with-borderattr-set-to-null.html: Added.
+
 2013-01-22  Abhishek Arya  <inferno@chromium.org>
 
         Heap-use-after-free in WebCore::RenderObject::isDescendantOf
diff --git a/LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border-expected.txt b/LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border-expected.txt
new file mode 100644 (file)
index 0000000..8948e86
--- /dev/null
@@ -0,0 +1,3 @@
+https://bugs.webkit.org/show_bug.cgi?id=102112: There should be a black box below. 
+PASS getComputedStyle(document.querySelector("table")).borderTopWidth is "1px"
+
diff --git a/LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html b/LayoutTests/fast/dom/HTMLTableElement/table-with-invalid-border.html
new file mode 100644 (file)
index 0000000..9f4fc52
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<head>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+</script>
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+    <p>https://bugs.webkit.org/show_bug.cgi?id=102112: There should be a black box below. <br> 
+
+    <table border="djska" width="100%" height="100%">
+        <tr>
+            <td height="26"></td>
+        </tr>
+    </table>
+    <div id="console"></div>
+    
+    <script>
+    shouldBeEqualToString('getComputedStyle(document.querySelector("table")).borderTopWidth', "1px");
+    </script>
+</body>
diff --git a/LayoutTests/fast/table/table-with-borderattr-null-expected.txt b/LayoutTests/fast/table/table-with-borderattr-null-expected.txt
new file mode 100644 (file)
index 0000000..8a95913
--- /dev/null
@@ -0,0 +1,3 @@
+There should be a black box below. 
+PASS getComputedStyle(document.querySelector("table")).borderTopWidth is "1px"
+
diff --git a/LayoutTests/fast/table/table-with-borderattr-null.html b/LayoutTests/fast/table/table-with-borderattr-null.html
new file mode 100644 (file)
index 0000000..961f17a
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<head>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+</script>
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+    <p>There should be a black box below. <br> 
+
+    <table id="table" border="null" width="100%" height="100%">
+        <tr>
+            <td height="26"></td>
+        </tr>
+    </table>
+    <div id="console"></div>
+    
+    <script>
+    shouldBeEqualToString('getComputedStyle(document.querySelector("table")).borderTopWidth', "1px");
+    </script>
+</body>
diff --git a/LayoutTests/fast/table/table-with-borderattr-set-to-null-expected.txt b/LayoutTests/fast/table/table-with-borderattr-set-to-null-expected.txt
new file mode 100644 (file)
index 0000000..8a95913
--- /dev/null
@@ -0,0 +1,3 @@
+There should be a black box below. 
+PASS getComputedStyle(document.querySelector("table")).borderTopWidth is "1px"
+
diff --git a/LayoutTests/fast/table/table-with-borderattr-set-to-null.html b/LayoutTests/fast/table/table-with-borderattr-set-to-null.html
new file mode 100644 (file)
index 0000000..8423167
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<head>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+</script>
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+    <p>There should be a black box below. <br> 
+
+    <table id="table" width="100%" height="100%">
+        <tr>
+            <td height="26"></td>
+        </tr>
+    </table>
+
+    <div id="console"></div>
+    <script>
+        var table = document.getElementById("table");
+        table.setAttribute("border", null);
+        shouldBeEqualToString('getComputedStyle(document.querySelector("table")).borderTopWidth', "1px");
+    </script>
+</body>
index 00beea6..631f5a4 100644 (file)
@@ -1,3 +1,29 @@
+2013-01-22  Robert Hogan  <robert@webkit.org>
+
+        When we do setAttribute("border", null) on a table we should create a border like every other browser
+        https://bugs.webkit.org/show_bug.cgi?id=102112
+
+        Reviewed by Ryosuke Niwa.
+
+        http://www.whatwg.org/specs/web-apps/current-work/multipage/rendering.html#tables says:
+        "If the [table's border] attribute is present but parsing the attribute's value using the rules for parsing 
+        non-negative integers generates an error, a default value of 1px is expected to be used for that property instead."
+
+        Match the spec and bring us into line with other browsers by observing the 'parsing non-negative integers' algorithm.
+
+        Tests: fast/dom/HTMLTableElement/table-with-invalid-border.html
+               fast/table/table-with-borderattr-null.html
+               fast/table/table-with-borderattr-set-to-null.html
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::parseBorderWidthAttribute):
+        (WebCore::HTMLElement::applyBorderAttributeToStyle):
+        * html/HTMLElement.h:
+        (HTMLElement):
+        * html/HTMLTableElement.cpp:
+        (WebCore::HTMLTableElement::collectStyleForPresentationAttribute):
+        (WebCore::HTMLTableElement::parseAttribute):
+
 2013-01-22  Abhishek Arya  <inferno@chromium.org>
 
         Heap-use-after-free in WebCore::RenderObject::isDescendantOf
index 6b4ad30..bd792e9 100644 (file)
@@ -133,18 +133,18 @@ static inline int unicodeBidiAttributeForDirAuto(HTMLElement* element)
     return CSSValueWebkitIsolate;
 }
 
-static unsigned parseBorderWidthAttribute(const Attribute& attribute)
+unsigned HTMLElement::parseBorderWidthAttribute(const QualifiedName& name, const AtomicString& value) const
 {
-    ASSERT(attribute.name() == borderAttr);
+    ASSERT_UNUSED(name, name == borderAttr);
     unsigned borderWidth = 0;
-    if (!attribute.isEmpty())
-        parseHTMLNonNegativeInteger(attribute.value(), borderWidth);
+    if (value.isEmpty() || !parseHTMLNonNegativeInteger(value, borderWidth))
+        return hasLocalName(tableTag) ? 1 : borderWidth;
     return borderWidth;
 }
 
 void HTMLElement::applyBorderAttributeToStyle(const Attribute& attribute, StylePropertySet* style)
 {
-    addPropertyToPresentationAttributeStyle(style, CSSPropertyBorderWidth, parseBorderWidthAttribute(attribute), CSSPrimitiveValue::CSS_PX);
+    addPropertyToPresentationAttributeStyle(style, CSSPropertyBorderWidth, parseBorderWidthAttribute(attribute.name(), attribute.value()), CSSPrimitiveValue::CSS_PX);
     addPropertyToPresentationAttributeStyle(style, CSSPropertyBorderStyle, CSSValueSolid);
 }
 
index 28521a8..c075069 100644 (file)
@@ -120,6 +120,7 @@ protected:
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) OVERRIDE;
     virtual bool isPresentationAttribute(const QualifiedName&) const OVERRIDE;
     virtual void collectStyleForPresentationAttribute(const Attribute&, StylePropertySet*) OVERRIDE;
+    unsigned parseBorderWidthAttribute(const QualifiedName&, const AtomicString& value) const;
 
     virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
     void calculateAndAdjustDirectionality();
index c27d519..59f0b0c 100644 (file)
@@ -310,10 +310,9 @@ void HTMLTableElement::collectStyleForPresentationAttribute(const Attribute& att
         addHTMLLengthToStyle(style, CSSPropertyWidth, attribute.value());
     else if (attribute.name() == heightAttr)
         addHTMLLengthToStyle(style, CSSPropertyHeight, attribute.value());
-    else if (attribute.name() == borderAttr) {
-        int borderWidth = attribute.isEmpty() ? 1 : attribute.value().toInt();
-        addPropertyToPresentationAttributeStyle(style, CSSPropertyBorderWidth, borderWidth, CSSPrimitiveValue::CSS_PX);
-    } else if (attribute.name() == bordercolorAttr) {
+    else if (attribute.name() == borderAttr) 
+        addPropertyToPresentationAttributeStyle(style, CSSPropertyBorderWidth, parseBorderWidthAttribute(attribute.name(), attribute.value()), CSSPrimitiveValue::CSS_PX);
+    else if (attribute.name() == bordercolorAttr) {
         if (!attribute.isEmpty())
             addHTMLColorToStyle(style, CSSPropertyBorderColor, attribute.value());
     } else if (attribute.name() == bgcolorAttr)
@@ -376,11 +375,7 @@ void HTMLTableElement::parseAttribute(const QualifiedName& name, const AtomicStr
 
     if (name == borderAttr)  {
         // FIXME: This attribute is a mess.
-        m_borderAttr = true;
-        if (!value.isNull()) {
-            int border = value.isEmpty() ? 1 : value.toInt();
-            m_borderAttr = border;
-        }
+        m_borderAttr = parseBorderWidthAttribute(name, value);
     } else if (name == bordercolorAttr) {
         m_borderColorAttr = !value.isEmpty();
     } else if (name == frameAttr) {