1355340 - mouse scroll slow, inaccurate, dropping ticks, jumping

Status: VERIFIED FIXED

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

Description

[Edson Freitas]

Attachment: mozregression-output.txt

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.

Comment 1

[Edson Freitas]

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..)

Comment 2

[James Magahern [:buzzert]]

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.

Comment 3

[bintoro]

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.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Perhaps the behaviour changed from one Sierra version to another? What is the full version of 10.12 you are using?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

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?

Comment 6

[bintoro]

Sounds plausible. I went straight to the most recent version, 10.12.4.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Also tagging masayuki since I know Markus is really busy right now.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.)

Comment 9

[Markus Stange [:mstange]]

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.

Comment 10

[Markus Stange [:mstange]]

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 11

[Markus Stange [:mstange]]

Attachment: Bug 1355340 - Don't attempt to accumulate line scrolls for line scroll events.

Review commit: https://reviewboard.mozilla.org/r/132172/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132172/

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a44f35a3a9b
Don't attempt to accumulate line scrolls for line scroll events. r=masayuki

Comment 14

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/4a44f35a3a9b

Comment 15

[bintoro]

Seems good in the latest nightly. Thanks guys!

Comment 16

[Markus Stange [:mstange]]

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

Comment 17

[Markus Stange [:mstange]]

[Tracking Requested - why for this release]: regression

Comment 19

[Marcia Knous [:marcia]]

Tracking for 54/55 and esr52.

Comment 20

[Gerry Chang [:gchang]]

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.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/64901233ef75

Comment 22

[Andrei Vaida [:avaida]]

(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.

Comment 23

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

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.

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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+

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/6b174b41fa44

Comment 27

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I can confirm this issue is fixed, I verified using Firefox 55.2.0esr (2014.06.07) on Mac OS X 10.12.