Changeset 130663 in webkit

Timestamp:

Oct 8, 2012, 11:44:03 AM (14 years ago)

Author:

robert@webkit.org

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/inline/bpm-inline-ancestors-expected.html (added)
• LayoutTests/fast/inline/bpm-inline-ancestors.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -291 +291 @@
 }
 
+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)
 {
@@ -298 +306 @@
     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();