1235686 - Mouse scroll speed may not be correct if Synatpcis's or Alps's touchpad and another pointing device are used on an environment

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[nikpmr]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151029151421

Steps to reproduce:

Start Firefox, first use the touchpad to scroll down a page, then switch back to the mouse wheel. 


Actual results:

The page scrolls one line at a time. 


Expected results:

The page should scroll multiple lines at a time.

Starting Firefox and then first using the mouse wheel to scroll produces the desired results. It is as if Firefox sets the scroll speed based on which input device it detects first.

Comment 1

[Paul Oiegas [:poiegas], PM for (Ecosystem QA, Services QA, WebCompat QA, Ads QA, and Solo QA teams) PTO till 09/23]

Hi reporter,

I have managed to reproduce this issue on the latest Firefox release (43.0.3) and on the latest Nightly build (46.0a1-20160104030217) on Windows 10 x64. I have tested this using a Dell XPS 12 2 in 1 laptop device. 
I marking this issue as NEW and I'm setting a component for it. If the component is not the right one, please feel free to update it with a more appropriate one. 

Thanks,
Paul

Comment 3

[nikpmr]

Thank you for your attention to this Paul. 
Hopefully a resolution can be found soon.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

masayuki, have you ever seen this kind of behavior?

I need to find a mouse to test this.

Comment 5

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

(In reply to Olli Pettay [:smaug] from comment #4)
> masayuki, have you ever seen this kind of behavior?

Yeah, probably, I can reproduce this. Indeed, I *feel* the scroll speed slower if I scroll a page by touchpad.

My touchpad is Synaptics's touchpad and my mouse is Logitech's MX Master.

Comment 6

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

I can reproduce this bug without restarting Firefox. After you reproduce this bug,

1. Open Settings.
2. Move to Devices -> Mouse and Touchpad
3. Change scroll lines to different value.
4. Restore scroll lines to original value.
5. You can test it again.

According to this STR, we failed to refresh system settings in MouseScrollHandler?

Comment 7

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

When I use touchpad first, SystemParametersInfo(SPI_GETWHEELSCROLLLINES) returns 1. However, when I use mouse first, SystemParametersInfo(SPI_GETWHEELSCROLLLINES) returns 3. However, there is no WM_SETTINGCHANGE message with SPI_SETWHEELSCROLLLINES at changing the device.

Comment 8

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

I confirmed that any message isn't sent to us when I change mouse device. We should stop caching mouse scroll speed at least on Win 10.

Does somebody know this is reproducible on Win 8.1 or earlier?

Comment 9

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

Attachment: screenshot of the tray in the taskbar

Hi, could you tell me which vendor's touchpad are you using?

I guess that in the tray in the taskbar, there is a touchpad's settings like this screenshot. From this, you can check your touchpad's vendor. I think that the name may be one of "Synaptics", "Elantech" or "Alps".

Comment 10

[Ryo Kato [:r_kato]]

I reproduced this bug on x86_32 Windows 7 and the latest Nightly.

I'm using Synaptics TouchPad V7.5 and Toshiba dynabook's mouse (USB wireless type).

Comment 11

[Ryo Kato [:r_kato]]

I also reproduced this bug on the following Firefox

38.5.2 (esr)
43.0.4 (release)
44.0b6 (beta)
45.0a2 (2016-01-07)(aurora)
46.0a1 (2016-01-07)(nightly)

Comment 12

[nikpmr]

I am using a Synaptics touch pad and Logitech mouse as well on Windows 10.

Comment 13

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

Ryo Kato-san and nikpmr, thank you very much for reporting your environment. That's very helpful information.

I'm thinking that we should forget the cache if Synaptics's driver is installed into the environment.

Comment 14

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

I confirmed that I can reproduce same bug with Alps Apoint 10.100.404.105.

Unfortunately, I don't have a PC whose touchpad is Elantech.

Comment 15

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=39480c5de0a0

Comment 16

[Paul Oiegas [:poiegas], PM for (Ecosystem QA, Services QA, WebCompat QA, Ads QA, and Solo QA teams) PTO till 09/23]

Hi Masayuki,

Sorry for the delay, other timezone. :)
Device Dell XPS 12 2 in 1 laptop. And I'm using Dell Touchpad that is manufactured by Synaptics. Driver version is 19.0.15.2.

Comment 17

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

Thank you, Paul.

So, now, all victims are using Synaptics. And I confirmed the Alps's touchpad too. We should not use system settings cache if one of them is installed.

Comment 18

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

Attachment: part.1 MouseScrollHandler should clear system settings cache at handling wheel messages if the pref doesn't allow to cache system settings

We need a new path not to use system settings cache for computing scroll amount.

This patch creates the path and user can disable or forcibly enable it intentionally if our check logic (implemented by following patches) doesn't work well.

Comment 19

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

Attachment: part.2 Don't forget system settings cache at handling wheel messages if the scroll amout values are initialized with prefs

If user or auto mated test overrides the scroll amount with prefs, we can trust the cached scroll amount. So, in such case, we should not use the new path.

Comment 20

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

Attachment: part.3 Don't trust system settings cache if SynTP of Synaptics is installed

If we can retrieve driver version of SynTP (Synaptics's touchpad) from registry, we should mark the system settings cache dirty at handling every WM_MOUSE(H)WHEEL.

Comment 21

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

Attachment: part.4 Don't trust system settings cache if Apoint of Alps is installed

Apoint which is Alps's touchpad also returns different value for calls of SystemParametersInfo(SPI_GETWHEELSCROLLLINES). So, we should do same thing as SynTP for Apoint too.

Comment 22

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

Hi, for checking we have same path in the registry, I'd be happy if you'd test the patched build:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-515ec3691fa987ff6c2c160be72d42f2dc271333/try-win32/firefox-46.0a1.en-US.win32.zip

If this build doesn't fix this bug on your environment, let me know the registry path to the version of SynTP driver. The path must be under HKEY_LOCAL_MACHINE\SOFTWARE\Synaptics(\SynTP).

Comment 23

[Paul Oiegas [:poiegas], PM for (Ecosystem QA, Services QA, WebCompat QA, Ads QA, and Solo QA teams) PTO till 09/23]

Hi,

Just tested your build on the Dell XPS 12 and I cannot reproduce the issue anymore. Both touchapd and mouse scroll functionalities are working fast and smooth now.

Good job :)

Comment 24

[Ryo Kato [:r_kato]]

I've just tested the build and confirmed that this issue cannot be reproduce on x86_32 Windows 7. Both ways of scrolling are working smoothly ;)

Comment 25

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

Comment on attachment 8705529 [details] [diff] [review]
part.1 MouseScrollHandler should clear system settings cache at handling wheel messages if the pref doesn't allow to cache system settings

Oh, I found a regression. The patched build cannot scroll tabbar and tree view smoothly.

Comment 26

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

Paul, Ryo Kato-san, thank you very much for your testing!

Comment 27

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7012d97373ab

Comment 28

[nikpmr]

I can also confirm that your build corrects the issue.

Comment 29

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

Attachment: part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings

Okay, at handling mouse wheel messages, we should update the cache if the cache is disabled by pref and only when the new value is different from old value (i.e., user changed the input device), we should reset the transaction.

Comment 30

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

Attachment: part.2 Don't refresh the cache of system settings at handling wheel messages if the scroll amout values are initialized with prefs

If user or automated test overrides the scroll amount with prefs, we can trust the cached scroll amount. So, in such case, we should not use the new path.

Comment 31

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

Attachment: part.3 Don't trust system settings cache if SynTP of Synaptics is installed

If we can retrieve driver version of SynTP (Synaptics's touchpad) from registry, we should update the system settings cache at handling every WM_MOUSE(H)WHEEL.

Comment 32

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

Attachment: part.4 Don't trust system settings cache if Apoint of Alps is installed

Apoint which is Alps's touchpad also returns different value for calls of SystemParametersInfo(SPI_GETWHEELSCROLLLINES). So, we should do same thing as SynTP for Apoint too.

Comment 33

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

(In reply to nikpmr from comment #28)
> I can also confirm that your build corrects the issue.

Thank you for your test. I guess all synatipcs users must be helped with the patches.

And I found a test who has Elantech's touchpad. However, he said that Elantech's touchpad doesn't cause this bug. I confirmed it with log files which are logged by him.

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

Curious, could we not have driver specific checks, and just call RefreshCache always (or in other words, not cache)? If not, why not?

Comment 35

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

Yeah, we can stop caching system settings in any environments. However, I'm afraid to waste CPU power at handling every wheel message since e.g., Microsoft and Logitech's expensive mouse send much a lot of wheel messages for smoother scrolling.

Anyway, this is hack for illegal touchpad drivers. So, I don't like that normal environment's running cost will increase for supporting illegal devices...

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Well, aren't synaptics and alps the most common touchpads? So we'd end up bypassing the cache
anyway rather often.
Could we detect the case when OS has more than one pointing device?

Comment 37

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

(In reply to Olli Pettay [:smaug] from comment #36)
> Well, aren't synaptics and alps the most common touchpads?

Although, I cannot find the market share. I feel that most notebook users use Synaptics's touchpad. Maybe, next player is Elantech because I saw them on cheaper notebooks/netbooks around 2009, and Alps is now rare (I was surprised my wife's PC has it!) because I don't see it after touchpad supports multi-touch.

> So we'd end up bypassing the cache anyway rather often.

Absolutely, except desktop PC users.

> Could we detect the case when OS has more than one pointing device?

I guess that we can do that with using GetRawInputDeviceList.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms645598%28v=vs.85%29.aspx

And we can detect the change of number of input devices with WM_INPUT_DEVICE_CHANGE.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms645591%28v=vs.85%29.aspx

But I'm not familiar with raw input handling, so, it's much risky to uplift.
(I think that this bug is making a lot of our users feel Firefox slow, so, we should uplift these patches as far as possible.)

Comment 38

[Jim Mathies [:jimm]]

Comment on attachment 8706069 [details] [diff] [review]
part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings

Review of attachment 8706069 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/WinMouseScrollHandler.h
@@ +278,5 @@
> +    // device to another one, the result of SystemParametersInfo() may be
> +    // changed without WM_SETTINGCHANGE message.  For avoiding this trouble,
> +    // we need to modify cache of system settings at every wheel message
> +    // handling if we meet known device whose utility may hook the API.
> +    void WillHandleWheelMessage(bool aIsVertical);

Maybe use something a bit more accurate like "TrustedScrollSettingsDriver()". WillHandleWheelMessage doesn't really seem to match what this method does.

Comment 39

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e701ede86cd8111898ae2435b2dd95ab500284a7
Bug 1235686 part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/47df2f4f6e9321f52e4c797f70d722eb53b8118d
Bug 1235686 part.2 Don't refresh the cache of system settings at handling wheel messages if the scroll amout values are initialized with prefs r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c543377304031c4d7c9a948d4c5bf310d7bdc96
Bug 1235686 part.3 Don't trust system settings cache if SynTP of Synaptics is installed r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/d262f278909c57dfeae0f51067d0be734c8d4f7e
Bug 1235686 part.4 Don't trust system settings cache if Apoint of Alps is installed r=jimm

Comment 40

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e701ede86cd8
https://hg.mozilla.org/mozilla-central/rev/47df2f4f6e93
https://hg.mozilla.org/mozilla-central/rev/3c5433773040
https://hg.mozilla.org/mozilla-central/rev/d262f278909c

Comment 41

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

Attachment: part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings (r=jimm)

Approval Request Comment
[Feature/regressing bug #]: Not a regression, but this fix may make users feel Firefox faster
[User impact if declined]: Users who use Synatpcis's touchpad (according to the Synaptics's data, the market share is 70%) and external mouse, the external mouse's wheel scrolls content very slow (typically, 1/3 of system default settings) if user tries to scroll the touchpad first.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low because these patches just disable system settings of scroll speed cache is disabled if Synaptics's touchpad driver or Alps's touchpad driver is install into the system.
[String/UUID change made/needed]: Nothing.

Comment 42

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

Attachment: part.2 Don't refresh the cache of system settings at handling wheel messages if the scroll amout values are initialized with prefs (r=jimm)

Approval Request Comment
See comment 41.

Comment 43

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

Attachment: part.3 Don't trust system settings cache if SynTP of Synaptics is installed (r=jimm)

Approval Request Comment
See comment 41.

Comment 44

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

Attachment: part.4 Don't trust system settings cache if Apoint of Alps is installed (r=jimm)

Approval Request Comment
See comment 41.

Comment 45

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

Comment on attachment 8708268 [details] [diff] [review]
part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings (r=jimm)

Firefox 44 release enters RC mode this week and the update bar continues to be high. We are only taking fixes for critical (recent) regressions, top crashes and sec-crit fixes. Given that criteria, this issue does not meet the bar for Beta44.

Comment 46

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

Comment on attachment 8708270 [details] [diff] [review]
part.2 Don't refresh the cache of system settings at handling wheel messages if the scroll amout values are initialized with prefs (r=jimm)

Does meet the Beta44 RC uplift criteria, please my last comment for more details.

Comment 47

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

Comment on attachment 8708271 [details] [diff] [review]
part.3 Don't trust system settings cache if SynTP of Synaptics is installed (r=jimm)

Does not meet the Beta44 RC uplift criteria, please my last comment for more details.

Comment 48

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

Comment on attachment 8708272 [details] [diff] [review]
part.4 Don't trust system settings cache if Apoint of Alps is installed (r=jimm)

Does not meet the Beta44 RC uplift criteria, please my last comment for more details.

Comment 49

[Filip Zakrzewski]

Hey Ritu, what about Firefox 45? Is it going to be skipped?

Comment 50

[Sylvestre Ledru [:Sylvestre]]

Guys, could you verify that it is fixed on 46? This is a pretty big patch and I would like to be sure that we are correctly fixing the issue before making a call. Thanks

Comment 51

[nikpmr]

Using 46 now. Problem has been resolved. Thanks a bunch.

Comment 52

[Paul Oiegas [:poiegas], PM for (Ecosystem QA, Services QA, WebCompat QA, Ads QA, and Solo QA teams) PTO till 09/23]

Hi Sylvestre,

I have retested this on latest Nightly build (46.0a1-20160118030338) and the original issue is no longer reproducible. I have also tested with slower and faster scrolling speeds settings for both touchpad and mouse and both the devices are working independently of each other now.

Do you guys think of any more thorough test scenarios on this ? If yes, I would be glad to help to execute them.

Thanks,
Paul

Comment 53

[Sylvestre Ledru [:Sylvestre]]

No, I trust you judgment, if you consider the issue as fixed, I am happy!

Comment 54

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8708268 [details] [diff] [review]
part.1 MouseScrollHandler should refresh the cache of system settings at handling wheel messages if the pref doesn't allow to cache system settings (r=jimm)

So, let's take it!

Comment 55

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f4a0de216259
https://hg.mozilla.org/releases/mozilla-aurora/rev/1183441520d4
https://hg.mozilla.org/releases/mozilla-aurora/rev/1a004023656f
https://hg.mozilla.org/releases/mozilla-aurora/rev/d2e68fee0230