[Webkit-unassigned] [Bug 54623] RTL web content should have left-hand scrollbar.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54623


Hironori Bono <hbono at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #114018|0                           |1
        is obsolete|                            |
 Attachment #121427|                            |review?
               Flag|                            |




--- Comment #38 from Hironori Bono <hbono at chromium.org>  2012-01-06 05:14:03 PST ---
Created an attachment (id=121427)
 --> (https://bugs.webkit.org/attachment.cgi?id=121427&action=review)
Patch v8 (refactored)

Greetings,

Many thanks for your review and comments. Also sorry for my slow update.
This feature s enabled only on Chromium just because Dave told Safari did not need it as written in his comment #12. (Even though I'm not sure if other ports need it, I would like to disable it because it may surprise them to change the behavior of scrollbars without noticing them.)
As you pointed out, my previous change has lots of copy-and-paste code. (This is just because I could not find good function names.) I have created three new functions (rtlAwareCornerX, rtlAwareVerticalScrollbarX, and rtlAwareHorizontalScrollbarX) to simplify this change. (Unfortunately, we need three functions: the first one is for moving a resizer, the second one is for moving a vertical scrollbar, and the third one is for moving a horizontal scrollbar, respectively.) Moving a vertical scrollbar needs to move a horizontal scrollbar and a resizer.

Regards,

Hironori Bono

(In reply to comment #36)
> (From update of attachment 114018 [details])
> 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!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.