Border, margin and padding of an inline's inline ancestors counted twice
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 18:44:03 +0000 (18:44 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 18:44:03 +0000 (18:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=63074

Reviewed by David Hyatt.

Source/WebCore:

In a line such as '<span><span><img>Text' the border, padding and margin belonging
to the two spans was counted twice by RenderBlockLineLayout::nextLineBreak(): once
when adding the width of the <img> object to the line, and a second time when adding
the 'Text'. The result was that nextLineBreak() now had a length for the line that
exceeded the line's maximum length and inserted a bogus line break.

This all happened because the helper function that is used to add in the border etc.
from inline ancestors was crawling up the tree each time. It doesn't need to do that, it
should stop crawling up the tree when the current object is not the first or last sibling below a parent.

Test: fast/inline/bpm-inline-ancestors.html

* rendering/RenderBlockLineLayout.cpp:
(WebCore::shouldAddBorderPaddingMargin): Broke this check out into a helper function so that it
can help inlineLogicalWidth() return early and also treat empty RenderTexts the same as no previous/next
sibling on the line. This ensures that collapsed leading space does not interfere with the decision to
crawl up the ancestors accumulating border, padding, and margin.
(WebCore):
(WebCore::inlineLogicalWidth): Return early once the current child is no longer on the edge of its line -
this ensures the border, padding and margin of ancestors is not counted twice.

LayoutTests:

* fast/inline/bpm-inline-ancestors-expected.html: Added.
* fast/inline/bpm-inline-ancestors.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/inline/bpm-inline-ancestors-expected.html [new file with mode: 0644]
LayoutTests/fast/inline/bpm-inline-ancestors.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockLineLayout.cpp

index 2af1f04..dc8dd5d 100644 (file)
@@ -1,3 +1,13 @@
+2012-10-08  Robert Hogan  <robert@webkit.org>
+
+        Border, margin and padding of an inline's inline ancestors counted twice
+        https://bugs.webkit.org/show_bug.cgi?id=63074
+
+        Reviewed by David Hyatt.
+
+        * fast/inline/bpm-inline-ancestors-expected.html: Added.
+        * fast/inline/bpm-inline-ancestors.html: Added.
+
 2012-10-08  Sudarsana Nagineni  <sudarsana.nagineni@intel.com>
 
         [WK2][WTR] WebKitTestRunner needs testRunner.dispatchPendingLoadRequests
diff --git a/LayoutTests/fast/inline/bpm-inline-ancestors-expected.html b/LayoutTests/fast/inline/bpm-inline-ancestors-expected.html
new file mode 100644 (file)
index 0000000..31c3daa
--- /dev/null
@@ -0,0 +1,45 @@
+<html>
+<head>
+    <title>Float Test Page</title>
+    <style type="text/css">
+        #redblock {            
+            background: red;
+            width: 352px;
+        }
+        #wrapper {
+            border: 1px solid black;
+            width: 500px;
+            height: 200px
+        }
+        body {
+          font: 10px/1em Ahem;
+        }
+        li, ul {
+            display: inline;
+            margin: 0px 10px;
+            border: 0px;
+            padding: 0px;
+        }
+        ul {
+            padding-left: 40px;
+        }
+    </style>
+</head>
+
+<body>
+    <div id="wrapper">
+      <div id="redblock">
+        <ul><li><img src="resources/checker.png" />
+          Should be all one line.</li>
+        </ul>
+      </div>
+      <div id="redblock">
+        <ul><li><img src="resources/checker.png" />
+          Should be all one line.</li>
+        </ul>
+      </div>
+    </div>
+    </div>
+</body>
+</html>
+
diff --git a/LayoutTests/fast/inline/bpm-inline-ancestors.html b/LayoutTests/fast/inline/bpm-inline-ancestors.html
new file mode 100644 (file)
index 0000000..79230e8
--- /dev/null
@@ -0,0 +1,45 @@
+<html>
+<head>
+    <title>Float Test Page</title>
+    <style type="text/css">
+        #redblock {            
+            background: red;
+            float: left;
+        }
+        #wrapper {
+            border: 1px solid black;
+            width: 500px;
+            height: 200px
+        }
+        body {
+          font: 10px/1em Ahem;
+        }
+        li, ul {
+            display: inline;
+            margin: 0px 10px;
+            border: 0px;
+            padding: 0px;
+        }
+        ul {
+            padding-left: 40px;
+        }
+    </style>
+</head>
+
+<body>
+    <div id="wrapper">
+      <div id="redblock">
+        <ul><li><img src="resources/checker.png" />
+          Should be all one line.</li>
+        </ul>
+      </div>
+      <div id="redblock">
+        <ul><li>
+          <img src="resources/checker.png" />
+          Should be all one line.</li>
+        </ul>
+      </div>
+    </div>
+</body>
+</html>
+
index b942855..5e7e5e0 100644 (file)
@@ -1,3 +1,31 @@
+2012-10-08  Robert Hogan  <robert@webkit.org>
+
+        Border, margin and padding of an inline's inline ancestors counted twice
+        https://bugs.webkit.org/show_bug.cgi?id=63074
+
+        Reviewed by David Hyatt.
+
+        In a line such as '<span><span><img>Text' the border, padding and margin belonging
+        to the two spans was counted twice by RenderBlockLineLayout::nextLineBreak(): once
+        when adding the width of the <img> object to the line, and a second time when adding
+        the 'Text'. The result was that nextLineBreak() now had a length for the line that 
+        exceeded the line's maximum length and inserted a bogus line break.
+
+        This all happened because the helper function that is used to add in the border etc.
+        from inline ancestors was crawling up the tree each time. It doesn't need to do that, it
+        should stop crawling up the tree when the current object is not the first or last sibling below a parent.
+
+        Test: fast/inline/bpm-inline-ancestors.html
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::shouldAddBorderPaddingMargin): Broke this check out into a helper function so that it 
+        can help inlineLogicalWidth() return early and also treat empty RenderTexts the same as no previous/next
+        sibling on the line. This ensures that collapsed leading space does not interfere with the decision to
+        crawl up the ancestors accumulating border, padding, and margin.
+        (WebCore):
+        (WebCore::inlineLogicalWidth): Return early once the current child is no longer on the edge of its line - 
+        this ensures the border, padding and margin of ancestors is not counted twice. 
+
 2012-10-08  Mike West  <mkwst@chromium.org>
 
         Null-check for DOMWindow before feeding it to FeatureObserver.
index f7a1ce7..2f933f4 100755 (executable)
@@ -290,6 +290,14 @@ static inline LayoutUnit borderPaddingMarginEnd(RenderInline* child)
     return child->marginEnd() + child->paddingEnd() + child->borderEnd();
 }
 
+static bool shouldAddBorderPaddingMargin(RenderObject* child, bool &checkSide)
+{
+    if (!child || (child->isText() && !toRenderText(child)->textLength()))
+        return true;
+    checkSide = false;
+    return checkSide;
+}
+
 static LayoutUnit inlineLogicalWidth(RenderObject* child, bool start = true, bool end = true)
 {
     unsigned lineDepth = 1;
@@ -297,10 +305,12 @@ static LayoutUnit inlineLogicalWidth(RenderObject* child, bool start = true, boo
     RenderObject* parent = child->parent();
     while (parent->isRenderInline() && lineDepth++ < cMaxLineDepth) {
         RenderInline* parentAsRenderInline = toRenderInline(parent);
-        if (start && !child->previousSibling())
+        if (start && shouldAddBorderPaddingMargin(child->previousSibling(), start))
             extraWidth += borderPaddingMarginStart(parentAsRenderInline);
-        if (end && !child->nextSibling())
+        if (end && shouldAddBorderPaddingMargin(child->nextSibling(), end))
             extraWidth += borderPaddingMarginEnd(parentAsRenderInline);
+        if (!start && !end)
+            return extraWidth;
         child = parent;
         parent = child->parent();
     }