Title: [143092] trunk
- Revision
- 143092
- Author
- t...@chromium.org
- Date
- 2013-02-15 19:06:19 -0800 (Fri, 15 Feb 2013)
Log 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.
Also convert the needsLayout ASSERT into the if statement. This is because
RenderScrollbarPart can change border widths and not need a layout if the scrollbar
doesn't have a parent. In this case, we don't need to set any children for layout.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (143091 => 143092)
--- trunk/LayoutTests/ChangeLog 2013-02-16 03:06:11 UTC (rev 143091)
+++ trunk/LayoutTests/ChangeLog 2013-02-16 03:06:19 UTC (rev 143092)
@@ -1,3 +1,16 @@
+2013-02-15 Tony Chang <t...@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-15 Simon Fraser <simon.fra...@apple.com>
REGRESSION (r142505?): Crashes in WebCore::ScrollingStateNode::appendChild when using back/forward buttons
Added: trunk/LayoutTests/fast/block/dynamic-padding-border-expected.txt (0 => 143092)
--- trunk/LayoutTests/fast/block/dynamic-padding-border-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/dynamic-padding-border-expected.txt 2013-02-16 03:06:19 UTC (rev 143092)
@@ -0,0 +1,6 @@
+This test passes if there is no red showing.
+
+PASS
+PASS
+PASS
+PASS
Added: trunk/LayoutTests/fast/block/dynamic-padding-border.html (0 => 143092)
--- trunk/LayoutTests/fast/block/dynamic-padding-border.html (rev 0)
+++ trunk/LayoutTests/fast/block/dynamic-padding-border.html 2013-02-16 03:06:19 UTC (rev 143092)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.red {
+ background-color: red;
+}
+.green {
+ background-color: green;
+}
+</style>
+</head>
+<body>
+<p>This test passes if there is no red showing.</p>
+
+<div class="container" style="width: 100px">
+ <div id="test1" class="red" style="padding-left: 50px" data-expected-width="100">
+ <div class="green" style="height: 20px" data-expected-width="100"></div>
+ </div>
+</div>
+
+<div class="container" style="-webkit-writing-mode: vertical-rl; height: 100px">
+ <div id="test2" class="red" style="padding-top: 50px" data-expected-height="100">
+ <div class="green" style="width: 20px" data-expected-height="100"></div>
+ </div>
+</div>
+
+<div class="container" style="width: 100px; -webkit-writing-mode: horizontal-bt;">
+ <div id="test3" class="red" style="border-left: 50px solid red" data-expected-width="100">
+ <div class="green" style="height: 20px" data-expected-width="100"></div>
+ </div>
+</div>
+
+<div class="container" style="-webkit-writing-mode: vertical-lr; height: 100px">
+ <div id="test4" class="red" style="border-top: 50px solid red" data-expected-height="100">
+ <div class="green" style="width: 20px" data-expected-height="100"></div>
+ </div>
+</div>
+
+<script src=""
+<script>
+document.body.offsetLeft;
+document.getElementById("test1").style.paddingLeft = "0";
+document.getElementById("test2").style.paddingTop = "0";
+document.getElementById("test3").style.borderWidth = "0";
+document.getElementById("test4").style.borderWidth = "0";
+checkLayout(".container");
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/fast/table/border-collapsing/cached-change-row-border-width-expected.txt (143091 => 143092)
--- trunk/LayoutTests/fast/table/border-collapsing/cached-change-row-border-width-expected.txt 2013-02-16 03:06:11 UTC (rev 143091)
+++ trunk/LayoutTests/fast/table/border-collapsing/cached-change-row-border-width-expected.txt 2013-02-16 03:06:19 UTC (rev 143092)
@@ -6,6 +6,6 @@
RenderTable {TABLE} at (0,0) size 56x103 [border: (2px solid #0000FF)]
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]
Modified: trunk/Source/WebCore/ChangeLog (143091 => 143092)
--- trunk/Source/WebCore/ChangeLog 2013-02-16 03:06:11 UTC (rev 143091)
+++ trunk/Source/WebCore/ChangeLog 2013-02-16 03:06:19 UTC (rev 143092)
@@ -1,3 +1,27 @@
+2013-02-15 Tony Chang <t...@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.
+
+ Also convert the needsLayout ASSERT into the if statement. This is because
+ RenderScrollbarPart can change border widths and not need a layout if the scrollbar
+ doesn't have a parent. In this case, we don't need to set any children for layout.
+
+ 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-15 Mark Lam <mark....@apple.com>
Split SQLStatement work between the frontend and backend.
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (143091 => 143092)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2013-02-16 03:06:11 UTC (rev 143091)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2013-02-16 03:06:19 UTC (rev 143092)
@@ -232,12 +232,18 @@
RenderBoxModelObject::styleWillChange(diff, newStyle);
}
-static bool borderWidthChanged(const RenderStyle* oldStyle, const RenderStyle* newStyle)
+static bool borderOrPaddingLogicalWidthChanged(const RenderStyle* oldStyle, const RenderStyle* newStyle)
{
- return oldStyle->borderLeftWidth() != newStyle->borderLeftWidth()
- || oldStyle->borderTopWidth() != newStyle->borderTopWidth()
- || oldStyle->borderRightWidth() != newStyle->borderRightWidth()
- || oldStyle->borderBottomWidth() != newStyle->borderBottomWidth();
+ 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();
}
void RenderBox::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
@@ -314,9 +320,7 @@
updateExclusionShapeOutsideInfoAfterStyleChange(style()->shapeOutside(), oldStyle ? oldStyle->shapeOutside() : 0);
#endif
- if (oldStyle && (newStyle->boxSizing() == BORDER_BOX || oldStyle->boxSizing() == BORDER_BOX) && diff == StyleDifferenceLayout
- && (newStyle->paddingBox() != oldStyle->paddingBox() || borderWidthChanged(oldStyle, newStyle))) {
- ASSERT(needsLayout());
+ if (oldStyle && diff == StyleDifferenceLayout && needsLayout() && borderOrPaddingLogicalWidthChanged(oldStyle, newStyle)) {
for (RenderObject* child = firstChild(); child; child = child->nextSibling())
child->setChildNeedsLayout(true, MarkOnlyThis);
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
