Closed Bug 1258851 Opened 4 years ago Closed 4 years ago
Scrollbar dragging with scroll-to-click goes crazy and oscillates between the top and the bottom end of the scrollbar
MozReview Request: Bug 1258851 - Propagate the isScrollbarContainer layer flag to the compositor. r?mattwoodrow
58 bytes, text/x-review-board-request
MozReview Request: Bug 1258851 - Update HitTestingTreeNode::IsScrollbarNode to include the scrollbar track layers. r?rbarker
58 bytes, text/x-review-board-request
STR: 1. On OS X, in your system settings under General, set "Click in the scrollbar to:" to "Jump to the spot that's clicked". 2. If you're using overlay scrollbars, scroll a little on any page to make the scrollbar visible. 3. Click in a part of the scrollbar that is not covered by the scrollbar thumb and keep the mouse button pressed. 4. Quickly drag up and down, quickly enough that you'd get checkerboarding if scrollbar dragging were APZ-ified. Observe how the page wildly jumps up and down during the drag, until you pause your mouse motion and move more slowly. It looks like we get into this state more easily on pages that are slower to draw. attachment 8682494 [details] works well as a testcase. I'm going to look for a regression window.
Regression window is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=102886e9ac63b3fa33a6f1b394aea054abce2dfd&tochange=946ed22cad04431c75ab5093989dfedf1bae5a3e (mozregression couldn't find inbound builds in that range)
Pretty sure "scroll to click" isn't a thing :)
Summary: Scrollbar dragging with scroll-to-click goes crazy and oscillates between the top and the bottom end of the scrollbar → Scrollbar dragging with click-to-scroll goes crazy and oscillates between the top and the bottom end of the scrollbar
"scroll to click" in the sense of "please scroll to the click position" :) https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSliderFrame.cpp#662
Oh, my bad. I read it as "scroll to cause clicking", and thought "scroll" and "click" were accidentally mixed up :)
Summary: Scrollbar dragging with click-to-scroll goes crazy and oscillates between the top and the bottom end of the scrollbar → Scrollbar dragging with scroll-to-click goes crazy and oscillates between the top and the bottom end of the scrollbar
Component: Layout → Panning and Zooming
I can repro on Nightly still, looking into this.
Assignee: nobody → bugmail.mozilla
Bug 1242690 had a bunch of code that skipped untransforming mouse events if they were part of a drag block that started on a scrollbar thumb. The reason for this is that the scrollbar thumb exists "outside" of the scrollable area of the page, and doing an untransform on those mouse events resulted in the oscillation in bug 1244549. That fix was incomplete, because it targetted the scrollthumb only, when in fact it should target the entire scrollbar (including the scrolltrack) for the same reason. Doing that fixes this bug, and I have a WIP for that. However, looking at bug 1259781, I suspect the mouse events for autoscroll also need to skip being untransformed, because it's the same underlying concept. That is, if mouse movements are directly driving a change in the scroll position on the main thread, then those mouse movements are interpreted relative to the screen and not relative to the content, and they shouldn't be untransformed. I'm trying to come up with a cleaner model to handle all this, will think about it a bit more.
This is the WIP. Turns out there's already a "mIsScrollbarContainer" flag on layers which is not being propagated across IPC so I fixed that and used it.
I think the "cleaner model" to handle all this dives into the murky world of resolutions and layout-side untransforms, fixing the TODO at  and bug 1224748 along the way. It's also going to be a big complicated change with a lot of risk. I'll pursue it in bug 1259781 but I'd like to land the fix I came up with for this bug. I think this patch is something we'll need anyway, and it fixes the user-visible bug just fine. Bug 1259781 is much harder to notice/reproduce and so we can take some time to fix it properly with the big complicated change.  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/util/APZCCallbackHelper.cpp?rev=e5232739c23c#461
Comment on attachment 8741150 [details] [diff] [review] WIP Will replace this with cleaned up MozReviews
Attachment #8741150 - Attachment is obsolete: true
Review commit: https://reviewboard.mozilla.org/r/46535/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46535/
Review commit: https://reviewboard.mozilla.org/r/46537/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46537/
Attachment #8741507 - Flags: review?(rbarker) → review+
Comment on attachment 8741507 [details] MozReview Request: Bug 1258851 - Update HitTestingTreeNode::IsScrollbarNode to include the scrollbar track layers. r?rbarker https://reviewboard.mozilla.org/r/46537/#review43149
Comment on attachment 8741506 [details] MozReview Request: Bug 1258851 - Propagate the isScrollbarContainer layer flag to the compositor. r?mattwoodrow https://reviewboard.mozilla.org/r/46535/#review43215
Attachment #8741506 - Flags: review?(matt.woodrow) → review+
4 years ago
No longer depends on: 1266176
3 years ago
Depends on: 1265509
You need to log in before you can comment on or make changes to this bug.