Closed
Bug 1355374
Opened 7 years ago
Closed 7 years ago
Scrollbar doesn't follow mouse cursor if scrollable area is partially off-screen
Categories
(Core :: Panning and Zooming, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | verified |
People
(Reporter: 684sigma, Assigned: botond)
References
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(4 files, 1 obsolete file)
I found a problem in Nightly 55. It doesn't happen in Beta 53. Sometimes when scrolling a scrollable area up, scrollbar moves slower than mouse cursor. Here's how to reproduce it: 1. Open attached page 2. Click "Append" 3. Start dragging the scrollbar starting from its upper point, move mouse to the lower edge of the screen and back Result: Scrollbar moves slower than mouse cursor once it [scrollbar] touches the lower side of the window Expected: Scrollbar should move at the same speed as mouse cursor
Has STR: --- → yes
Keywords: regression
Mozregression-gui generated this regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4cbc57e29e25e8abe783b16915bd74325a040b02&tochange=d7518eef6b8924b4ddad6cdeeae4eed1c6129b26 -> 1324117 – Enable APZ scrollbar dragging on Nightly https://bugzilla.mozilla.org/show_bug.cgi?id=1324117
Blocks: 1324117
Has Regression Range: --- → yes
Updated•7 years ago
|
Component: Untriaged → Panning and Zooming
Product: Firefox → Core
Updated•7 years ago
|
status-firefox52:
--- → unaffected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox55:
--- → affected
Comment 2•7 years ago
|
||
Too late for 53, not a release blocker.
Comment 3•7 years ago
|
||
I think Ovidiu got a little trigger happy here. This doesn't affect anything except 55, and is a bug in the async scrollbar dragging feature which is currently Nightly-only. I can reproduce on Windows, it only happens with apz.drag.enabled set to true.
Blocks: async-scrollbar-drag
status-firefox52:
unaffected → ---
status-firefox53:
wontfix → ---
OS: Unspecified → All
Hardware: Unspecified → All
Updated•7 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Whiteboard: [gfx-noted]
Updated•7 years ago
|
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
Comment 4•7 years ago
|
||
Sorry for that, but I followed what regression range indicated.
Comment 6•7 years ago
|
||
(marking as fix-optional for 55 since this is a nightly-only feature and blocks 1352312, which is the bug to ship by default)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Mike Taylor [:miketaylr] from comment #6) > (marking as fix-optional for 55 since this is a nightly-only feature and > blocks 1352312, which is the bug to ship by default) (I think you meant bug 1211610.)
Comment 8•7 years ago
|
||
(Ah crud, copypasta fail. :()
Assignee | ||
Comment 9•7 years ago
|
||
diagnosis |
The issue here is that LayerMetricsWrapper::GetScrollThumbLength() is using the size of the visible region of the scroll thumb's layer to determine the length of the scroll thumb. When the thumb is partially offscreen, it's visible region can be smaller than the entire thumb, so the calculation goes astray. [1] http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/gfx/layers/LayerMetricsWrapper.h#417
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8860528 [details] Bug 1355374 - Use the AsyncDragMetrics to communicate the scroll thumb length to APZ. https://reviewboard.mozilla.org/r/132540/#review135398
Attachment #8860528 -
Flags: review?(mstange) → review+
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8860529 [details] Bug 1355374 - Remove the no-longer-used LayerMetricsWrapper::GetScrollThumbLength() and HitTestingTreeNode::mScrollThumbLength. https://reviewboard.mozilla.org/r/132542/#review135428
Attachment #8860529 -
Flags: review?(bugmail) → review+
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8860530 [details] Bug 1355374 - Mochitest for dragging partially offscreen scroll thumb. https://reviewboard.mozilla.org/r/132544/#review135430 Looks good, thanks!
Attachment #8860530 -
Flags: review?(bugmail) → review+
Assignee | ||
Comment 16•7 years ago
|
||
And, of course, the test, which was passed for me locally, is failing in automation :/ https://treeherder.mozilla.org/#/jobs?repo=try&revision=32d81acfc9453f2883ac672228f730c1bc58356a
Assignee | ||
Comment 17•7 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #16) > And, of course, the test, which was passed for me locally, is failing in > automation :/ It's passing for me locally even when running the entire directory, so I'll have to push some logging to automation.
Assignee | ||
Comment 18•7 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #17) > It's passing for me locally even when running the entire directory, so I'll > have to push some logging to automation. https://treeherder.mozilla.org/#/jobs?repo=try&revision=04c813ea3d82a3481e8c7641a5bbf733528e3df2
Assignee | ||
Comment 19•7 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #18) > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=04c813ea3d82a3481e8c7641a5bbf733528e3df2 Trying again, this time with the logging code actually compiling (or whatever you call your variable names not having typos in them in JS-land): https://treeherder.mozilla.org/#/jobs?repo=try&revision=b485e9acd5a9f953a1f7f5ee2b2202b719aba7f0
Assignee | ||
Comment 20•7 years ago
|
||
The issue is that, in the test, I try to calculate a ratio of how many pixels of scrolling should occur per pixel of mouse movement during scrollbar dragging as follows: (height of scrolled content) / (height of scroll container) This is wrong; the correct ratio is: (height of scrolled content - height of scroll container) / (height of scroll container - length of scrollbar thumb) (Additionally, if there are scrollbar buttons present, their heights need to be subtracted from the denominator as well.) Since I got both the numerator and denominator wrong, by sheer luck on my local machine the numbers worked out such that the resulting ratio was close enough to the correct ratio to make the test pass; not so on the test machines. Unfortunately, web content is not able to determine the length of the scrollbar thumb, so I believe the correct ratio cannot be computed using standard web APIs. We'd have to use something like the APZ test data mechanism to expose the length of the thumb to the test code. I'm not convinced that this is worth doing; I'm just going to land the fix and abandon the test, though if someone feels strongly that we need this test, or has suggestions for how to test this more easily, I'm open to it.
Assignee | ||
Updated•7 years ago
|
Attachment #8860530 -
Attachment is obsolete: true
Comment 21•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 623d72b7b35d -d 49fbd8da1b0e: rebasing 391515:623d72b7b35d "Bug 1355374 - Use the AsyncDragMetrics to communicate the scroll thumb length to APZ. r=mstange" merging layout/xul/nsSliderFrame.cpp warning: conflicts while merging layout/xul/nsSliderFrame.cpp! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 24•7 years ago
|
||
Pushed by bballo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/687a8e437cda Use the AsyncDragMetrics to communicate the scroll thumb length to APZ. r=mstange https://hg.mozilla.org/integration/autoland/rev/ef85518ce6ba Remove the no-longer-used LayerMetricsWrapper::GetScrollThumbLength() and HitTestingTreeNode::mScrollThumbLength. r=kats
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/687a8e437cda https://hg.mozilla.org/mozilla-central/rev/ef85518ce6ba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 26•7 years ago
|
||
I verified this issue on Mac OS X 10.10 with FF Nightly 55.0a1(2017-04-25) and I can reproduce this issue. Please see the attached video. I also found a new issue that is reproducible if you don't click on "Append", only grab the scrollbar and move it up and down, this bug it's also recorded in the second part of the video.
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 27•7 years ago
|
||
(In reply to ovidiu boca[:Ovidiu] from comment #26) > I verified this issue on Mac OS X 10.10 with FF Nightly 55.0a1(2017-04-25) > and I can reproduce this issue. Please see the attached video. I also found > a new issue that is reproducible if you don't click on "Append", only grab > the scrollbar and move it up and down, this bug it's also recorded in the > second part of the video. Thanks for finding this! I filed bug 1359868 for it.
Comment 28•7 years ago
|
||
I verified this issue on Mac OS X 10.12, Ubuntu 16.04 x64 and Windows 10 x64 with FF 56.0a1(2017-07-03) and I can confirm the fix.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•