[webkit-reviews] review requested: [Bug 54623] RTL web content should have left-hand scrollbar. : [Attachment 88796] A trial change v2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 8 04:08:27 PDT 2011


Hironori Bono <hbono at chromium.org> has asked  for review:
Bug 54623: RTL web content should have left-hand scrollbar.
https://bugs.webkit.org/show_bug.cgi?id=54623

Attachment 88796: A trial change v2
https://bugs.webkit.org/attachment.cgi?id=88796&action=review

------- Additional Comments from Hironori Bono <hbono at chromium.org>
Greetings,

Thank you all for your review and comments. Even though I have updated my
change, I realize this change still needs a lot of cases to discuss about.

(In reply to comment #12)
> I do not believe making this a CSS property is the right way to go.  This
should just be a global policy decision and not controllable by authors.
> I also don't believe we want this change on Mac OS X, since scrollbars stay
on the right hand side even with RTL content in list boxes for example.

Thanks for your comment. (My last change added a CSS property just because I
badly misunderstood a comment from Eric.) I have updated this change to use a
setting. (If this feature is not needed by Mac, we need to move these layout
tests to 'LayoutTests/platform' and use an ENABLE() macro?)
 
> Being inconsistent with iframe scrollbars and top-level document scrollbars
also seems bad to me.

If I can recall comments from a native Hebrew speaker (Aharon), he suggested
not to move the vertical scrollbars of <iframe> or <body> even when their
direction is RTL. (He suggested to move them when the UI language is an RTL
one.) I would like to check it again. By the way, I'm personally wondering if I
shuuld include the code that moves the vertical scrollbars of RenderView
objects to this change because this change already becomes >35KB. It would be
definitely helpful to give me your opinions about it.

> Note also that you'd have to deal with vertical text scrollbars as well.

Yeah. Even though I notice vertical text, this change did not deal with it
because I would like to ask Aharon about it with this change. (Maybe showing to
the top-side?)

(In reply to comment #13)
> Minusing because of the use of a CSS property (and for the other reasons
mentioned in the previous comment).
> Not all platforms are going to want this change, so I think this is better
off as a Setting if you're going to do it.

Thank you. I have updated my change to use it.

> > Source/WebCore/rendering/RenderBlock.cpp:1415
> > +		 if (style()->isShowScrollbarOnLeft())
> 
> This is bad terminology and bad English.

Indeed. (This clearly shows my poor English skill.)

Regards,

Hironori Bono


More information about the webkit-reviews mailing list