Closed
Bug 1235686
Opened 9 years ago
Closed 9 years ago
Mouse scroll speed may not be correct if Synatpcis's or Alps's touchpad and another pointing device are used on an environment
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla46
People
(Reporter: nikpmr, Assigned: masayuki)
References
Details
Attachments
(5 files, 8 obsolete files)
52.40 KB,
image/png
|
Details | |
13.38 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
4.05 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
6.95 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
6.33 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
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
Status: UNCONFIRMED → NEW
status-firefox43:
--- → affected
status-firefox46:
--- → affected
Component: Untriaged → Event Handling
Ever confirmed: true
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Thank you for your attention to this Paul. Hopefully a resolution can be found soon.
Comment 4•9 years ago
|
||
masayuki, have you ever seen this kind of behavior? I need to find a mouse to test this.
Assignee | ||
Comment 5•9 years ago
|
||
(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.
Assignee | ||
Comment 6•9 years ago
|
||
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?
Assignee | ||
Comment 7•9 years ago
|
||
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.
Component: Event Handling → Widget: Win32
Assignee | ||
Comment 8•9 years ago
|
||
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?
Assignee | ||
Comment 9•9 years ago
|
||
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".
Flags: needinfo?(paul.oiegas)
Flags: needinfo?(nikpmr)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Comment 10•9 years ago
|
||
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•9 years ago
|
||
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)
Reporter | ||
Comment 12•9 years ago
|
||
I am using a Synaptics touch pad and Logitech mouse as well on Windows 10.
Flags: needinfo?(nikpmr)
Assignee | ||
Comment 13•9 years ago
|
||
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.
Assignee | ||
Comment 14•9 years ago
|
||
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.
Assignee | ||
Comment 15•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=39480c5de0a0
Comment 16•9 years ago
|
||
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.
Flags: needinfo?(paul.oiegas)
Assignee | ||
Comment 17•9 years ago
|
||
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.
Assignee | ||
Comment 18•9 years ago
|
||
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.
Attachment #8705529 -
Flags: review?(jmathies)
Assignee | ||
Comment 19•9 years ago
|
||
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.
Attachment #8705530 -
Flags: review?(jmathies)
Assignee | ||
Comment 20•9 years ago
|
||
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.
Attachment #8705544 -
Flags: review?(jmathies)
Assignee | ||
Comment 21•9 years ago
|
||
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.
Attachment #8705572 -
Flags: review?(jmathies)
Assignee | ||
Comment 22•9 years ago
|
||
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/masayuki@d-toybox.com-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).
Flags: needinfo?(paul.oiegas)
Flags: needinfo?(nikpmr)
Flags: needinfo?(foobar094)
Comment 23•9 years ago
|
||
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 :)
Flags: needinfo?(paul.oiegas)
Comment 24•9 years ago
|
||
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 ;)
Flags: needinfo?(foobar094)
Assignee | ||
Comment 25•9 years ago
|
||
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.
Attachment #8705529 -
Flags: review?(jmathies) → review-
Assignee | ||
Updated•9 years ago
|
Attachment #8705530 -
Flags: review?(jmathies) → review-
Assignee | ||
Updated•9 years ago
|
Attachment #8705544 -
Flags: review?(jmathies) → review-
Assignee | ||
Updated•9 years ago
|
Attachment #8705572 -
Flags: review?(jmathies) → review-
Assignee | ||
Comment 27•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7012d97373ab
Reporter | ||
Comment 28•9 years ago
|
||
I can also confirm that your build corrects the issue.
Flags: needinfo?(nikpmr)
Assignee | ||
Comment 29•9 years ago
|
||
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.
Attachment #8705529 -
Attachment is obsolete: true
Attachment #8705530 -
Attachment is obsolete: true
Attachment #8705544 -
Attachment is obsolete: true
Attachment #8705572 -
Attachment is obsolete: true
Attachment #8706069 -
Flags: review?(jmathies)
Assignee | ||
Comment 30•9 years ago
|
||
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.
Attachment #8706070 -
Flags: review?(jmathies)
Assignee | ||
Comment 31•9 years ago
|
||
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.
Attachment #8706071 -
Flags: review?(jmathies)
Assignee | ||
Comment 32•9 years ago
|
||
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.
Attachment #8706072 -
Flags: review?(jmathies)
Assignee | ||
Comment 33•9 years ago
|
||
(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.
Assignee | ||
Updated•9 years ago
|
Summary: Scroll speed varies based on what input devices is used first → Mouse scroll speed may not be correct if Synatpcis's or Alps's touchpad and another pointing device are used on an environment
Comment 34•9 years ago
|
||
Curious, could we not have driver specific checks, and just call RefreshCache always (or in other words, not cache)? If not, why not?
Assignee | ||
Comment 35•9 years ago
|
||
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•9 years ago
|
||
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?
Assignee | ||
Comment 37•9 years ago
|
||
(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.)
![]() |
||
Updated•9 years ago
|
Attachment #8706070 -
Flags: review?(jmathies) → review+
![]() |
||
Updated•9 years ago
|
Attachment #8706071 -
Flags: review?(jmathies) → review+
![]() |
||
Updated•9 years ago
|
Attachment #8706072 -
Flags: review?(jmathies) → review+
![]() |
||
Comment 38•9 years ago
|
||
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.
Attachment #8706069 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 39•9 years ago
|
||
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•9 years ago
|
||
bugherder |
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
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Assignee | ||
Comment 41•9 years ago
|
||
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.
Attachment #8706069 -
Attachment is obsolete: true
Attachment #8708268 -
Flags: approval-mozilla-beta?
Attachment #8708268 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 42•9 years ago
|
||
Approval Request Comment See comment 41.
Attachment #8706070 -
Attachment is obsolete: true
Attachment #8708270 -
Flags: approval-mozilla-beta?
Attachment #8708270 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 43•9 years ago
|
||
Approval Request Comment See comment 41.
Attachment #8706071 -
Attachment is obsolete: true
Attachment #8708271 -
Flags: approval-mozilla-beta?
Attachment #8708271 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•9 years ago
|
Attachment #8708271 -
Attachment description: bug1235686-3.diff → part.3 Don't trust system settings cache if SynTP of Synaptics is installed (r=jimm)
Assignee | ||
Comment 44•9 years ago
|
||
Approval Request Comment See comment 41.
Attachment #8706072 -
Attachment is obsolete: true
Attachment #8708272 -
Flags: approval-mozilla-beta?
Attachment #8708272 -
Flags: approval-mozilla-aurora?
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.
Attachment #8708268 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
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.
Attachment #8708270 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
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.
Attachment #8708271 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
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.
Attachment #8708272 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
status-firefox44:
--- → wontfix
Updated•9 years ago
|
status-firefox45:
--- → affected
Comment 50•9 years ago
|
||
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
Flags: needinfo?(paul.oiegas)
Flags: needinfo?(nikpmr)
Reporter | ||
Comment 51•9 years ago
|
||
Using 46 now. Problem has been resolved. Thanks a bunch.
Flags: needinfo?(nikpmr)
Comment 52•9 years ago
|
||
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
Flags: needinfo?(paul.oiegas)
Comment 54•9 years ago
|
||
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!
Attachment #8708268 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Attachment #8708270 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Attachment #8708271 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Attachment #8708272 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 55•9 years ago
|
||
bugherder uplift |
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
You need to log in
before you can comment on or make changes to this bug.
Description
•