[webkit-reviews] review granted: [Bug 85856] [Chromium] Cannot show the left side of an RTL element with a vertical scrollbar : [Attachment 151640] Patch v10 (Updated description)

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted 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 151640: Patch v10 (Updated description)
https://bugs.webkit.org/attachment.cgi?id=151640&action=review

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


Thanks for finding the spec for this.  Since IE is the only one with a
different behavior and since having clientLeft be to the right of the scrollbar
makes more sense, this looks correct to me.

Can you add some explicit tests for clientLeft (could be a separate patch)?

> LayoutTests/scrollbars/rtl/div-horizontal.html:36
> +var scrollbarWidthRTL = outerRTL.clientLeft > 0 ? outerRTL.clientLeft :
outerRTL.offsetWidth - outerRTL.clientWidth;

Why do we need the outerRTL.clientLeft > 0 check? Won't outerRTL.offsetWidth -
outerRTL.clientWidth always give the width of the scrollbar?

> LayoutTests/scrollbars/rtl/div-vertical.html:36
> +var scrollbarWidthRTL = outerRTL.clientLeft > 0 ? outerRTL.clientLeft :
outerRTL.offsetWidth - outerRTL.clientWidth;

Same question about the clientLeft check.