[webkit-reviews] review denied: [Bug 85856] [Chromium] Cannot show the left side of an RTL element with a vertical scrollbar : [Attachment 143519] Patch v2 (used reftests)

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Hironori Bono <hbono at chromium.org>'s
request for review:
Bug 85856: [Chromium] Cannot show the left side of an RTL element with a
vertical scrollbar
https://bugs.webkit.org/show_bug.cgi?id=85856

Attachment 143519: Patch v2 (used reftests)
https://bugs.webkit.org/attachment.cgi?id=143519&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143519&action=review


> Source/WebCore/rendering/RenderBlock.cpp:2869
> +    if (hasOverflowClip()) {
>	   scrolledOffset.move(-scrolledContentOffset());
> +	   if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
> +	       scrolledOffset.move(clientLeft(), 0);

It looks like if we move by clientLeft, we also move by the borderLeft.  Can
you add a test with a left border and make sure it works?

> Source/WebCore/rendering/RenderBox.h:200
> +    LayoutUnit clientLeft() const { return borderLeft() +
(style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ?
verticalScrollbarWidth() : 0); }

Is this correct in vertical writing mode?  The style parameter you check is for
logical left, but clientLift is not logical left.  Can you add a test for
vertical writing mode?	You might want to add the method clientLogicalStart to
RenderBox.