1378247 - [RTL] Scrollbar moves in opposite direction with horizontal scrolling

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[blinky]

Steps to reproduce:

1. Open https://onedrive.live.com/download?cid=F96BA52A2AF70D03&resid=F96BA52A2AF70D03!1476&authkey=APfKZqnv1yfw-RE.
2. Go to the scroll box.
3. Drag the scrollbar left.


Actual results:

Scrollbar moves in opposite direction with horizontal scrolling.

See https://www.youtube.com/watch?v=RQ7jw_3S_mY

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4cbc57e29e25e8abe783b16915bd74325a040b02&tochange=d7518eef6b8924b4ddad6cdeeae4eed1c6129b26

Regressed by: bug 1324117

Comment 2

[blinky]

Pushlog with enabled apz.drag.enabled = true;
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bd2089fe075e09bb3cbee1535e8ae960598f65c5&tochange=b72ab182dd554343b504a771e535214de53b5d6f

Regressed by: bug 1273137

Comment 3

[Alice0775 White]

[Tracking Requested - why for this release]:

Comment 4

[Kartikaya Gupta (email:[e-mail])]

I can look into this.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1378247 - Properly handle async scrollbar-dragging of RTL scrollframes.

For RTL scrollframes the scrollable rect can extend into the negative-x coordinate
space as the user can scroll leftwards from the "zero" position. The code was
assuming a zero minimum scroll position in a couple of places which broke down
on RTL scrollframes.

Review commit: https://reviewboard.mozilla.org/r/154576/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154576/

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Test page linked in comment 0

For posterity, attaching the test page as an attachment to the bug.

Comment 7

[Botond Ballo [:botond]]

Comment on attachment 8883669 [details]
Bug 1378247 - Properly handle async scrollbar-dragging of RTL scrollframes.

https://reviewboard.mozilla.org/r/154576/#review159686

Thanks for looking at this!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9aed48eb292
Properly handle async scrollbar-dragging of RTL scrollframes. r=botond

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9aed48eb292

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Can you confirm the issue is fixed for you in today's nightly? Thanks!

Comment 11

[blinky]

I confirm this bug has been fixed in the latest nightly build.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8883669 [details]
Bug 1378247 - Properly handle async scrollbar-dragging of RTL scrollframes.

Approval Request Comment
[Feature/Bug causing the regression]: Async scrollbar dragging
[User impact if declined]: scrollbar dragging on RTL scrollframes doesn't work properly
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no, already verified
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not really
[Why is the change risky/not risky?]: small code change to remove an assumption about valid scroll positions
[String changes made/needed]: none

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 8883669 [details]
Bug 1378247 - Properly handle async scrollbar-dragging of RTL scrollframes.

fix for apz with rtl, verified in nightly, beta55+

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/37354bad79e5

Comment 15

[Andrei Vaida [:avaida]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> [Is this code covered by automated tests?]: no
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: no, already verified

Setting qe-verify- based on Kartikaya's assessment on manual testing needs.