Changeset 142889 in webkit

Timestamp:

Feb 14, 2013, 9:53:16 AM (12 years ago)

Author:

tony@chromium.org

Message:

Padding and border changes doesn't trigger relayout of children
​https://bugs.webkit.org/show_bug.cgi?id=109639

Reviewed by Kent Tamura.

Source/WebCore:

In RenderBlock::layoutBlock, we only relayout our children if our logical width
changes. This misses cases where our logical width doesn't change (i.e., padding
or border changes), but our content width does change.

This is a more general case of bug 104997.

Test: fast/block/dynamic-padding-border.html

• rendering/RenderBox.cpp:

(WebCore::borderOrPaddingLogicalWidthChanged): Only check if the logical width changed.
(WebCore::RenderBox::styleDidChange): Drop the border-box condition since this can happen
even without border-box box sizing.

LayoutTests:

• fast/block/dynamic-padding-border-expected.txt: Added.
• fast/block/dynamic-padding-border.html: Added.
• fast/table/border-collapsing/cached-change-row-border-width-expected.txt: We should have been relaying

out the table when the border changed. The pixel results in this case is the same, but the
render tree shows the difference.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/block/dynamic-padding-border-expected.txt (added)
• LayoutTests/fast/block/dynamic-padding-border.html (added)
• LayoutTests/fast/table/border-collapsing/cached-change-row-border-width-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-14  Tony Chang  <tony@chromium.org>
+
+        Padding and border changes doesn't trigger relayout of children
+        https://bugs.webkit.org/show_bug.cgi?id=109639
+
+        Reviewed by Kent Tamura.
+
+        * fast/block/dynamic-padding-border-expected.txt: Added.
+        * fast/block/dynamic-padding-border.html: Added.
+        * fast/table/border-collapsing/cached-change-row-border-width-expected.txt: We should have been relaying
+        out the table when the border changed. The pixel results in this case is the same, but the
+        render tree shows the difference.
+
 2013-02-14  Vsevolod Vlasov  <vsevik@chromium.org>
 

trunk/LayoutTests/fast/table/border-collapsing/cached-change-row-border-width-expected.txt

@@ -7 +7 @@
         RenderTableSection {TBODY} at (1,2) size 54x100
           RenderTableRow {TR} at (0,0) size 54x50 [border: (4px solid #FFFF00)]
-            RenderTableCell {TD} at (0,23) size 54x4 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
+            RenderTableCell {TD} at (0,22) size 54x6 [border: (2px solid #00FF00)] [r=0 c=0 rs=1 cs=1]
           RenderTableRow {TR} at (0,50) size 54x50
             RenderTableCell {TD} at (0,73) size 54x3 [border: (2px none #000000)] [r=1 c=0 rs=1 cs=1]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-14  Tony Chang  <tony@chromium.org>
+
+        Padding and border changes doesn't trigger relayout of children
+        https://bugs.webkit.org/show_bug.cgi?id=109639
+
+        Reviewed by Kent Tamura.
+
+        In RenderBlock::layoutBlock, we only relayout our children if our logical width
+        changes. This misses cases where our logical width doesn't change (i.e., padding
+        or border changes), but our content width does change.
+
+        This is a more general case of bug 104997.
+
+        Test: fast/block/dynamic-padding-border.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::borderOrPaddingLogicalWidthChanged): Only check if the logical width changed.
+        (WebCore::RenderBox::styleDidChange): Drop the border-box condition since this can happen
+        even without border-box box sizing.
+
 2013-02-14  Peter Rybin  <prybin@chromium.org>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -233 +233 @@
 }
 
-static bool borderWidthChanged(const RenderStyle* oldStyle, const RenderStyle* newStyle)
-{
-    return oldStyle->borderLeftWidth() != newStyle->borderLeftWidth()
-        || oldStyle->borderTopWidth() != newStyle->borderTopWidth()
-        || oldStyle->borderRightWidth() != newStyle->borderRightWidth()
-        || oldStyle->borderBottomWidth() != newStyle->borderBottomWidth();
+static bool borderOrPaddingLogicalWidthChanged(const RenderStyle* oldStyle, const RenderStyle* newStyle)
+{
+    if (newStyle->isHorizontalWritingMode())
+        return oldStyle->borderLeftWidth() != newStyle->borderLeftWidth()
+            || oldStyle->borderRightWidth() != newStyle->borderRightWidth()
+            || oldStyle->paddingLeft() != newStyle->paddingLeft()
+            || oldStyle->paddingRight() != newStyle->paddingRight();
+
+    return oldStyle->borderTopWidth() != newStyle->borderTopWidth()
+        || oldStyle->borderBottomWidth() != newStyle->borderBottomWidth()
+        || oldStyle->paddingTop() != newStyle->paddingTop()
+        || oldStyle->paddingBottom() != newStyle->paddingBottom();
 }
 
@@ -315 +321 @@
 #endif
 
-    if (oldStyle && (newStyle->boxSizing() == BORDER_BOX || oldStyle->boxSizing() == BORDER_BOX) && diff == StyleDifferenceLayout
-        && (newStyle->paddingBox() != oldStyle->paddingBox() || borderWidthChanged(oldStyle, newStyle))) {
+    if (oldStyle && diff == StyleDifferenceLayout && borderOrPaddingLogicalWidthChanged(oldStyle, newStyle)) {
         ASSERT(needsLayout());
         for (RenderObject* child = firstChild(); child; child = child->nextSibling())