Account for margin after when laying out <legend> element
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Jun 2012 23:14:38 +0000 (23:14 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Jun 2012 23:14:38 +0000 (23:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=35981

Reviewed by Abhishek Arya.

Source/WebCore:

Tests: fast/forms/legend-after-margin-horizontal-writing-mode.html
       fast/forms/legend-after-margin-vertical-writing-mode.html
       fast/forms/legend-after-margin-with-before-border-horizontal-mode.html
       fast/forms/legend-small-after-margin-before-border-horizontal-mode.html

The existing code would ignore margin after when layouting out the <legend>. This
change only adds the code to handle the margin after, the margin before is still
ignored as it's not obvious how it should be working.

* rendering/RenderFieldset.cpp:
(WebCore::RenderFieldset::layoutSpecialExcludedChild):
Split the code in 2 code paths to reflect how we position and size. Those are covered by the
tests above.

LayoutTests:

* fast/forms/legend-after-margin-horizontal-writing-mode-expected.html: Added.
* fast/forms/legend-after-margin-horizontal-writing-mode.html: Added.
* fast/forms/legend-after-margin-vertical-writing-mode-expected.html: Added.
* fast/forms/legend-after-margin-vertical-writing-mode.html: Added.
* fast/forms/legend-after-margin-with-before-border-horizontal-mode-expected.html: Added.
* fast/forms/legend-after-margin-with-before-border-horizontal-mode.html: Added.
* fast/forms/legend-small-after-margin-before-border-horizontal-mode-expected.html: Added.
* fast/forms/legend-small-after-margin-before-border-horizontal-mode.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode-expected.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode-expected.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode-expected.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode-expected.html [new file with mode: 0644]
LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFieldset.cpp

index ccf3df17c7cd2e43c30c77a8f63f37c8d851b38f..6ac1f2504ccb1197f3c79936b48b5c9a051c7c98 100644 (file)
@@ -1,3 +1,19 @@
+2012-06-11  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Account for margin after when laying out <legend> element
+        https://bugs.webkit.org/show_bug.cgi?id=35981
+
+        Reviewed by Abhishek Arya.
+
+        * fast/forms/legend-after-margin-horizontal-writing-mode-expected.html: Added.
+        * fast/forms/legend-after-margin-horizontal-writing-mode.html: Added.
+        * fast/forms/legend-after-margin-vertical-writing-mode-expected.html: Added.
+        * fast/forms/legend-after-margin-vertical-writing-mode.html: Added.
+        * fast/forms/legend-after-margin-with-before-border-horizontal-mode-expected.html: Added.
+        * fast/forms/legend-after-margin-with-before-border-horizontal-mode.html: Added.
+        * fast/forms/legend-small-after-margin-before-border-horizontal-mode-expected.html: Added.
+        * fast/forms/legend-small-after-margin-before-border-horizontal-mode.html: Added.
+
 2012-06-05  Eric Uhrhane <ericu@chromium.org>
 
         Crash in fast/files/read tests during Garbage Collection
diff --git a/LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode-expected.html b/LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode-expected.html
new file mode 100644 (file)
index 0000000..b3065d1
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+    }
+    fieldset { background: #ddd; }
+    p { margin-top: 50px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>Legend</legend>
+        <p>There should be a 50px gap above to account for the margin-bottom on the legend.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode.html b/LayoutTests/fast/forms/legend-after-margin-horizontal-writing-mode.html
new file mode 100644 (file)
index 0000000..bb256e8
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+        margin-bottom: 50px;
+    }
+    fieldset { background: #ddd; }
+    p { margin-top: 0px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>Legend</legend>
+        <p>There should be a 50px gap above to account for the margin-bottom on the legend.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode-expected.html b/LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode-expected.html
new file mode 100644 (file)
index 0000000..d0c43d1
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    form {
+        -webkit-writing-mode: vertical-lr;
+        writing-mode: vertical-lr;
+    }
+    legend {
+        background: #fc0;
+    }
+    fieldset { background: #ddd; }
+    p { margin-left: 50px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>Legend</legend>
+        <p>There should be a 50px gap on the right to account for the margin-bottom on the legend.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode.html b/LayoutTests/fast/forms/legend-after-margin-vertical-writing-mode.html
new file mode 100644 (file)
index 0000000..baed177
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    form {
+        -webkit-writing-mode: vertical-lr;
+        writing-mode: vertical-lr;
+    }
+    legend {
+        background: #fc0;
+        margin-right: 50px;
+    }
+    fieldset { background: #ddd; }
+    p { margin-left: 0px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>Legend</legend>
+        <p>There should be a 50px gap on the right to account for the margin-bottom on the legend.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode-expected.html b/LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode-expected.html
new file mode 100644 (file)
index 0000000..e9db079
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+        font: 10px Ahem;
+    }
+    fieldset {
+        border-top: 70px solid #ddd;
+        background: #ddd;
+    }
+    p { margin-top: 20px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>xxxx</legend>
+        <p>There should be a 20px gap above to account for the margin bottom collapsing into the fieldset before border.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode.html b/LayoutTests/fast/forms/legend-after-margin-with-before-border-horizontal-mode.html
new file mode 100644 (file)
index 0000000..572bd0e
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+        margin-bottom: 50px;
+        font: 10px Ahem;
+    }
+    fieldset {
+        border-top: 70px solid #ddd;
+        background: #ddd;
+    }
+    p { margin-top: 0px; }
+</style>
+<body>
+<div>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</div>
+<form>
+    <fieldset>
+        <legend>xxxx</legend>
+        <p>There should be a 20px gap above to account for the margin bottom collapsing into the fieldset before border.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode-expected.html b/LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode-expected.html
new file mode 100644 (file)
index 0000000..78dd7be
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+        font: 10px Ahem;
+    }
+    fieldset {
+        border-top: 70px solid #ddd;
+        background: #ddd;
+    }
+    p { margin-top: 0px; }
+</style>
+<body>
+<p>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</p>
+<form>
+    <fieldset>
+        <legend>xxxx</legend>
+        <p>There should be no gap above as the legend's margin bottom should be collapsed into the fieldset's border.</p>
+    </fieldset>
+</form>
+</body>
+</html>
diff --git a/LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode.html b/LayoutTests/fast/forms/legend-small-after-margin-before-border-horizontal-mode.html
new file mode 100644 (file)
index 0000000..2ea9297
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    legend {
+        background: #fc0;
+        margin-bottom: 30px;
+        font: 10px Ahem;
+    }
+    fieldset {
+        border-top: 70px solid #ddd;
+        background: #ddd;
+    }
+    p { margin-top: 0px; }
+</style>
+<body>
+<p>bug <a href="https://bugs.webkit.org/show_bug.cgi?id=35981">35981</a>: Can't apply a bottom-margin to the legend element</p>
+<form>
+    <fieldset>
+        <legend>xxxx</legend>
+        <p>There should be no gap above as the legend's margin bottom should be collapsed into the fieldset's border.</p>
+    </fieldset>
+</form>
+</body>
+</html>
index cde10730d06881642b3ce7b310fce4cd301509e1..c433637139287c50407f371bd9ef4c1ea58e3e23 100644 (file)
@@ -1,3 +1,24 @@
+2012-06-11  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Account for margin after when laying out <legend> element
+        https://bugs.webkit.org/show_bug.cgi?id=35981
+
+        Reviewed by Abhishek Arya.
+
+        Tests: fast/forms/legend-after-margin-horizontal-writing-mode.html
+               fast/forms/legend-after-margin-vertical-writing-mode.html
+               fast/forms/legend-after-margin-with-before-border-horizontal-mode.html
+               fast/forms/legend-small-after-margin-before-border-horizontal-mode.html
+
+        The existing code would ignore margin after when layouting out the <legend>. This
+        change only adds the code to handle the margin after, the margin before is still
+        ignored as it's not obvious how it should be working.
+
+        * rendering/RenderFieldset.cpp:
+        (WebCore::RenderFieldset::layoutSpecialExcludedChild):
+        Split the code in 2 code paths to reflect how we position and size. Those are covered by the
+        tests above.
+
 2012-06-11  James Robinson  <jamesr@chromium.org>
 
         [chromium] Use WebGraphicsContext3D in rate limiting logic inside compositor
index 8870a872f8f9afda1a164816be05f35ea24fc17e..3138f22fc26f4793bc6896418e1762fbbd8a0f06 100644 (file)
@@ -101,10 +101,24 @@ RenderObject* RenderFieldset::layoutSpecialExcludedChild(bool relayoutChildren)
 
         setLogicalLeftForChild(legend, logicalLeft);
 
-        LayoutUnit b = borderBefore();
-        LayoutUnit h = logicalHeightForChild(legend);
-        setLogicalTopForChild(legend, max<LayoutUnit>((b - h) / 2, 0));
-        setLogicalHeight(max(b, h) + paddingBefore());
+        LayoutUnit fieldsetBorderBefore = borderBefore();
+        LayoutUnit legendLogicalHeight = logicalHeightForChild(legend);
+
+        LayoutUnit legendLogicalTop;
+        LayoutUnit collapsedLegendExtent;
+        // FIXME: We need to account for the legend's margin before too.
+        if (fieldsetBorderBefore > legendLogicalHeight) {
+            // The <legend> is smaller than the associated fieldset before border
+            // so the latter determines positioning of the <legend>. The sizing depends
+            // on the legend's margins as we want to still follow the author's cues.
+            // Firefox completely ignores the margins in this case which seems wrong.
+            legendLogicalTop = (fieldsetBorderBefore - legendLogicalHeight) / 2;
+            collapsedLegendExtent = max<LayoutUnit>(fieldsetBorderBefore, legendLogicalTop + legendLogicalHeight + marginAfterForChild(legend));
+        } else
+            collapsedLegendExtent = legendLogicalHeight + marginAfterForChild(legend);
+
+        setLogicalTopForChild(legend, legendLogicalTop);
+        setLogicalHeight(paddingBefore() + collapsedLegendExtent);
     }
     return legend;
 }