Closed Bug 1354924 Opened 4 years ago Closed 4 years ago
.default .delta _multiplier _y not working properly for external mouse wheel on OS X
59 bytes, text/x-review-board-request
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: Access page about:config, change mousewheel.default.delta_multiplier_y from 100 (default) to a negative value, say -200. Actual results: in page about:config, it works as expected (double scrool movement, invert direction), but in any other pages (say, google: long page), the behavior remains unchanged. Expected results: The behavior must be changed in all pages.
Component: Untriaged → Panning and Zooming
OS: Unspecified → Mac OS X
Product: Firefox → Core
Hardware: Unspecified → x86
I can reproduce this. It's strange, I'm pretty sure this used to work. And the code for picking up the multiplier is still around  so it should still be working. I'll try and debug this when I get a chance, it should be pretty quick.  http://searchfox.org/mozilla-central/rev/eace920a0372051a11d8d275bd9b8f14f3024ecd/gfx/layers/apz/src/APZCTreeManager.cpp#855
Actually, now I can't reproduce it any more. I tried on Fx 52, aurora and nightly. I'm wondering if I imagined reproducing it before. I tried a bunch of different values for mousewheel.default.delta_multiplier_y and they all behave as expected.
Yeah I'm definitely not seeing this. Edson, can you reproduce the problem on a Nightly build? If so, can you try using mozregression to identify when the problem started for you?
All I can say is that, in my apple computer, I can still reproduce the behavior. Restarted, and still there. In my other computer (windows 7 32bits), I can't reproduce. Tried with another mouse in my apple (first try: logitech; second one, microsoft), same results. I tried with another mouse because after upgrading the system to sierra (the last version of apple desktop os), I started to have a lot of troubles with my mouse, a logitech m705, including a poor behavior with the scroll (ridiculously slow). After searching the forums, I discovered many users complaining about the same problem, just after upgrading to sierra. The problem was so serious that logitech had to release a new version of drivers to normalize the situation. But, by my experience and by what others said in the forums, that do not solve all problems. The main problem I'm facing now is that with firefox the scrolling behavior is somewhat wrecked. if I scroll (only a little) slowly, I can almost do a 180 degree rotation without see any page movement. Then, when the page finally starts moving, I can perceive that each displacement needs a bunch of 'ticks' of the wheel. And there is not smooth displacement, but step ones. Really annoying. Currently this is only occurring with firefox. I also use chrome, safari and opera, and in all those three browsers, the new logitech drivers solved the problem. This is how I came to deal with the multiplier_y: trying to adjust/speed up the scrolling movement. Maybe this issue has something to do with the changes apple has made to OSX mouse subsystem. I don't think has something to do with the logitech drivers (that, by the way, came with logitech app named LCC, "logitech control center"), because with the microsoft mouse I can still reproduce the problem. I will try with a Nightly build/mozregression (nice to meet to both) to see what happens.
Ah, thanks! The relevant piece of information was that you are using an external wheel mouse and not a Mac trackpad. I was able to reproduce the problem with an external wheel mouse. It looks like for wheel mice, we pull the multipliers in IAPZCTreeManager, at . However this only happens if we use the WidgetWheelEvent variant of the function, and not the ScrollWheelInput variant. The IAPZCTreeManager was originally added to abstract the GPU process on Windows, and on that platform we always use the WidgetWheelEvent, so the issue never showed up there. On OS X we pass in a ScrollWheelInput at  which goes straight to APZCTreeManager::ReceiveInputEvent(InputData& ...) without going through the IAPZCTreeManager::ReceiveInputEvent(WidgetInputEvent& ...) wrapper. This is definitely a "edge case fell through the cracks" kind of problem, because on OS X the only input events we send using the InputData variants are pan/wheel events, and pan inputs already have their own multiplier handling in APZCTreeManager.cpp.  http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/gfx/layers/apz/public/IAPZCTreeManager.cpp#134  http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/gfx/layers/apz/public/IAPZCTreeManager.cpp#134
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: mousewheel.default.delta_multiplier_y not working properly → mousewheel.default.delta_multiplier_y not working properly for external mouse wheel on OS X
Comment on attachment 8856745 [details] Bug 1354924 - Fix mousewheel multiplier prefs for external mouse devices on OS X. https://reviewboard.mozilla.org/r/128666/#review131122 Does DispatchWheelInputOnControllerThread, which constructs a ScrollWheelInput from a WidgetWheelEvent without applying multipliers , suffer from the same problem?  http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/widget/nsBaseWidget.cpp#1192
Attachment #8856745 - Flags: review?(botond) → review+
A different behavior caused by a non native mouse... Trick one. Also, thanks to mozregression, I was able to identify the build where the annoying scroll behavior started, and as I not the only one experiencing this problem, will open another request to deal with. Thanks.
See Also: → 1355340
(In reply to Botond Ballo [:botond] from comment #7) > Does DispatchWheelInputOnControllerThread, which constructs a > ScrollWheelInput from a WidgetWheelEvent without applying multipliers , > suffer from the same problem? Good catch, it probably does. Although I don't think that code is actually exercised on any platform, at least while layers.async-pan-zoom.separate-event-thread is false. I'll verify.
(In reply to Kartikaya Gupta (email:firstname.lastname@example.org) from comment #9) > (In reply to Botond Ballo [:botond] from comment #7) > > Does DispatchWheelInputOnControllerThread, which constructs a > > ScrollWheelInput from a WidgetWheelEvent without applying multipliers , > > suffer from the same problem? > > Good catch, it probably does. Verified that yes, that is the case. But it was already a problem before, because we can't call EventStateManager::GetUserPrefsForWheelEvent from any thread except the main thread, hence the comment at . I'll update that comment to point to the new location of the other call of IAPZCTreeManager.cpp but other than that I don't think we need to fix that right now, we can deal with it if/when we ever get around to turning on layers.async-pan-zoom.separate-event-thread.  http://searchfox.org/mozilla-central/rev/eace920a0372051a11d8d275bd9b8f14f3024ecd/gfx/layers/apz/src/APZCTreeManager.cpp#849
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/08ca449b5f84 Fix mousewheel multiplier prefs for external mouse devices on OS X. r=botond
Hi Edson, can you check the latest nightly build and verify that this works now? If so I can request uplift for this patch to go into Firefox 54 as well.
By "latest nightly" I really mean a nightly from April 12 which I don't think has been generated yet. Probably will be in a few hours.
Yes, it's working now!
Comment on attachment 8856745 [details] Bug 1354924 - Fix mousewheel multiplier prefs for external mouse devices on OS X. Approval Request Comment [Feature/Bug causing the regression]: bug 1281575 (unconfirmed) [User impact if declined]: mousewheel prefs may hsve no effect on external mice on os x. [Is this code covered by automated tests?]: not really, no. [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: after uplifting, sure. STR in a previous comment. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: not particularly [Why is the change risky/not risky?]: well-understood codepaths [String changes made/needed]: none
Attachment #8856745 - Flags: approval-mozilla-aurora?
Comment on attachment 8856745 [details] Bug 1354924 - Fix mousewheel multiplier prefs for external mouse devices on OS X. Fix a regression related to mousewheel prefs and was verified. Aurora54+.
Attachment #8856745 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Do we want to consider this for ESR52 also?
I don't think it's important enough to warrant uplifting to ESR.
You need to log in before you can comment on or make changes to this bug.