- Revision
- 123572
- Author
- [email protected]
- Date
- 2012-07-24 21:51:55 -0700 (Tue, 24 Jul 2012)
Log Message
Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
https://bugs.webkit.org/show_bug.cgi?id=91756
Reviewed by Tony Chang.
Source/WebCore:
My r123067 moves the top-left origin of an RTL element right when its vertical
scrollbar is shown at its left side. (That is, r123067 moves all child objects
in the RTL element right.) This change also increases RenderBox::clientLeft()
at the same time, i.e. it also moves child objects right. Furthermore, my r109512
moves positioned objects in an RTL element right at the same time. This makes
WebKit move objects in an RTL element up to three times by the scrollbar width.
(Moving an absolute object right increases the scrollWidth value and it causes
this bug.) This change removes unnecessary code that moves objects right in my
r109512 and RenderBox::clientLeft().
Test: scrollbars/rtl/div-absolute.html
fast/block/float/026.html
fast/block/float/028.html
fast/overflow/unreachable-overflow-rtl-bug.html
* dom/Element.cpp:
(WebCore::Element::clientLeft): Increase clientLeft value by the width of a vertical scrollbar as written in the CSSOM specification.
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::addOverflowFromPositionedObjects): Removed unnecessary code.
(WebCore::RenderBlock::determineLogicalLeftPositionForChild): Removed unnecessary code.
* rendering/RenderBox.h:
(WebCore::RenderBox::clientLeft): Removed unnecessary code.
LayoutTests:
This change adds a test that compares CSSOM properties of an RTL element which
includes positioned objects with the CSSOM properties of an LTR one. This change
also uses clientLeft properties in offsetX-offsetY.html to remove a hard-coded
value in the test and adds rebaselined results for Windows.
* fast/events/offsetX-offsetY.html: Replaced a hard-coded value 'borderLeft' with clientLeft.
* platform/chromium-linux/fast/block/float/026-expected.png:
* platform/chromium-linux/fast/block/float/028-expected.png:
* platform/chromium-win/fast/block/float/026-expected.png:
* platform/chromium-win/fast/block/float/028-expected.png:
* platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.png:
* platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt:
* scrollbars/rtl/div-absolute-expected.txt: Added.
* scrollbars/rtl/div-absolute.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (123571 => 123572)
--- trunk/LayoutTests/ChangeLog 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/LayoutTests/ChangeLog 2012-07-25 04:51:55 UTC (rev 123572)
@@ -1,3 +1,25 @@
+2012-07-24 Hironori Bono <[email protected]>
+
+ Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
+ https://bugs.webkit.org/show_bug.cgi?id=91756
+
+ Reviewed by Tony Chang.
+
+ This change adds a test that compares CSSOM properties of an RTL element which
+ includes positioned objects with the CSSOM properties of an LTR one. This change
+ also uses clientLeft properties in offsetX-offsetY.html to remove a hard-coded
+ value in the test and adds rebaselined results for Windows.
+
+ * fast/events/offsetX-offsetY.html: Replaced a hard-coded value 'borderLeft' with clientLeft.
+ * platform/chromium-linux/fast/block/float/026-expected.png:
+ * platform/chromium-linux/fast/block/float/028-expected.png:
+ * platform/chromium-win/fast/block/float/026-expected.png:
+ * platform/chromium-win/fast/block/float/028-expected.png:
+ * platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.png:
+ * platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt:
+ * scrollbars/rtl/div-absolute-expected.txt: Added.
+ * scrollbars/rtl/div-absolute.html: Added.
+
2012-07-24 Dan Bernstein <[email protected]>
RenderBlock::positionForPoint can fail when the block or its children have a vertical writing mode
Modified: trunk/LayoutTests/fast/events/offsetX-offsetY.html (123571 => 123572)
--- trunk/LayoutTests/fast/events/offsetX-offsetY.html 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/LayoutTests/fast/events/offsetX-offsetY.html 2012-07-25 04:51:55 UTC (rev 123572)
@@ -76,8 +76,8 @@
var content = document.getElementById('overflow-contents');
positions = sumPositions(content);
var inside = document.getElementById('inside-overflow');
- var borderLeft = 2;
- clientX = positions.offsetLeft + borderLeft + content.clientWidth - inside.clientWidth - window.scrollX + offsetX;
+ var overflow = document.getElementById('overflow');
+ clientX = positions.offsetLeft + overflow.clientLeft + content.clientWidth - inside.clientWidth - window.scrollX + offsetX;
dispatchEvent(clientX, clientY, 'inside-overflow', offsetX, offsetY);
offsetX = 11;
Modified: trunk/LayoutTests/platform/chromium-linux/fast/block/float/026-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-linux/fast/block/float/028-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-win/fast/block/float/026-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-win/fast/block/float/028-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt (123571 => 123572)
--- trunk/LayoutTests/platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/LayoutTests/platform/chromium-win/fast/overflow/unreachable-overflow-rtl-bug-expected.txt 2012-07-25 04:51:55 UTC (rev 123572)
@@ -11,5 +11,5 @@
text run at (0,0) width 31: "RTL:"
layer at (8,28) size 106x106 clip at (11,31) size 85x85 scrollWidth 220 scrollHeight 270
RenderBlock (relative positioned) {DIV} at (0,20) size 106x106 [border: (3px solid #000000)]
-layer at (8,154) size 106x106 clip at (26,157) size 85x85 scrollX 150 scrollWidth 235 scrollHeight 270
+layer at (8,154) size 106x106 clip at (26,157) size 85x85 scrollX 135 scrollWidth 220 scrollHeight 270
RenderBlock (relative positioned) {DIV} at (0,146) size 106x106 [border: (3px solid #000000)]
Added: trunk/LayoutTests/scrollbars/rtl/div-absolute-expected.txt (0 => 123572)
--- trunk/LayoutTests/scrollbars/rtl/div-absolute-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollbars/rtl/div-absolute-expected.txt 2012-07-25 04:51:55 UTC (rev 123572)
@@ -0,0 +1,18 @@
+Test if the widths of RTL elements are the same as the widths of the LTR elements when they include absolutely-positioned children.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Verify the widths of the outer RTL element are the same as the widths of the outer LTR element.
+PASS outerLTR.offsetWidth == outerRTL.offsetWidth is true
+PASS outerLTR.clientWidth == outerRTL.clientWidth is true
+PASS outerLTR.scrollWidth == outerRTL.scrollWidth is true
+Verify the widths of the inner RTL element are the same as the widths of the inner LTR element.
+PASS innerLTR.offsetWidth == innerRTL.offsetWidth is true
+PASS innerLTR.clientWidth == innerRTL.clientWidth is true
+PASS innerLTR.scrollWidth == innerRTL.scrollWidth is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+foo
+foo
Added: trunk/LayoutTests/scrollbars/rtl/div-absolute.html (0 => 123572)
--- trunk/LayoutTests/scrollbars/rtl/div-absolute.html (rev 0)
+++ trunk/LayoutTests/scrollbars/rtl/div-absolute.html 2012-07-25 04:51:55 UTC (rev 123572)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Bug 91756</title>
+<script src=""
+<style>
+div.outer { overflow: auto; width: 100px; position: relative; height: 100px; border: solid; }
+div.inner { position: absolute; top: 250px; }
+</style>
+</head>
+<body>
+<div id="outerLTR" class="outer"><div id="innerLTR" class="inner" style="left: 200px;">foo</div></div>
+<div id="outerRTL" class="outer" style="direction: rtl;"><div id="innerRTL" class="inner" style="right: 200px;">foo</div>
+</div>
+<script type="text/_javascript_">
+description('Test if the widths of RTL elements are the same as the widths of the LTR elements when they include absolutely-positioned children.');
+
+debug('Verify the widths of the outer RTL element are the same as the widths of the outer LTR element.');
+var outerLTR = document.getElementById('outerLTR');
+var outerRTL = document.getElementById('outerRTL');
+shouldBeTrue('outerLTR.offsetWidth == outerRTL.offsetWidth');
+shouldBeTrue('outerLTR.clientWidth == outerRTL.clientWidth');
+shouldBeTrue('outerLTR.scrollWidth == outerRTL.scrollWidth');
+
+debug('Verify the widths of the inner RTL element are the same as the widths of the inner LTR element.');
+var innerLTR = document.getElementById('innerLTR');
+var innerRTL = document.getElementById('innerRTL');
+shouldBeTrue('innerLTR.offsetWidth == innerRTL.offsetWidth');
+shouldBeTrue('innerLTR.clientWidth == innerRTL.clientWidth');
+shouldBeTrue('innerLTR.scrollWidth == innerRTL.scrollWidth');
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (123571 => 123572)
--- trunk/Source/WebCore/ChangeLog 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/Source/WebCore/ChangeLog 2012-07-25 04:51:55 UTC (rev 123572)
@@ -1,3 +1,33 @@
+2012-07-24 Hironori Bono <[email protected]>
+
+ Avoid moving child objects multiple times when vertical scrollbar are shown at the left side.
+ https://bugs.webkit.org/show_bug.cgi?id=91756
+
+ Reviewed by Tony Chang.
+
+ My r123067 moves the top-left origin of an RTL element right when its vertical
+ scrollbar is shown at its left side. (That is, r123067 moves all child objects
+ in the RTL element right.) This change also increases RenderBox::clientLeft()
+ at the same time, i.e. it also moves child objects right. Furthermore, my r109512
+ moves positioned objects in an RTL element right at the same time. This makes
+ WebKit move objects in an RTL element up to three times by the scrollbar width.
+ (Moving an absolute object right increases the scrollWidth value and it causes
+ this bug.) This change removes unnecessary code that moves objects right in my
+ r109512 and RenderBox::clientLeft().
+
+ Test: scrollbars/rtl/div-absolute.html
+ fast/block/float/026.html
+ fast/block/float/028.html
+ fast/overflow/unreachable-overflow-rtl-bug.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::clientLeft): Increase clientLeft value by the width of a vertical scrollbar as written in the CSSOM specification.
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::addOverflowFromPositionedObjects): Removed unnecessary code.
+ (WebCore::RenderBlock::determineLogicalLeftPositionForChild): Removed unnecessary code.
+ * rendering/RenderBox.h:
+ (WebCore::RenderBox::clientLeft): Removed unnecessary code.
+
2012-07-24 Dan Bernstein <[email protected]>
RenderBlock::positionForPoint can fail when the block or its children have a vertical writing mode
Modified: trunk/Source/WebCore/dom/Element.cpp (123571 => 123572)
--- trunk/Source/WebCore/dom/Element.cpp 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-07-25 04:51:55 UTC (rev 123572)
@@ -412,8 +412,12 @@
{
document()->updateLayoutIgnorePendingStylesheets();
- if (RenderBox* renderer = renderBox())
- return adjustForAbsoluteZoom(roundToInt(renderer->clientLeft()), renderer);
+ if (RenderBox* renderer = renderBox()) {
+ LayoutUnit clientLeft = renderer->clientLeft();
+ if (renderer->style() && renderer->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
+ clientLeft += renderer->verticalScrollbarWidth();
+ return adjustForAbsoluteZoom(roundToInt(clientLeft), renderer);
+ }
return 0;
}
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (123571 => 123572)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-07-25 04:51:55 UTC (rev 123572)
@@ -1669,12 +1669,8 @@
positionedObject = *it;
// Fixed positioned elements don't contribute to layout overflow, since they don't scroll with the content.
- if (positionedObject->style()->position() != FixedPosition) {
- int x = positionedObject->x();
- if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
- x -= verticalScrollbarWidth();
- addOverflowFromChild(positionedObject, IntSize(x, positionedObject->y()));
- }
+ if (positionedObject->style()->position() != FixedPosition)
+ addOverflowFromChild(positionedObject, IntSize(positionedObject->x(), positionedObject->y()));
}
}
@@ -2208,8 +2204,6 @@
void RenderBlock::determineLogicalLeftPositionForChild(RenderBox* child)
{
LayoutUnit startPosition = borderStart() + paddingStart();
- if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
- startPosition -= verticalScrollbarWidth();
LayoutUnit totalAvailableLogicalWidth = borderAndPaddingLogicalWidth() + availableLogicalWidth();
// Add in our start margin.
Modified: trunk/Source/WebCore/rendering/RenderBox.h (123571 => 123572)
--- trunk/Source/WebCore/rendering/RenderBox.h 2012-07-25 04:42:44 UTC (rev 123571)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2012-07-25 04:51:55 UTC (rev 123572)
@@ -197,7 +197,7 @@
// More IE extensions. clientWidth and clientHeight represent the interior of an object
// excluding border and scrollbar. clientLeft/Top are just the borderLeftWidth and borderTopWidth.
- LayoutUnit clientLeft() const { return borderLeft() + (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? verticalScrollbarWidth() : 0); }
+ LayoutUnit clientLeft() const { return borderLeft(); }
LayoutUnit clientTop() const { return borderTop(); }
LayoutUnit clientWidth() const;
LayoutUnit clientHeight() const;
