Closed Bug 1424591 Opened 3 years ago Closed 3 years ago
SAP: Scroll bar cannot be dragged while holding left mouse button
12.84 KB, text/html
661 bytes, text/plain
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.
59 bytes, text/x-review-board-request
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
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.
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.
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.
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
This time with a Try push that actually includes the change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7387c7212a765013792804086658aa16796e8a5&selectedJob=151840450
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 email@example.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
You need to log in before you can comment on or make changes to this bug.