[webkit-reviews] review denied: [Bug 23556] On RTL pages, horizontal scrollbar is missing : [Attachment 45958] Workaround fix 4

[bugzilla-daemon at webkit.org]

mitz at webkit.org has denied Hironori Bono <hbono at chromium.org>'s request for
review:
Bug 23556: On RTL pages, horizontal scrollbar is missing
https://bugs.webkit.org/show_bug.cgi?id=23556

Attachment 45958: Workaround fix 4
https://bugs.webkit.org/attachment.cgi?id=45958&action=review

------- Additional Comments from mitz at webkit.org
This doesn’t look like the right approach to fixing this bug. There is no
apparent reason why it should involve increasing the size of RenderBlock, and
this seems to account only for block children of blocks in normal flow.
Changing the margin like has all sorts of consequences, e.g. affecting computed
style. I don’t understand why any of this is necessary unless the body element
is RTL. Looking at what Firefox does, it seems to be the only consideration.
This patch also doesn’t address the initial position of the horizontal
scrollbar on a RTL page. It maintains a lot of asymmetry between the RTL and
LTR cases.

I think the correct approach should be similar to a case that already behaves
correctly: RTL blocks with overflow: scroll. The logic for those is in
RenderLayer. They mirror the behavior of LTR blocks with overflow: scroll.
RenderView should behave the same with respect to the direction of the body
element. Currently it has a LTR bias: docWidth() considers the rightmost
position and the right edges of child boxes. layout() adds overflow to the
right but never to the left, etc.

The correct place to fix this is mainly in RenderView, but some changes to
FrameView and Frame will probably be needed too. I suggest studying the
RenderLayer code.