Created attachment 370010 [details] [diff] [review] patch
Comment on attachment 370010 [details] [diff] [review] patch > if (scrollLeft) >- x = rect.left - (rect.right - rect.left); >+ x = rect.left - rect.width; > else >- x = rect.right + (rect.right - rect.left); >+ x = rect.right + rect.width; This is just cleanup... .width is new in 1.9.1.
Does this work correctly for RTL? _scrollIndex is set depending on the value of _isLTRScrollbox, but scrollByIndex also makes the same adjustment to the passed in index.
Yes, it does. Anything else would be a bug in the current code, i.e. RTL would already have been broken or the two scrollByIndex methods would behave inconsistently.
(In reply to comment #3) > i.e. RTL would already have been broken or the two scrollByIndex methods would > behave inconsistently. Well sure, but the latter is not as severe as the former, so how the behavior is changing is important. There certainly seems to be a bug with your patch applied - I'd like to understand what it is, and understand how it differs from any existing bugs.
(In reply to comment #4) > There certainly seems to be a bug with your patch applied What exactly are you seeing?
If you're using the force rtl extension, make sure to open a new window, since _isLTRScrollbox is cached.
I haven't tested it, but it doesn't make sense for _isLTRScrollbox to be checked twice. Maybe I'm missing something...
_isLTRScrollbox is used again after _elementFromPoint(x), because the code below really wants .leftSibling and .rightSibling rather than .previousSibling and .nextSibling.
Fixed in bug 483552.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.