Last Comment Bug 829952 - Scrolling using some high-resolution-scroll aware touchpads feels slow
: Scrolling using some high-resolution-scroll aware touchpads feels slow
Status: RESOLVED FIXED
[snappy]
: regression
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: x86 Windows 7
: -- normal with 1 vote (vote)
: mozilla22
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
:
:
Mentors:
Depends on:
Blocks: 719320
  Show dependency treegraph
 
Reported: 2013-01-12 07:21 PST by Olli Pettay [:smaug] (reviewing overload)
Modified: 2013-12-27 14:20 PST (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
-
wontfix
+
wontfix
fixed
22+


Attachments
testcase (2.16 KB, text/html)
2013-02-20 03:38 PST, Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
no flags Details
log (638.51 KB, patch)
2013-02-20 03:40 PST, Olli Pettay [:smaug] (reviewing overload)
no flags Details | Diff | Splinter Review
Testcase #2 (cross browser + total distance) (3.08 KB, text/html)
2013-02-24 21:28 PST, Avi Halachmi (:avih)
no flags Details
Testcase #2 (cross browser + total distance): Slight improvement (3.17 KB, text/html)
2013-02-24 22:56 PST, Avi Halachmi (:avih)
no flags Details
Patch (16.50 KB, patch)
2013-03-14 01:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
no flags Details | Diff | Splinter Review
Patch (16.96 KB, patch)
2013-03-14 02:47 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
bugs: review+
avihpit: feedback+
Details | Diff | Splinter Review
Patch (16.94 KB, patch)
2013-03-14 18:18 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
roc: review+
roc: superreview+
Details | Diff | Splinter Review
Patch for Aurora (17.01 KB, patch)
2013-03-19 08:25 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28))
bajaj.bhavana: approval‑mozilla‑aurora-
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] (reviewing overload) 2013-01-12 07:21:59 PST
I tested Nightly on various Windows laptops (in a shop), and at least on some of them scrolling using *touchpad* felt slow. Could we do UX testing, and perhaps change the scrolling settings based on drivers or whatever is causing the slowness.
(This is *not* performance problem, but ux problem.)

Normally I don't use Windows, so my experience is close to first-time user.
Comment 1 Asa Dotzler [:asa] 2013-01-12 16:56:45 PST
There are several major trackpad/touchpad manufacturers and various drivers/config defaults that come with new PCs. Unlike the old simple trackpads, the devices shipping on Windows notebooks for the last couple of years have been larger and support multi-touch. Unfortunately, each vendor has a different idea about the various gestures. Two finger scrolling, being the most commonly encountered multi-touch gesture for most Windows notebook users, is somewhat inconsistent and it's certainly the first one we should tackle.

With several different hardware devices available from each of several different vendors, and no doubt with different driver versions and user options for most of them, how do we tackle the most important targets first? We'd need an inventory and some idea about popularity of the various combinations. Maybe by way of sales data from the major trackpad vendors, we could get some idea of which are most popular?

Is there any way to get this information directly from our users so we don't have to go to third parties or guess?

If we find that some significant populations of Firefox users are getting a scrolling behavior that feels much slower than others, this is probably snappy worthy.
Comment 2 Jared Wein [:jaws] (please needinfo? me) 2013-01-14 09:32:20 PST
toolkit.scrollbox.verticalScrollDistance=3 is the pref in about:config that determines how far the scrolling is moved, in combination with whatever the touchpad/mouse drivers have for how many lines to scroll for each scroll request.

We could increase the verticalScrollDistance pref if we feel that it will be a positive improvement.
Comment 3 Patrick Finch 2013-01-14 11:56:30 PST
(In reply to Asa Dotzler [:asa] from comment #1)
> Is there any way to get this information directly from our users so we don't
> have to go to third parties or guess?

I don't think that any survey we do would have the geographic scale to be representative, but I will verify with metrics.

I think we'd have to see what vendor detail is available - that might be expensive.  I'll find out.
Comment 4 Olli Pettay [:smaug] (reviewing overload) 2013-01-29 11:17:48 PST
Seems like most of Asus laptops for example suffer from this.
Changing mousewheel.delta.delta_multiplier_y from 100 to 250 gives more reasonable scrolling.
Comment 5 Olli Pettay [:smaug] (reviewing overload) 2013-01-29 13:52:07 PST
On an Asus laptop 
mousewheel.delta.delta_multiplier_y 250
mousewheel.min_line_scroll_amount 2
mousewheel.acceleration.factor 5
made scrolling feel significantly better.

But some UX folks should investigate this issue too.
Comment 6 Asa Dotzler [:asa] 2013-01-29 13:57:29 PST
(In reply to Olli Pettay [:smaug] from comment #5)
> On an Asus laptop 
> mousewheel.delta.delta_multiplier_y 250
> mousewheel.min_line_scroll_amount 2
> mousewheel.acceleration.factor 5
> made scrolling feel significantly better.
> 
> But some UX folks should investigate this issue too.

Which trackpad hardware and driver version do you have? I'd like to compare with a couple of Asus notebooks I have.
Comment 7 Olli Pettay [:smaug] (reviewing overload) 2013-01-29 14:05:28 PST
Asus PS/2 Port ClickPad
Driver v. 1.0.0.125
Comment 8 Olli Pettay [:smaug] (reviewing overload) 2013-02-16 15:48:25 PST
There seem to be some reports about this issue elsewhere and the solution people take is
"use some other browser".

I know very little about Windows widget level code, but could we somehow detect different kinds
of touchpads and use different default configs for some of them?
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-17 07:42:41 PST
Historically, mouse driver is implemented hacky because Windows added new feature a couple of times and applications need to handle the new feature themselves. Actually, some mouse drivers change the native message by focused window class or active process name. So, we may not have any way to fix this bug. But anyway, we need the exact message log of such environment.

You can get the log with following envs:

NSPR_LOG_FILE=(path to log file)
NSPR_LOG_MODULES=MouseScrollHandlerWidgets:1
Comment 10 Olli Pettay [:smaug] (reviewing overload) 2013-02-18 13:14:58 PST
My friend just got Samsung ultrabook 5. I believe that is rather popular laptop.
Scrolling is horrible.
Settings from comment 5 make scrolling quite nice.
Comment 11 Asa Dotzler [:asa] 2013-02-19 11:29:04 PST
There are only a couple major trackpad vendors in the Windows ecosystem. I imagine the driver combinations make it a pretty big matrix but there may be common defaults across the hardware or possibly across major versions of drivers. 

Alternatively, is there some simple measurement we could do client side to use to set the value, perhaps during install time or shortly after first run? 

Also, what do other browsers do? Can someone look at Chrome and IE and see if they're doing a better job here?
Comment 12 Olli Pettay [:smaug] (reviewing overload) 2013-02-19 11:43:32 PST
Settings from comment 5 seem to give pretty much the same behavior what IE has.
Comment 13 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-19 17:31:57 PST
(In reply to Asa Dotzler [:asa] from comment #11)
> Alternatively, is there some simple measurement we could do client side to
> use to set the value, perhaps during install time or shortly after first
> run? 

Users can switch pointing device anytime. So, we need to check the strange driver at every native event.

> Also, what do other browsers do? Can someone look at Chrome and IE and see
> if they're doing a better job here?

Chrome does always twice faster scroll on Windows. That was discussed for a couple of years ago. Then, we also use twice faster scroll only when both registry values (for vertical and horizontal) of scroll amount are not customized.

However, there is a possible thing. That is, the driver doesn't send strange value for other browsers. I suspect this. Therefore, I'd like to see the log mentioned in comment 9.
Comment 14 Olli Pettay [:smaug] (reviewing overload) 2013-02-20 03:32:34 PST
Trying to still figure out how to create the logs mentioned in comment 9.

I compared scrolling of www.hs.fi from bottom to top. On IE and Chrome it took about 14
"full touchpad scrolls", on Nightly 20. (Using Asus U500vz/Win8)
Comment 15 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-20 03:38:33 PST
Created attachment 715960 [details]
testcase

Or, you can check wheel event's delta values and delta mode. If the delta mode is DOM_DELTA_PIXEL, it uses gesture event.
Comment 16 Olli Pettay [:smaug] (reviewing overload) 2013-02-20 03:40:01 PST
Created attachment 715962 [details] [diff] [review]
log
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-20 04:25:30 PST
Hmm, 0.3 line is scrolled by each native event. And the system settings are default (3 lines/chars). So, the scrolling speed must be multiplied by 2 in ESM.

So, I have no idea why you feel this slow. How many times did you swipe the touchpad?
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-20 04:38:19 PST
Ah, perhaps, the normal scroll speed which you think is overridden scroll speed. And the tested device isn't overridden the scroll speed by ESM.

http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#645

The device supports high-resolution scroll. Therefore, nsMouseWheelTransaction::OverrideSystemScrollSpeed() does nothing.
http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#645

I think that we should improve the mechanism for high resolution scrollable devices.
Comment 19 Olli Pettay [:smaug] (reviewing overload) 2013-02-20 05:10:43 PST
Ah, of course :)

Can we detect whether the input is coming from a touchscreen or touchpad, since only the latter one
should get some acceleration.
Comment 20 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-02-20 05:15:31 PST
I don't familiar with raw input. If it's possible, we need to handle raw input event, probably.

# Basically, acceleration is a job of such device driver.
Comment 21 Avi Halachmi (:avih) 2013-02-24 21:28:12 PST
Created attachment 717732 [details]
Testcase #2 (cross browser + total distance)

This testcase works on all browsers (except for IE which doesn't display the gradient), records (hopefully) all related events, accumulates them, and displays the page position at the end of the scroll.

Here are my results after testing using this testcase on Nightly, Chrome, IE9 and Opera, with both a quick scroll, and a longer one.

System: Windows 7 home premium.
HW: Asus N56VZ, Elantech touchpad.

- All browsers allowed pixel-accurate vertical scroll with the touchpad.
- Firefox displays fractional line deltas for wheel.deltaY(DOM_DELTA_LINE)
- No browser displayed any horizontal event data,
    eventhough all browsers scrolled horizontally in "line" jumps.

In general, if Firefox scrolls X pixels for a top-to-bottom swipe of the touchpad, then, for the same swipe, on my system:

- Chrome scrolls ~1.7x
- IE9 scrolls ~6x
- Opera scrolls ~13x

Also, opera is the only browser which scrolled longer distance (consistently) on the slow swipe. All other browsers scrolled slightly shorter distance than the short swipe.

All the tests were repeated several times to make sure the results are relatively consistent.


Quick scroll: about 1000ms top to bottom
----------------------------------------

Nightly (22):
-------------
pageYOffset: 478
Accumulated:
Count: 114, Sum: 26.1, Event: wheel.deltaY(DOM_DELTA_LINE)
Count: 99, Sum: 406.0, Event: MozMousePixelScroll.detail(VERTICAL_AXIS)
Count: 26, Sum: 26.0, Event: DOMMouseScroll.detail(VERTICAL_AXIS)

Chrome (25.0...):
-----------------
pageYOffset: 819
Accumulated:
Count: 99, Sum: -988.0, Event: mousewheel.wheelDeltaY
Count: 99, Sum: -988.0, Event: mousewheel.wheelDelta

IE9:
----
pageYOffset: 2898
Accumulated:
Count: 24, Sum: 3644.0, Event: wheel.deltaY(DOM_DELTA_PIXEL)
Count: 24, Sum: -2592.0, Event: mousewheel.wheelDelta

Opera 12.11:
------------
pageYOffset: 6528
Accumulated:
Count: 103, Sum: 172.0, Event: mousewheel.detail
Count: 103, Sum: -6880.0, Event: mousewheel.wheelDeltaY
Count: 103, Sum: -6880.0, Event: mousewheel.wheelDelta


Slow scroll: about 3000ms top to bottom:
----------------------------------------

Nightly:
--------
pageYOffset: 389
Accumulated:
Count: 180, Sum: 21.1, Event: wheel.deltaY(DOM_DELTA_LINE)
Count: 155, Sum: 242.0, Event: MozMousePixelScroll.detail(VERTICAL_AXIS)
Count: 21, Sum: 21.0, Event: DOMMouseScroll.detail(VERTICAL_AXIS)

Chrome:
-------
pageYOffset: 686
Accumulated:
Count: 151, Sum: -826.0, Event: mousewheel.wheelDeltaY
Count: 151, Sum: -826.0, Event: mousewheel.wheelDelta

IE9:
----
pageYOffset: 2243
Accumulated:
Count: 40, Sum: 2807.0, Event: wheel.deltaY(DOM_DELTA_PIXEL)
Count: 40, Sum: -2016.0, Event: mousewheel.wheelDelta

Opera:
------
pageYOffset: 9473 (yes, it really did scroll that far)
Accumulated:
Count: 168, Sum: 246.0, Event: mousewheel.detail
Count: 168, Sum: -9840.0, Event: mousewheel.wheelDeltaY
Count: 168, Sum: -9840.0, Event: mousewheel.wheelDelta
Comment 22 Avi Halachmi (:avih) 2013-02-24 22:56:28 PST
Created attachment 717750 [details]
Testcase #2 (cross browser + total distance): Slight improvement

This revised testcase has slightly prettier print, and also takes into account the scroll which happens after the last event (as happens with Smooth scroll). However, the results are within 5% of the previous variant of this testcase, the the previous results are still valid.

Also, for reference, I'm attaching a test run of this testcase using a proper mouse wheel (full "swipe" of the mouse wheel (with discrete notches) in a relatively medium steady speed).

Note that using the mouse wheel, the scroll distances are almost identical between browsers.

So indeed we have a slowness issue (on my system, as well) using the touchpad.


Results using one swipe of a mouse wheel with discrete notches:

Nightly:
--------
pageYOffset: 1596
Accumulated:
Count: 14, Sum: 42.0, Event: wheel.deltaY (deltaMode: DOM_DELTA_LINE)
Count: 14, Sum: 42.0, Event: DOMMouseScroll.detail (axis: VERTICAL_AXIS)
Count: 14, Sum: 798.0, Event: MozMousePixelScroll.detail (axis: VERTICAL_AXIS)

Chrome:
-------
pageYOffset: 1400
Accumulated:
Count: 14, Sum: -1680.0, Event: mousewheel.wheelDeltaY

IE9:
----
pageYOffset: 1863
Accumulated:
Count: 7, Sum: 2324.0, Event: wheel.deltaY (deltaMode: DOM_DELTA_PIXEL)
Count: 7, Sum: -1680.0, Event: mousewheel.wheelDelta

Opera:
------
pageYOffset: 1680
Accumulated:
Count: 14, Sum: 42.0, Event: mousewheel.detail
Count: 14, Sum: -1680.0, Event: mousewheel.wheelDeltaY
Count: 14, Sum: -1680.0, Event: mousewheel.wheelDelta
Comment 23 mayankleoboy1 2013-03-04 19:04:35 PST
i use a Dell inspiron N5010 with a Synaptic touchpad, Win7 x64.  In the touchpad drivers,I  adjust the acceleration and speed of scrolling using FF as the base. This has the result that Opera is extremely fast (unusable) , IE10 is fast, Chrome is almost like Firefox. (and of course, FF feels just OK )

Nightly 22 : 
-----------
pageYOffset: 29369
Accumulated:
Count: 69, Sum: 5511.5, Event: wheel.deltaY (deltaMode: DOM_DELTA_LINE)
Count: 69, Sum: 5511.0, Event: DOMMouseScroll.detail (axis: VERTICAL_AXIS)
Count: 69, Sum: 104702.0, Event: MozMousePixelScroll.detail (axis: VERTICAL_AXIS)


Chrome 25:
----------
pageYOffset: 29380
Accumulated:
Count: 43, Sum: -114113.0, Event: mousewheel.wheelDeltaY (deltaMode: DOM_DELTA_PIXEL)
Count: 43, Sum: -114113.0, Event: mousewheel.wheelDelta (deltaMode: DOM_DELTA_PIXEL)


Opera :
-------
pageYOffset: 29378
Accumulated:
Count: 36, Sum: 836.0, Event: mousewheel.detail
Count: 36, Sum: -33440.0, Event: mousewheel.wheelDeltaY
Count: 36, Sum: -33440.0, Event: mousewheel.wheelDelta

IE10:
------
 pageYOffset: 29350
Accumulated:
Count: 16, Sum: 39340.6, Event: wheel.deltaY (deltaMode: DOM_DELTA_PIXEL)
Count: 16, Sum: -80328.0, Event: mousewheel.wheelDelta
Comment 24 Ronan Jouchet 2013-03-09 09:19:46 PST
Here are some Linux-based results on a Dell XPS1645 (synaptic touchpad), Ubuntu 12.10 x64, radeonhd driver (free community-maintained, non-binaryblob). I didn't touch my mouse/touchpad settings. 1. Firefox feels extremely sluggish, 2. Chrome feels fast, Epiphany/Web feels a bit sluggish. Below are my results for http://avih.github.com/testcases/mozwheel-test-v4.html

UA: Mozilla/5.0 (X11; Linux x86_64; rv:21.0) Gecko/20130309 Firefox/21.0
-----------------
pageYOffset: 5415
Accumulated:
Count: 1995, Sum: 393.0, Event: wheel.deltaY (deltaMode: DOM_DELTA_LINE)
Count: 1919, Sum: 333.0, Event: DOMMouseScroll.detail (axis: VERTICAL_AXIS)
Count: 1919, Sum: 6327.0, Event: MozMousePixelScroll.detail (axis: VERTICAL_AXIS)
Count: 115, Sum: 97.0, Event: wheel.deltaX (deltaMode: DOM_DELTA_LINE)
Count: 116, Sum: 98.0, Event: DOMMouseScroll.detail (axis: HORIZONTAL_AXIS)
Count: 116, Sum: 980.0, Event: MozMousePixelScroll.detail (axis: HORIZONTAL_AXIS)

UA: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.28 Safari/537.31
-----------------
pageYOffset: 0
Accumulated:
Count: 13, Sum: 1560.0, Event: mousewheel.wheelDeltaY (deltaMode: DOM_DELTA_PIXEL)
Count: 731, Sum: -5160.0, Event: mousewheel.wheelDelta (deltaMode: DOM_DELTA_PIXEL)
Count: 74, Sum: -6720.0, Event: mousewheel.wheelDeltaX (deltaMode: DOM_DELTA_PIXEL)

UA: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.6+ (KHTML, like Gecko) Chromium/23.0.1271.95 Chrome/23.0.1271.95 Safari/537.6+ Ubuntu/12.10 (3.6.1-0ubuntu1) Epiphany/3.6.1
-----------------
pageYOffset: 0
Accumulated:
Count: 189, Sum: -10681.0, Event: mousewheel.wheelDeltaX
Count: 418, Sum: 10059.0, Event: mousewheel.wheelDeltaY
Count: 420, Sum: 9961.0, Event: mousewheel.wheelDelta
Comment 25 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 01:57:47 PDT
Created attachment 724831 [details] [diff] [review]
Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=81cacb2fd919
Comment 26 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 02:47:11 PDT
Created attachment 724842 [details] [diff] [review]
Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=da912014fb9b

I realized that OverrideSystemScrollSpeed() is now called very many times because we support higher resolution scrolling since D3E WheelEvent support.

Therefore, I guess that checking the prefs is not so cheap cost. I think that it's better to use Add*VarCache().
Comment 27 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 04:25:24 PDT
This must be a regression of bug 719320 (D3E WheelEvent).
Comment 28 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 04:57:26 PDT
If you want to test patched build actually, you can get it from here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/masayuki@d-toybox.com-da912014fb9b/try-win32/
Comment 29 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 05:05:42 PDT
Comment on attachment 724842 [details] [diff] [review]
Patch

This regression is caused by my wrong decision. I assumed that high-resolution support pointing devices set good scroll speed and have their own acceleration system.

However, actually, the logged environment supports high resolution scroll but the user feels the speed slow.

So, I think that when pointing device doesn't customize the scrolling speed, we should always use "system scroll speed override".

And for reducing the run time cost of calls nsIWidget::OverrideSystemMouseScrollSpeed(), we should redesign its arguments for D3E WheelEvent. And now, we support higher resolution scroll before implementing D3E WheelEvent. Therefore, OverrideSystemMouseScrollSpeed() is called higher frequently. So, I think that we should cache the pref values into static variables.
Comment 30 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 05:10:12 PDT
FYI: Logitech mouse driver, SetPoint 6.5 uses too high resolution scroll. Therefore, I believe that wheel event handler should reduce run time cost as far as possible.
Comment 31 timbugzilla 2013-03-14 05:16:10 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> FYI: Logitech mouse driver, SetPoint 6.5 uses too high resolution scroll.
> Therefore, I believe that wheel event handler should reduce run time cost as
> far as possible.

Interesting. I've been having a problem where clicking on a click-to-play plugin element changes the number of lines scrolled per mousewheel increment (from 6 to 3) with my Logitech MX Revolution mouse.
Comment 32 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 05:22:12 PDT
Note that I'm not sure if this regression fix will satisfy all of them who complained about this in this bug. If they will still feel slow after landing the patch, let's separate it to another bug.
Comment 33 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 05:30:42 PDT
(In reply to timbugzilla from comment #31)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> > FYI: Logitech mouse driver, SetPoint 6.5 uses too high resolution scroll.
> > Therefore, I believe that wheel event handler should reduce run time cost as
> > far as possible.
> 
> Interesting. I've been having a problem where clicking on a click-to-play
> plugin element changes the number of lines scrolled per mousewheel increment
> (from 6 to 3) with my Logitech MX Revolution mouse.

SetPoint checks the scroll target window before sending wheel events. Therefore, when plugin window has focus, it probably doesn't use high resolution scroll. And the 3 is probably the correct scroll amount. 6 is the result of overridden by our "system scroll speed override".
https://wiki.mozilla.org/Gecko:Mouse_Wheel_Scrolling#Override_system_of_system_scroll_speed
Comment 34 Olli Pettay [:smaug] (reviewing overload) 2013-03-14 10:11:52 PDT
Comment on attachment 724842 [details] [diff] [review]
Patch


> DeltaValues
> nsMouseWheelTransaction::OverrideSystemScrollSpeed(widget::WheelEvent* aEvent)
> {
>   MOZ_ASSERT(sTargetFrame, "We don't have mouse scrolling transaction");
>   MOZ_ASSERT(aEvent->deltaMode == nsIDOMWheelEvent::DOM_DELTA_LINE);
> 
>-  DeltaValues result(aEvent);
>+  DeltaValues originalDeltaValues(aEvent);
> 
>   // If the event doesn't scroll to both X and Y, we don't need to do anything
>-  // here.  And also, if the event indicates the device supports high
>-  // resolution scroll, we shouldn't need to override it.
>-  if ((!aEvent->lineOrPageDeltaX && !aEvent->lineOrPageDeltaY) ||
>-      (static_cast<double>(aEvent->lineOrPageDeltaX) != aEvent->deltaX) ||
>-      (static_cast<double>(aEvent->lineOrPageDeltaY) != aEvent->deltaY)) {
>-    return result;
>+  // here.
>+  if (!aEvent->deltaX && !aEvent->deltaY) {
>+    return originalDeltaValues;
>   }
Does returning aEvent here not work? DeltaValues has the right kind of ctor, so I'd assume it should work.

> 
>   // We shouldn't override the scrolling speed on non root scroll frame.
>   if (sTargetFrame !=
>         sTargetFrame->PresContext()->PresShell()->GetRootScrollFrame()) {
>-    return result;
>+    return originalDeltaValues;
>   }
Same here. But if not, this code is ok too.


>+  NS_ENSURE_TRUE(widget, originalDeltaValues);
>+  DeltaValues overriddenDeltaValues(0.0, 0.0);
>+  nsresult rv =
>+    widget->OverrideSystemMouseScrollSpeed(aEvent->deltaX, aEvent->deltaY,
>+                                           overriddenDeltaValues.deltaX,
>+                                           overriddenDeltaValues.deltaY);
>+  return NS_FAILED(rv) ? originalDeltaValues : overriddenDeltaValues;
If returning aEvent worked, this would become
return NS_FAILED(rv) ? DeltaValues(originalDeltaValues) : overriddenDeltaValues;

>+nsWindow::OverrideSystemMouseScrollSpeed(double aOriginalDeltaX,
>+                                         double aOriginalDeltaY,
>+                                         double &aOverriddenDeltaX,
>+                                         double &aOverriddenDeltaY)
Not sure about the coding style in this file, but usually
double& aFoo

>+nsBaseWidget::OverrideSystemMouseScrollSpeed(double aOriginalDeltaX,
>+                                             double aOriginalDeltaY,
>+                                             double &aOverriddenDeltaX,
>+                                             double &aOverriddenDeltaY)
ditto
Comment 35 Matt Brubeck (:mbrubeck) 2013-03-14 11:23:23 PDT
Nominating for tracking because this is a recent user-facing regression (introduced in Firefox 17).
Comment 36 Avi Halachmi (:avih) 2013-03-14 14:29:35 PDT
Comment on attachment 724842 [details] [diff] [review]
Patch

Looks good as far as I can tell.
Comment 37 timbugzilla 2013-03-14 14:32:16 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #33)
> (In reply to timbugzilla from comment #31)
> > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #30)
> > > FYI: Logitech mouse driver, SetPoint 6.5 uses too high resolution scroll.
> > > Therefore, I believe that wheel event handler should reduce run time cost as
> > > far as possible.
> > 
> > Interesting. I've been having a problem where clicking on a click-to-play
> > plugin element changes the number of lines scrolled per mousewheel increment
> > (from 6 to 3) with my Logitech MX Revolution mouse.
> 
> SetPoint checks the scroll target window before sending wheel events.
> Therefore, when plugin window has focus, it probably doesn't use high
> resolution scroll. And the 3 is probably the correct scroll amount. 6 is the
> result of overridden by our "system scroll speed override".
> https://wiki.mozilla.org/Gecko:
> Mouse_Wheel_Scrolling#Override_system_of_system_scroll_speed

Thanks for the analysis of this - is there anything that can be done?
Comment 38 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-14 18:18:34 PDT
Created attachment 725207 [details] [diff] [review]
Patch

Thank you, smaug and avih!
Comment 39 mayankleoboy1 2013-03-14 21:17:33 PDT
When this lands, will we have to recaliberate the acceleration of the touchpads ?
Comment 40 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-15 02:11:15 PDT
Thank you roc.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0df4c867df8

(In reply to mayankleoboy1 from comment #39)
> When this lands, will we have to recaliberate the acceleration of the
> touchpads ?

Depends on the system settings. See https://wiki.mozilla.org/Gecko:Mouse_Wheel_Scrolling#Override_system_of_system_scroll_speed
Comment 41 Alex Keybl [:akeybl] 2013-03-15 14:43:19 PDT
Too close to FF20's release to take a touchpad fix, but we'll track for FF21.
Comment 42 Phil Ringnalda (:philor) 2013-03-16 15:59:17 PDT
https://hg.mozilla.org/mozilla-central/rev/d0df4c867df8
Comment 43 bhavana bajaj [:bajaj] 2013-03-18 17:13:06 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #40)
> Thank you roc.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d0df4c867df8
> 
> (In reply to mayankleoboy1 from comment #39)
> > When this lands, will we have to recaliberate the acceleration of the
> > touchpads ?
> 
> Depends on the system settings. See
> https://wiki.mozilla.org/Gecko:
> Mouse_Wheel_Scrolling#Override_system_of_system_scroll_speed

Can you please request approval nomination with risk analysis to uplift this on aurora, looks like its fixed on m-c now ? Thanks !
Comment 44 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-18 17:27:05 PDT
Indeed, however, I'd like somebody verify the fix on some laptops before request.
Comment 45 Olli Pettay [:smaug] (reviewing overload) 2013-03-19 01:08:58 PDT
/me reboots this laptop in a minute to test behavior on Win8.
Comment 46 Olli Pettay [:smaug] (reviewing overload) 2013-03-19 01:45:30 PDT
At least on this Asus U500vz Nightlies feel now good.
Comment 47 Avi Halachmi (:avih) 2013-03-19 01:51:43 PDT
On my system new Nightlies now scroll with 2x distance for touchpad scroll, compared to before this patch. To me it feels better than before and good in general. It's now about 60% faster than Chrome (25), and about on par with IE10.
Comment 48 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-19 05:03:35 PDT
Thank you, guys.
# IIRC, Google Chrome doesn't aware with high resolution scroll event.
Comment 49 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-19 08:25:39 PDT
Created attachment 726673 [details] [diff] [review]
Patch for Aurora

I merged the patch by my hands. I'll test it.
# Unfortunately, tomorrow is a national holiday of Japan, so, I'm not sure if I can check it tomorrow.
Comment 50 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-26 01:21:55 PDT
Comment on attachment 726673 [details] [diff] [review]
Patch for Aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
Bug 719320

User impact if declined:
Some users feel the scroll speed on root scrollable element slower than Fx16 or earlier.

Testing completed (on m-c, etc.):
The patch was already landed on m-c last week.

Risk to taking this patch (and alternatives if risky): 
Some other users might feel too fast the scroll speed by the system scroll override. Although, they can disable this feature with about:config:
mousewheel.system_scroll_override_on_root_content.enabled

String or UUID changes made by this patch: 
This needs the change of nsIWidget, which causes UUID change of it.
Comment 51 bhavana bajaj [:bajaj] 2013-03-27 15:52:25 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #50)
> Comment on attachment 726673 [details] [diff] [review]
> Patch for Aurora
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #):
> Bug 719320
> 
> User impact if declined:
> Some users feel the scroll speed on root scrollable element slower than Fx16
> or earlier.
> 
> Testing completed (on m-c, etc.):
> The patch was already landed on m-c last week.
> 
> Risk to taking this patch (and alternatives if risky): 
> Some other users might feel too fast the scroll speed by the system scroll
> override.

 Although, they can disable this feature with about:config:
> mousewheel.system_scroll_override_on_root_content.enabled
> 
> String or UUID changes made by this patch: 
> This needs the change of nsIWidget, which causes UUID change of it.

Given the risk , limited testing this has got and considering we are a couple of days away from the merge, this patch is almost hitting our beta audience directly without enough aurora population experiencing it.

Considering this issue is been there since Fx17 I think we should maintain the status quo for one more cycle in this case to avoid any unforeseen regressions this may cause due to so many different hardware configurations out there. wdyt ?
Comment 52 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-27 16:11:37 PDT
Excuse me, I don't know what means "wdyt".

Unfortunately, we don't have much time for next merge. So, I recommend to take this to aurora *not* strongly. If you worry the risk, it's also a good choice for risk management. This bug fix might kill better UX with the higher scrolling speed. So, the risk isn't so low.
Comment 53 bhavana bajaj [:bajaj] 2013-03-27 17:03:29 PDT
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #52)
> Excuse me, I don't know what means "wdyt".

Sorry about the slang here just meant to ask your opinion here (what do you think - WDYT) :)
> 
> Unfortunately, we don't have much time for next merge. So, I recommend to
> take this to aurora *not* strongly. If you worry the risk, it's also a good
> choice for risk management. This bug fix might kill better UX with the
> higher scrolling speed. So, the risk isn't so low.

Considering we are in the end of aurora cycle and based on the outlined risk letting this ride the trains instead of uplift here.
Comment 54 Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2013-03-27 17:11:37 PDT
(In reply to bhavana bajaj [:bajaj] from comment #53)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #52)
> > Excuse me, I don't know what means "wdyt".
> 
> Sorry about the slang here just meant to ask your opinion here (what do you
> think - WDYT) :)

Thank you, such English knowledge helps me.

> > 
> > Unfortunately, we don't have much time for next merge. So, I recommend to
> > take this to aurora *not* strongly. If you worry the risk, it's also a good
> > choice for risk management. This bug fix might kill better UX with the
> > higher scrolling speed. So, the risk isn't so low.
> 
> Considering we are in the end of aurora cycle and based on the outlined risk
> letting this ride the trains instead of uplift here.

Okay, I agree with you. Thank you for the judge.

Note You need to log in before you can comment on or make changes to this bug.