SAP: Scroll bar cannot be dragged while holding left mouse button

RESOLVED FIXED in Firefox 59

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: malkiel.hadari, Assigned: botond)

Tracking

({regression, testcase})

55 Branch
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 wontfix, firefox58 wontfix, firefox59 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 attachments)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce:

1.	Open the attached file HTML_and_CSS_only_Sample.html
2.	Using the mouse only, try to drag the scrollbar to scroll the page 


Note: if you use the mouse wheel instead to scroll, then the scrollbar IS moved and the page does scroll.

Issue found on desktop Firefox browser (no such issue in Chrome or IE).



Actual results:

Actual result: The scrollbar is freezes (cannot be dragged) and the page does not scroll


Expected results:

Expected result: The scrollbar is dragged and the page scrolls
Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e0709edc613808cd1073632634e004c10ce16e47&tochange=58f1f2ef3d07643330ea732adcafa2611cb2f92b
Blocks: 1352863
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Panning and Zooming
Ever confirmed: true
Keywords: regression, testcase
Product: Firefox → Core
Version: 57 Branch → 55 Branch
I'm not able to reproduce on Linux, even with slider.snapMultiplier set to 6 (as it is on Windows).

I'll try on a Windows machine next week.
(In reply to Botond Ballo [:botond] from comment #2)
> I'm not able to reproduce on Linux, even with slider.snapMultiplier set to 6
> (as it is on Windows).

Oh, never mind, slider.snapMultiplier is a "Once" pref (requires a restart to take effect). I *can* reproduce on Linux with slider.snapMultiplier set to 6.
Posted file Reduced testcase
Further reduced the testcase a bit. Seems to be related to perspective / transform. Somehow that causes us to compute the distance between the mouse and the scrollbar thumb incorrectly, causing the "mouse is far away from the scrollbar thumb" codepath to misfire.
Note: if you need a quick workaround, setting the pref "slider.snapMultiplier" to 0 should fix it.
Backing out the second patch from bug 1168263 ("Exclude perspective transforms from the transform used to convert from screen coordinates to an APZC's coordinate space") fixes the problem.
Blocks: 1168263
So in the change mentioned in the previous comment, we modified the code that computes the transform from an APZC up to its parent to exclude the CSS transforms of layers which represented perspective transforms.

The reason this is a problem, is that this ancestor transform is used to convert between Screen coordinates and an APZC's ParentLayer coordinates. If we exclude a component of this transform, and use the resulting transform to convert form Screen to ParentLayer coordinates, the result will not *really* be in ParentLayer coordinates. If that result is then compared to something that really is in ParentLayer coordinates (such as quantities stored on the layer itself), that comparison is bogus.

The motivation for excluding perspective transforms is to address scenarios where we have multiple layers that scroll together, and some of them are covered by a perspective transform and some are not. In such cases, including the perspective transform leads to the different layers having different ancestor transforms, which violates APZ constraint (the APZC needs to have a single ancestor transform).

The page in this bug is not like that: here, the APZC in question only has one layer, and the perspective transform is on an ancestor layer. In such cases, excluding the perspective transform is not necessary.

So, the problem can be resolved by limiting the exclusion to perspective transforms which are on the path from layers that scroll together to their common ancestor.
Assignee: nobody → botond
I have an implementation of the solution approach described in comment 7, but it's failing layout/generic/test/test_bug1198135.html:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=feccfcf83df70b34e44d3624a5cfe3cb1189879f&selectedJob=151714632
Curiously, I can't reproduce that test failure locally.
I managed to reproduce the test failure locally by just opening the test page in a browser rather than actually running the test harness, and fix it with a simple tweak to the original patch:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=45b289978ffb22086eb612c8a38794af9cfc6755
Comment on attachment 8937094 [details]
Bug 1424591 - Only exclude a perspective transform from an APZC ancestor transform if it's on the path from layers that scroll together to their common ancestor.

https://reviewboard.mozilla.org/r/207800/#review213688

LGTM, thanks! I like the introduction of the AncestorTransform struct, it makes things overall more readable :)
Attachment #8937094 - Flags: review?(bugmail) → review+
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/26b24e77688e
Only exclude a perspective transform from an APZC ancestor transform if it's on the path from layers that scroll together to their common ancestor. r=kats
https://hg.mozilla.org/mozilla-central/rev/26b24e77688e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Blocks: 1429373
No longer blocks: 1429373
See Also: → 1429373
You need to log in before you can comment on or make changes to this bug.