frameborder="no" on frameset is ignored if border attribute set
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2012 00:55:47 +0000 (00:55 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jun 2012 00:55:47 +0000 (00:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=17767

Patch by Elliott Sprehn <esprehn@gmail.com> on 2012-06-28
Reviewed by Tony Chang.

Source/WebCore:

Fixes <frameset> frameborder and border handling. Previously we'd
override the frameborder=no setting if border was set. We also
treated frameborder="anything" the same as frameborder=0 since we
we just converted it to a number so frameborder=yes was incorrectly
treated the same as frameborder=no.

Tests: fast/frames/frameset-frameborder-boolean-values.html
       fast/frames/frameset-frameborder-inheritance.html
       fast/frames/frameset-frameborder-overrides-border.html

* html/HTMLFrameSetElement.cpp: Proper parsing of yes,no,1,0 values.
(WebCore::HTMLFrameSetElement::parseAttribute):
* html/HTMLFrameSetElement.h:
(WebCore::HTMLFrameSetElement::border): Border should be 0 if frameborder=no.

LayoutTests:

Add lots of tests for <frameset> frameborder and border handling.

* fast/frames/frameset-frameborder-boolean-values-expected.txt: Added.
* fast/frames/frameset-frameborder-boolean-values.html: Added.
* fast/frames/frameset-frameborder-inheritance-expected.txt: Added.
* fast/frames/frameset-frameborder-inheritance.html: Added.
* fast/frames/frameset-frameborder-overrides-border-expected.txt: Added.
* fast/frames/frameset-frameborder-overrides-border.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/frames/frameset-frameborder-boolean-values-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/frameset-frameborder-boolean-values.html [new file with mode: 0644]
LayoutTests/fast/frames/frameset-frameborder-inheritance-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/frameset-frameborder-inheritance.html [new file with mode: 0644]
LayoutTests/fast/frames/frameset-frameborder-overrides-border-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/frameset-frameborder-overrides-border.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLFrameSetElement.cpp
Source/WebCore/html/HTMLFrameSetElement.h

index ae0f216..fef8cd1 100644 (file)
@@ -1,3 +1,19 @@
+2012-06-28  Elliott Sprehn  <esprehn@gmail.com>
+
+        frameborder="no" on frameset is ignored if border attribute set
+        https://bugs.webkit.org/show_bug.cgi?id=17767
+
+        Reviewed by Tony Chang.
+
+        Add lots of tests for <frameset> frameborder and border handling.
+
+        * fast/frames/frameset-frameborder-boolean-values-expected.txt: Added.
+        * fast/frames/frameset-frameborder-boolean-values.html: Added.
+        * fast/frames/frameset-frameborder-inheritance-expected.txt: Added.
+        * fast/frames/frameset-frameborder-inheritance.html: Added.
+        * fast/frames/frameset-frameborder-overrides-border-expected.txt: Added.
+        * fast/frames/frameset-frameborder-overrides-border.html: Added.
+
 2012-06-28  Joshua Bell  <jsbell@chromium.org>
 
         IndexedDB: Implement IDBTransaction internal active flag
diff --git a/LayoutTests/fast/frames/frameset-frameborder-boolean-values-expected.txt b/LayoutTests/fast/frames/frameset-frameborder-boolean-values-expected.txt
new file mode 100644 (file)
index 0000000..f0890c3
--- /dev/null
@@ -0,0 +1,39 @@
+Check frameset frameborder attribute allows 1,yes for true and 0,no for false
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+frameborder=1 is true
+PASS frames[0].offsetWidth is 145
+PASS frames[1].offsetWidth is 145
+
+
+frameborder=yes is true
+PASS frames[0].offsetWidth is 145
+PASS frames[1].offsetWidth is 145
+
+
+frameborder=0 is false
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+
+
+frameborder=no is false
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+
+
+Invalid values like frameborder=false are ignored
+PASS frames[0].offsetWidth is 147
+PASS frames[1].offsetWidth is 147
+
+
+Invalid values like frameborder=2 are ignored
+PASS frames[0].offsetWidth is 147
+PASS frames[1].offsetWidth is 147
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+        
diff --git a/LayoutTests/fast/frames/frameset-frameborder-boolean-values.html b/LayoutTests/fast/frames/frameset-frameborder-boolean-values.html
new file mode 100644 (file)
index 0000000..0482af9
--- /dev/null
@@ -0,0 +1,81 @@
+<!doctype html>
+
+<script src="../js/resources/js-test-pre.js"></script>
+
+<iframe data-test="frameborder=1 is true" data-assert-frame-width="145">
+    <!doctype html>
+    <!--
+        To test true values we need to nest in a false value first since the
+        default value is already true. If we didn't do this then the tests would
+        pass even if the browser ignored the attribute entirely.
+    -->
+    <frameset rows="*" cols="*" frameborder="0">
+        <frameset rows="*" cols="50,50" frameborder="1" border="10">
+            <frame src="data:text/html,<body bgcolor=red>">
+            <frame src="data:text/html,<body bgcolor=blue>">
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe data-test="frameborder=yes is true" data-assert-frame-width="145">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0">
+        <frameset rows="*" cols="50,50" frameborder="yes" border="10">
+            <frame src="data:text/html,<body bgcolor=red>">
+            <frame src="data:text/html,<body bgcolor=blue>">
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe data-test="frameborder=0 is false" data-assert-frame-width="150">
+    <!doctype html>
+    <frameset rows="*" cols="50,50" frameborder="0">
+        <frame src="data:text/html,<body bgcolor=red>">
+        <frame src="data:text/html,<body bgcolor=blue>">
+    </frameset>
+</iframe>
+
+<iframe data-test="frameborder=no is false" data-assert-frame-width="150">
+    <!doctype html>
+    <frameset rows="*" cols="50,50" frameborder="nO">
+        <frame src="data:text/html,<body bgcolor=red>">
+        <frame src="data:text/html,<body bgcolor=blue>">
+    </frameset>
+</iframe>
+
+<iframe data-test="Invalid values like frameborder=false are ignored" data-assert-frame-width="147">
+    <!doctype html>
+    <frameset rows="*" cols="50,50" frameborder="false">
+        <frame src="data:text/html,<body bgcolor=red>">
+        <frame src="data:text/html,<body bgcolor=blue>">
+    </frameset>
+</iframe>
+
+<iframe data-test="Invalid values like frameborder=2 are ignored" data-assert-frame-width="147">
+    <!doctype html>
+    <frameset rows="*" cols="50,50" frameborder="2">
+        <frame src="data:text/html,<body bgcolor=red>">
+        <frame src="data:text/html,<body bgcolor=blue>">
+    </frameset>
+</iframe>
+
+<script>
+    description('Check frameset frameborder attribute allows 1,yes for true and 0,no for false');
+
+    iframes = [].slice.call(document.querySelectorAll('iframe'));
+    iframes.forEach(function(iframe) {
+        // Can't use srcdoc since that wouldn't synchronously load the content.
+        iframe.contentDocument.write(iframe.textContent);
+        iframe.contentDocument.close();
+
+        frameset = iframe.contentDocument.querySelector('frameset');
+        expectedWidth = iframe.dataset.assertFrameWidth;
+        frames = frameset.querySelectorAll('frame');
+        debug(iframe.dataset.test);
+        shouldBe('frames[0].offsetWidth', expectedWidth);
+        shouldBe('frames[1].offsetWidth', expectedWidth);
+        debug('<br>');
+    });
+</script>
+
+<script src="../js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/frames/frameset-frameborder-inheritance-expected.txt b/LayoutTests/fast/frames/frameset-frameborder-inheritance-expected.txt
new file mode 100644 (file)
index 0000000..078bfa0
--- /dev/null
@@ -0,0 +1,35 @@
+Check frameset frameborder and border attribute inheritance
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+missing frameborder attribute should inherit it
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+
+
+inherited frameborder=no should inherit border=0
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+
+
+frameborder=yes and border should override inherited value
+PASS frames[0].offsetWidth is 145
+PASS frames[1].offsetWidth is 145
+
+
+inherited frameborder=0 should have border=0 even if specified differently
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+
+
+bordercolor should inherit even if frameborder=0
+PASS frames[0].offsetWidth is 145
+PASS frames[1].offsetWidth is 145
+PASS getComputedStyle(frameset).borderColor is "rgb(0, 128, 0)"
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+      
diff --git a/LayoutTests/fast/frames/frameset-frameborder-inheritance.html b/LayoutTests/fast/frames/frameset-frameborder-inheritance.html
new file mode 100644 (file)
index 0000000..aed3030
--- /dev/null
@@ -0,0 +1,100 @@
+<!doctype html>
+
+<script src="../js/resources/js-test-pre.js"></script>
+
+<iframe
+    data-test="missing frameborder attribute should inherit it"
+    data-assert-frame-width="150">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0">
+        <frameset rows="*" cols="50,50">
+            <frame src="data:text/html,<body bgcolor=red>">
+            <frame src="data:text/html,<body bgcolor=blue>">
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe
+    data-test="inherited frameborder=no should inherit border=0"
+    data-assert-frame-width="150">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0">
+        <frameset rows="*" cols="50,50" frameborder="1">
+            <frame src="data:text/html,<body bgcolor=green>">
+            <frame src="data:text/html,<body bgcolor=yellow>">
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe
+    data-test="frameborder=yes and border should override inherited value"
+    data-assert-frame-width="145">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0">
+        <frameset rows="*" cols="50,50" frameborder="1" border="10">
+            <frame src="data:text/html,<body bgcolor=orange>">
+            <frame src="data:text/html,<body bgcolor=pink>">
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe
+    data-test="inherited frameborder=0 should have border=0 even if specified differently"
+    data-assert-frame-width="150">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0" border="10">
+        <!--
+
+            This inherits frameborder="0" so the border="20" should be ignored
+            and children of it should inherit border="0".
+        -->
+        <frameset rows="*" cols="*" border="20">
+            <frameset rows="*" cols="50,50" frameborder="1">
+                <frame src="data:text/html,<body bgcolor=green>">
+                <frame src="data:text/html,<body bgcolor=yellow>">
+            </frameset>
+        </frameset>
+    </frameset>
+</iframe>
+
+<iframe
+    data-test="bordercolor should inherit even if frameborder=0"
+    data-assert-frame-width="145"
+    data-assert-border-color="rgb(0, 128, 0)">
+    <!doctype html>
+    <frameset rows="*" cols="*" frameborder="0" bordercolor="green">
+        <frameset rows="*" cols="*" frameborder="1">
+            <frameset rows="*" cols="50,50" border="10">
+                <frame src="data:text/html,<body bgcolor=red>">
+                <frame src="data:text/html,<body bgcolor=blue>">
+            </frameset>
+        </frameset>
+    </frameset>
+</iframe>
+
+<script>
+    description('Check frameset frameborder and border attribute inheritance');
+
+    iframes = [].slice.call(document.querySelectorAll('iframe'));
+    iframes.forEach(function(iframe) {
+        // Can't use srcdoc since that wouldn't synchronously load the content.
+        iframe.contentDocument.write(iframe.textContent);
+        iframe.contentDocument.close();
+
+        frameset = iframe.contentDocument.querySelector('frameset');
+        expectedWidth = iframe.dataset.assertFrameWidth;
+        expectedBorderColor = iframe.dataset.assertBorderColor;
+        frames = frameset.querySelectorAll('frame');
+        debug(iframe.dataset.test);
+        shouldBe('frames[0].offsetWidth', expectedWidth);
+        shouldBe('frames[1].offsetWidth', expectedWidth);
+        if (expectedBorderColor) {
+            // This check won't work in Gecko since they don't expose the border color properly.
+            frameset = frames[0].parentNode;
+            shouldBeEqualToString('getComputedStyle(frameset).borderColor', expectedBorderColor);
+        }
+        debug('<br>');
+    });
+</script>
+
+<script src="../js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/frames/frameset-frameborder-overrides-border-expected.txt b/LayoutTests/fast/frames/frameset-frameborder-overrides-border-expected.txt
new file mode 100644 (file)
index 0000000..b10522b
--- /dev/null
@@ -0,0 +1,11 @@
+Bug 17767: Frameset with frameborder=0 and border=N should not show a border
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS frames[0].offsetWidth is 150
+PASS frames[1].offsetWidth is 150
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/frames/frameset-frameborder-overrides-border.html b/LayoutTests/fast/frames/frameset-frameborder-overrides-border.html
new file mode 100644 (file)
index 0000000..f927c95
--- /dev/null
@@ -0,0 +1,26 @@
+<!doctype html>
+
+<script src="../js/resources/js-test-pre.js"></script>
+
+<iframe>
+    <!doctype html>
+    <frameset rows="*" cols="150,*" frameborder="0" border="10">
+        <frame src="data:text/html,<body bgcolor=green>">
+        <frame src="data:text/html,<body bgcolor=yellow>">
+    </frameset>
+</iframe>
+
+<script>
+    // Can't use srcdoc since that wouldn't synchronously load the content.
+    frame = document.querySelector('iframe');
+    frameDoc = frame.contentDocument;
+    frameDoc.write(frame.textContent);
+    frameDoc.close();
+
+    description('Bug 17767: Frameset with frameborder=0 and border=N should not show a border');
+    frames = frameDoc.querySelectorAll('frame');
+    shouldBe('frames[0].offsetWidth', '150');
+    shouldBe('frames[1].offsetWidth', '150');
+</script>
+
+<script src="../js/resources/js-test-post.js"></script>
index 05a251f..1d90cd1 100644 (file)
@@ -1,3 +1,25 @@
+2012-06-28  Elliott Sprehn  <esprehn@gmail.com>
+
+        frameborder="no" on frameset is ignored if border attribute set
+        https://bugs.webkit.org/show_bug.cgi?id=17767
+
+        Reviewed by Tony Chang.
+
+        Fixes <frameset> frameborder and border handling. Previously we'd
+        override the frameborder=no setting if border was set. We also
+        treated frameborder="anything" the same as frameborder=0 since we
+        we just converted it to a number so frameborder=yes was incorrectly
+        treated the same as frameborder=no.
+
+        Tests: fast/frames/frameset-frameborder-boolean-values.html
+               fast/frames/frameset-frameborder-inheritance.html
+               fast/frames/frameset-frameborder-overrides-border.html
+
+        * html/HTMLFrameSetElement.cpp: Proper parsing of yes,no,1,0 values.
+        (WebCore::HTMLFrameSetElement::parseAttribute):
+        * html/HTMLFrameSetElement.h:
+        (WebCore::HTMLFrameSetElement::border): Border should be 0 if frameborder=no.
+
 2012-06-28  Joshua Bell  <jsbell@chromium.org>
 
         IndexedDB: Implement IDBTransaction internal active flag
index b0e7f04..a2e2f82 100644 (file)
@@ -93,12 +93,13 @@ void HTMLFrameSetElement::parseAttribute(const Attribute& attribute)
         }
     } else if (attribute.name() == frameborderAttr) {
         if (!attribute.isNull()) {
-            // false or "no" or "0"..
-            if (attribute.value().toInt() == 0) {
+            const AtomicString& value = attribute.value();
+            if (equalIgnoringCase(value, "no") || equalIgnoringCase(value, "0")) {
                 m_frameborder = false;
-                m_border = 0;
+                m_frameborderSet = true;
+            } else if (equalIgnoringCase(value, "yes") || equalIgnoringCase(value, "1")) {
+                m_frameborderSet = true;
             }
-            m_frameborderSet = true;
         } else {
             m_frameborder = false;
             m_frameborderSet = false;
@@ -108,8 +109,6 @@ void HTMLFrameSetElement::parseAttribute(const Attribute& attribute)
     } else if (attribute.name() == borderAttr) {
         if (!attribute.isNull()) {
             m_border = attribute.value().toInt();
-            if (!m_border)
-                m_frameborder = false;
             m_borderSet = true;
         } else
             m_borderSet = false;
index a4c391b..9c3311a 100644 (file)
@@ -38,7 +38,7 @@ public:
 
     int totalRows() const { return m_totalRows; }
     int totalCols() const { return m_totalCols; }
-    int border() const { return m_border; }
+    int border() const { return hasFrameBorder() ? m_border : 0; }
 
     bool hasBorderColor() const { return m_borderColorSet; }