Closed Bug 1355340 Opened 8 years ago Closed 8 years ago

mouse scroll slow, inaccurate, dropping ticks, jumping

Categories

(Core :: Widget: Cocoa, defect, P2)

52 Branch
All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 54+ verified
firefox53 --- wontfix
firefox54 + verified
firefox55 + verified

People

(Reporter: edson.irm, Assigned: mstange)

References

Details

(Keywords: regression, Whiteboard: tpi:+)

Attachments

(2 files)

Attached file mozregression-output.txt (deleted) —
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Firefox/52.0 Build ID: 20170323105023 Steps to reproduce: Scrolling mouse wheel very slowly, to highlight the issue. Actual results: The page won't move. Still scrolling, sometimes beyond 180 degree, and the the page still don't move. Then, the page starts "jumping". Not smoothly, jumping. And even so, I can still perceive the browser losing a handful of clicks/ticks of the wheel betwheen the jumps. Very anoying. Expected results: The scroll should perform smoothly. Using mozregression, I was able to track down the issue to revision d72a1ec0, see attachment. In the immediately preceding revision, the scroll behavior is pleasant. Also, I believe I have found others complaining about the same problem. See: https://www.reddit.com/r/firefox/comments/63cd0m/how_to_get_firefox_to_scroll_like_safarichrome/ In common with them, I use a logitech mouse. Once this was relevant to solve bug 1354924 (that I opened yesterday), I'm also notifying this.
Blocks: 1292201
Component: Untriaged → Widget: Cocoa
Keywords: regression
OS: Unspecified → Mac OS X
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Whiteboard: tpi:+
A little warning: mozregression --launch 545b89141c833cce95e058017d2177e179be4e27 mozregression --launch d72a1ec07543e7663b982da61a9b8735e66981bf NOT getting the requested changesets (at least on my machine). See bug 1356086. (third bug, I'm starting feeling unlucky these days..)
I see this comment in nsChildView.mm // Pre-10.12, deltaX/deltaY had the accumulation behavior that we want, and // it worked more reliably than doing it on our own, so use it on pre-10.12 // versions. For example, with a traditional USB mouse, the first wheel // "tick" would always senda line scroll of at least one line, but with our // own accumulation you sometimes need to do multiple wheel ticks before one // line has been accumulated. Reverting the change for the condition for OnSierraOrLater (i.e., commenting out the accumulator code) gets the behavior that I want. Does anybody know why this change was made? NSEvents on Sierra still have floating point scroll deltas (~0.100), so I'm not sure what changed in Sierra.
Also using a Logitech mouse here. After upgrading to Sierra, the scroll wheel stopped doing anything useful in FF. Scrolling would begin once I was past 7–10 ticks or so. Ended up purchasing a separate helper app to control the scroll wheel, but it's still nowhere as good as it used to be. I wish I had known it was Firefox whose behavior had changed. How can we get this fixed? I too would like to see an answer to comment 2. Let me know if I can help out by testing a potential resolution.
Perhaps the behaviour changed from one Sierra version to another? What is the full version of 10.12 you are using?
Note bug 1292201 comment 4 where Markus filed a bug against Apple for the behaviour they had in early Sierra beta builds. The code quoted in comment 2 appears to be a workaround for that bug. Presumably Apple fixed the bug in the latest Sierra release and so now we can remove the workaround. I'm not running Sierra so I can't confirm this. Markus, are you still on an old Sierra version - and if so, can you update to see if this is the case?
Flags: needinfo?(mstange)
Sounds plausible. I went straight to the most recent version, 10.12.4.
Also tagging masayuki since I know Markus is really busy right now.
Flags: needinfo?(masayuki)
Yep. But I also have some P1 bugs and some security bugs. (This is marked as P2, I think that it's serious for some users, though.)
Flags: needinfo?(masayuki)
I'll try to look into this today. I think when I wrote the original patch I was only looking at what touchpads do, and not what USB mice with "ticking" scroll wheels do.
Flags: needinfo?(mstange)
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Looks like I even knew about this problem at the time, see comment 2. I don't know why I didn't investigate this more closely before I landed the patch :( And the fix is really easy: Restrict the accumulation to pixel scroll events, and don't do it for line scroll wheel tick events.
Comment on attachment 8860140 [details] Bug 1355340 - Don't attempt to accumulate line scrolls for line scroll events. https://reviewboard.mozilla.org/r/132172/#review135158 Nice. Can we uplift this to all branches? (including ESR52)
Attachment #8860140 - Flags: review?(masayuki) → review+
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/4a44f35a3a9b Don't attempt to accumulate line scrolls for line scroll events. r=masayuki
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Seems good in the latest nightly. Thanks guys!
Comment on attachment 8860140 [details] Bug 1355340 - Don't attempt to accumulate line scrolls for line scroll events. I think we should uplift this everywhere we can. It's as risk-free a patch as you can get, and it fixes a frustrating regression. Approval Request Comment [Feature/Bug causing the regression]: bug 1292201 [User impact if declined]: Frustratingly slow and erratic scrolling for users with tick-based scroll wheel mice on macOS 10.12 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: extremely simple patch [String changes made/needed]: none [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: Frustratingly slow and erratic scrolling for users with tick-based scroll wheel mice on macOS 10.12 Fix Landed on Version: 55 Risk to taking this patch (and alternatives if risky): virtually zero String or UUID changes made by this patch: none
Attachment #8860140 - Flags: approval-mozilla-esr52?
Attachment #8860140 - Flags: approval-mozilla-beta?
Attachment #8860140 - Flags: approval-mozilla-aurora?
[Tracking Requested - why for this release]: regression
Hardware: Unspecified → All
Tracking for 54/55 and esr52.
Comment on attachment 8860140 [details] Bug 1355340 - Don't attempt to accumulate line scrolls for line scroll events. Fix a performance issue when scrolling wheel mice on macOS 10.12. Beta54+. Should be in 54 beta 3.
Attachment #8860140 - Flags: approval-mozilla-beta?
Attachment #8860140 - Flags: approval-mozilla-beta+
Attachment #8860140 - Flags: approval-mozilla-aurora?
Attachment #8860140 - Flags: approval-mozilla-aurora-
(In reply to Markus Stange [:mstange] from comment #16) > [Is this code covered by automated tests?]: no > [Has the fix been verified in Nightly?]: yes > [Needs manual test from QE? If yes, steps to reproduce]: no I think we should at least spot-check this, especially since it affects user experience.
Flags: qe-verify+
I have reproduced this issue using Firefox 55.0a1 (2017.04.10) on Mac OS 10.12. I can confirm this issue is fixed, I verified using Firefox 54.0b4 and 55.0a1 on Mac OS X 10.12.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment on attachment 8860140 [details] Bug 1355340 - Don't attempt to accumulate line scrolls for line scroll events. Poor perf with scrolling on Sierra for some users, low risk patch, fix has been verified, ESR52.2+
Attachment #8860140 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
I can confirm this issue is fixed, I verified using Firefox 55.2.0esr (2014.06.07) on Mac OS X 10.12.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: