Closed Bug 1450099 Opened 2 years ago Closed 2 years ago

Mac scroll bar scrolls on click even when not shown

Categories

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

59 Branch
x86_64
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: agashlin, Assigned: botond)

References

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(2 files)

STEPS TO REPRODUCE:
1. Navigate to a page long enough to scroll vertically
2. Position the mouse cursor in the far right, where the scrollbar would appear
3. Click

EXPECTED RESULTS:
The click passes through to the page.

ACTUAL RESULTS:
The scrollbar appears and the page scrolls. This makes it impossible to click targets within the scrollbar area.

Seems to be regressed by bug 1422070, regression range is:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=aaaa51a8bf64cde9a2a47647902518502d3dd88e&tochange=0cb270dda57624ae29346c6da60292280589f728

Tested on a MacBook Pro (Retina, 13-inch, Early 2015) running High Sierra 10.13.3 (17D102)
Regression in Fx 59.
Keywords: regression
Priority: -- → P2
Hardware: Unspecified → x86_64
Whiteboard: [gfx-noted]
Version: unspecified → 59 Branch
The problem is that APZ is entering the SCROLLBAR_DRAG state too eagerly: it's entered as soon as you click anywhere on the scrollbar track, when it should only be entered when you click on the thumb.
Assignee: nobody → botond
(The purpose of the first patch is to mitigate the fact that the second patch causes FindScrollThumbNode() to be called slightly more often.)
Comment on attachment 8965087 [details]
Bug 1450099 - Add a fast-path to FindScrollThumbNode() for when the drag metrics has not been initialized.

https://reviewboard.mozilla.org/r/233784/#review239440

::: gfx/layers/apz/src/APZCTreeManager.cpp:2558
(Diff revision 1)
>  RefPtr<HitTestingTreeNode>
>  APZCTreeManager::FindScrollThumbNode(const AsyncDragMetrics& aDragMetrics)
>  {
>    RecursiveMutexAutoLock lock(mTreeLock);
>  
> +  if (!aDragMetrics.mDirection) {

Should be safe to move this above the lock acquisition.
Attachment #8965087 - Flags: review?(bugmail) → review+
Comment on attachment 8965088 [details]
Bug 1450099 - Only enter the SCROLLBAR_DRAG state if the scrollbar thumb is clicked.

https://reviewboard.mozilla.org/r/233786/#review239442
Attachment #8965088 - Flags: review?(bugmail) → review+
Markus tried the patches locally and confirmed that they fix the problem.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=574eddb0d54d1ba69fd7d779f57ff19543375d96
Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/16bec207dc41
Add a fast-path to FindScrollThumbNode() for when the drag metrics has not been initialized. r=kats
https://hg.mozilla.org/integration/autoland/rev/2dc5471ebd4a
Only enter the SCROLLBAR_DRAG state if the scrollbar thumb is clicked. r=kats
https://hg.mozilla.org/mozilla-central/rev/16bec207dc41
https://hg.mozilla.org/mozilla-central/rev/2dc5471ebd4a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Looks like we missed our chance to uplift this to Beta for Fx60 still. Is this something we should keep on the radar for possible ESR60 inclusion still?
Flags: needinfo?(botond)
I would lean towards no, as this seems like a minor edge case, but let's ask Adam as well: do you think this bug has a significant impact on user experience?
Flags: needinfo?(botond) → needinfo?(agashlin)
(In reply to Botond Ballo [:botond] from comment #14)
> I would lean towards no, as this seems like a minor edge case, but let's ask
> Adam as well: do you think this bug has a significant impact on user
> experience?

It's literally an edge case, I wouldn't worry too much about it. I only noticed it on one Microsoft doc site that had a button partially overlapping the scrollbar, so I could work around it easily.
Flags: needinfo?(agashlin)
Sounds like it can ride the trains then. Thanks for the extra info.
You need to log in before you can comment on or make changes to this bug.