Commit 71837dd7 authored by hbono@chromium.org's avatar hbono@chromium.org

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@123572 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 24f2d531
2012-07-24 Hironori Bono <hbono@chromium.org>
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 <mitz@apple.com>
RenderBlock::positionForPoint can fail when the block or its children have a vertical writing mode
......@@ -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;
......
......@@ -11,5 +11,5 @@ layer at (0,0) size 800x600
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)]
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
<!DOCTYPE html>
<html>
<head>
<title>Bug 91756</title>
<script src="../../fast/js/resources/js-test-pre.js"></script>
<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="../../fast/js/resources/js-test-post.js"></script>
</body>
</html>
2012-07-24 Hironori Bono <hbono@chromium.org>
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 <mitz@apple.com>
RenderBlock::positionForPoint can fail when the block or its children have a vertical writing mode
......@@ -412,8 +412,12 @@ int Element::clientLeft()
{
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;
}
......
......@@ -1669,12 +1669,8 @@ void RenderBlock::addOverflowFromPositionedObjects()
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 @@ LayoutUnit RenderBlock::computeStartPositionDeltaForChildAvoidingFloats(const Re
void RenderBlock::determineLogicalLeftPositionForChild(RenderBox* child)
{
LayoutUnit startPosition = borderStart() + paddingStart();
if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
startPosition -= verticalScrollbarWidth();
LayoutUnit totalAvailableLogicalWidth = borderAndPaddingLogicalWidth() + availableLogicalWidth();
// Add in our start margin.
......
......@@ -197,7 +197,7 @@ public:
// 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;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment