[webkit-reviews] review denied: [Bug 54623] RTL web content should have left-hand scrollbar. : [Attachment 114018] Patch v7

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 13 15:54:39 PST 2011


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

Attachment 114018: Patch v7
https://bugs.webkit.org/attachment.cgi?id=114018&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
This seems like a lot of copy-paste code.

I'm willing to believe that this *is* the right solution, but I need you to
convince me of that.

Why is this the right abstraction layer?  Can we make this change at a lower
level?	Could we add helper functions like "adjustRectForRTLAwareScrollBar"
which would contain this ifdef and do nothing when this ifdef is disabled?

I'm nto sure what the right way to write this change is.  You are *much* more
familiar with this code than I am, and I trust your judgement!

I do, however need you to convince me, and future readers of this patch, (in
the ChangeLog, ideally) that this is the right way to go.

If you can come up with a solution which involves less copy-paste code, that
would be ideal.

Thanks again for the patch!


More information about the webkit-reviews mailing list