1267445 - Sideways two-finger scrolling in a split google sheets is twitchy.

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect, P1)

Description

[Johnny Stenback (:jst)]

If you split a Google spreadsheet horizontally then the sideways scrolling of the right side of the split is very unpredictable and very twitchy in Firefox on at OS X (though not on Linux).

For those who do a ton of work in Google Sheets this is very annoying and causes users to switch to another browser... See linked spreadsheet for an example, try to scroll the right side of the spreadsheet sideways with two finger scrolling on a track pad.

https://docs.google.com/spreadsheets/d/1mzi9wk0VtVr8kUEF0pxmotY8fnQB5-TGWzHRgMA3ZWU/edit

Comment 1

[Flore Allemandou [:flore]]

At first I agreed with you... Then I looked into my trackpad preferences (additional gestures) and noticed that the first line (sorry my computer is in french, so it may not be a correct translation):
'swipe between pages' was selected as '2 finger scroll' (1st item) and if I understand correctly this feature, it's for going back and forth in the browsing page history. If I changed it to '3 fingers swipe' (2nd or 3rd item on the list), I was able to scroll horizontally in your google spreadsheet.

However, I tested the same link in safari with the '2 finger scroll' selected (as it is the default setting) and there is no problem, the horizontal scroll is perfect (and erratic on Nightly with these same settings).

But if I open a large image (wider than my window) and zoom it to actual size, I can scroll horizontally and vertically without a problem on Firefox (and without going one page back).

So clearly there is a problem on how Firefox handles the split google spreadsheet (not like the non spreadsheet)

Confirmed on Nightly 20160503030421 on mac OSX

Comment 2

[Jim Mathies [:jimm]]

jst, are you still seeing this? Testing in Safari and firefox 49, two finger scroll moves from cell to cell, which ais expected.

Comment 3

[Johnny Stenback (:jst)]

I am, or more specifically mrbkap was as he helped me test this. Something's still not right here.

Comment 4

[Jim Mathies [:jimm]]

Note this is related to horizontal scroll of the split panel.

Comment 5

[Jim Mathies [:jimm]]

From Kats:

It looks like Google Sheets uses a canvas and intercepts
DOMMouseScroll events to implement their own scrolling. As long as we
are dispatching those events properly it should work fine, but
horizontal swiping is impacted by the back/forward detection as well.
That might be impacting the events we're sending, I'm not sure. If you
want me to investigate more I can do that, but it'll delay touch
support on Windows which is what I'm working on now.

Comment 6

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

So we do dispatch pixel level wheel events on OSX, and the legacy
MozMousePixelScroll and DOMMouseScroll. If the page is handling only DOMMouseScroll, I assume it might
miss some information.
It should use wheel events which are implemented in all the browsers.

Comment 7

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

This looks like an evang issue to me.

Comment 8

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

Is the behavior the same if chrome UA string is used?

Comment 9

[Jim Mathies [:jimm]]

STR:

1) open test case
https://docs.google.com/spreadsheets/d/1mzi9wk0VtVr8kUEF0pxmotY8fnQB5-TGWzHRgMA3ZWU/edit
2) click on the right side pane
3) drag two fingers back and forth over the mac track pad horizontally

result: nothing
expected: the right hand spread sheet pane should scroll horizontally

4) drag two fingers vertically across the track pad to start a vertical scroll, then continue horizontally

result: spreadsheet scrolls vertically then horizontally
expected: same

It seem we don't do a good job of detecting the start of a horizontal drag.

Comment 10

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

Or more likely the web page isn't handling the event for that scroll.

Comment 11

[Mike Taylor [:miketaylr]]

Karl, given comment #6, can we bring this up with Google?

Comment 12

[Johnny Stenback (:jst)]

Olli already raised this with Google last week. IIRC we're still waiting on a response.

Comment 13

[Karl Dubost💡 :karlcow]

Yup confirmed that the contact has been started. Thanks Olli. 
Message-ID: <[e-mail]>
Date: Fri, 29 Jul 2016 00:28:36 +0300

And Google has routed to someone who might be able to help.
Switching to sitewait.

Comment 14

[Stephen A Pohl [:spohl] (OOO until 2/27)]

I've been looking into this. I believe there are two issues:
1. It is impossible to scroll horizontally, unless a vertical scroll is initiated first.
2. When scrolling horizontally, we skip over columns when scrolling right but skip over different columns when scrolling left, which makes it seem erratic (or "twitchy", as stated in the bug title).

I've focused on 1 first since it appears to be the greater problem of the two.

It turns out that we fail to scroll horizontally because we believe that we're initiating a horizontal swipe. While tracking horizontal swipes we're not sending DOMMouseScroll events. I find it suspect that scrollTarget is null in EventStateManager::PostHandleEvent[1] when we clearly seem to have a scroll target in Google Docs. If scrollTarget was non-null, we wouldn't set mViewPortIsOverscrolled to true for the event[2] and we would not track scroll events as swipe[3].

I've confirmed that if we no longer track these horizontal scroll events as swipe, DOMMouseScroll events are sent successfully and we can properly initiate horizontal scrolls in Google Docs. This still leaves problem 2 though.

[1] https://hg.mozilla.org/mozilla-central/annotate/26e22af660e543ebb69930f082188b69ec756185/dom/events/EventStateManager.cpp#l3223
[2] https://hg.mozilla.org/mozilla-central/annotate/26e22af660e543ebb69930f082188b69ec756185/dom/events/EventStateManager.cpp#l3224
[3] https://hg.mozilla.org/mozilla-central/annotate/26e22af660e543ebb69930f082188b69ec756185/widget/cocoa/nsChildView.mm#l2937

Comment 15

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

Don't forget to test MozMousePixelScroll handling. That could lead to (2) (which I thought this bug is about).

Comment 16

[Stephen A Pohl [:spohl] (OOO until 2/27)]

(In reply to Olli Pettay [:smaug] from comment #15)
> Don't forget to test MozMousePixelScroll handling. That could lead to (2)
> (which I thought this bug is about).

For each initiated horizontal scroll we send one MozMousePixelScroll event with an mDelta of 1. All subsequent obj-c scroll events are then tracked as swipe and we don't send any more MozMousePixelScroll events until a new horizontal swipe is initiated. We don't send a DOMMouseScroll event along with this first MozMousePixelScroll, since the mDelta for DOMMouseScroll is 0.

If we initiate a vertical scroll first and immediately go to a horizontal scroll without releasing the touchpad, both DOMMouseScroll and MozMousePixelScroll events are sent continuously, since we're not tracking the horizontal scroll as swipe.

From what I can tell based on the loaded .js files[1][2][3][4][5], Google Docs only seems to handle DOMMouseScroll and mousewheel events.

So, comment 6 still applies: If Google Docs handled wheel events, it would seem that this would fix issue 2 from comment 14.

We should still do something about issue 1 in comment 14. Detecting that we have a scroll target on the page seems like the right fix, but at this point it isn't clear to me why this isn't already the case.


[1] https://docs.google.com/static/spreadsheets2/client/js/1101830378-ritz_waffle_i18n_core.js
[2] https://docs.google.com/static/spreadsheets2/client/js/2654039544-ritz_waffle_i18n_viewer.js
[3] https://docs.google.com/static/spreadsheets2/client/js/3599812221-ritz_waffle_i18n_gvizcharts.js
[4] https://docs.google.com/static/spreadsheets2/client/js/3575830417-ritz_waffle_i18n_docos.js
[5] https://docs.google.com/static/spreadsheets2/c76dd15d6a/ritz/6C357BC239E040DCE0E28AAB9D96A9E6.cache.js

Comment 17

[Stephen A Pohl [:spohl] (OOO until 2/27)]

Unfortunately, my attempts at figuring out why we have a null scrollTarget in EventStateManager::PostHandleEvent[1] (see comment 14) have been fruitless so far. :smaug, is this something straightforward for you to figure out? Or do you know who could help here? Thanks!

[1] https://hg.mozilla.org/mozilla-central/annotate/26e22af660e543ebb69930f082188b69ec756185/dom/events/EventStateManager.cpp#l3223

Comment 18

[Stephen A Pohl [:spohl] (OOO until 2/27)]

I have filed bug 1320421 to deal with the first issue described in comment 14. This bug should track the tech evangelism issue of having Google Docs switch to the new wheel events (second issue described in comment 14 & comment 16).

Clearing n-i as there is a patch with feedback request for the first issue from comment 14 in bug 1320421.

Comment 19

[Desigan Chinniah [:cyberdees] [:dees] [London - GMT]]

(In reply to Stephen A Pohl [:spohl] from comment #18)
> I have filed bug 1320421 to deal with the first issue described in comment
> 14. This bug should track the tech evangelism issue of having Google Docs
> switch to the new wheel events (second issue described in comment 14 &
> comment 16).

+:miketaylr - for the TE bit
+:ssaviano - for the Google Suite Sheets team supporting new wheel events comments

Comment 20

[Steven Saviano]

Focusing our efforts first on https://bugzilla.mozilla.org/show_bug.cgi?id=1205573

Will evaluate this issue after that is resolved. Thanks for sharing!

Comment 21

[Mike Taylor [:miketaylr]]

We did reach out to Google about this (see Comment #13) 4 months ago, but it seems like nothing happened. I re-pinged the thread.

Comment 22

[Mike Taylor [:miketaylr]]

(and to follow up, Google is going to work on Bug 1205573 before getting to this -- thanks Steven!)

Comment 23

[Josh Danziger]

FYI - I've filed bug # 33172560 to track this issue on the Google side.

Comment 24

[Andrew Overholt [:overholt]]

Bug 1320421 comment 2 indicates DOMMouseScroll is no longer used in Google Sheets. Steven, can you confirm some changes were made in Google Docs? TIA :)

Comment 25

[Steven Saviano]

Yes, we have been making changes as recommended. I believe another change or so is still slated to rollout this week for better mouse wheel support. We still need to verify this specific issue though. Myself or Josh will report back.

Comment 26

[Steven Saviano]

Still waiting here on confirmation from specific eng that all cases are handled. Didn't loose track.

Comment 27

[Steven Saviano]

Confirmed that this is fixed on our end. Andrew, I'm not sure I can mark this as fixed (doesn't seem like it), but feel free to verify and close out.

Comment 28

[Mike Taylor [:miketaylr]]

I can confirm this is fixed and working now -- thanks!

Comment 29

[Johnny Stenback (:jst)]

(In reply to Steven Saviano from comment #27)
> Confirmed that this is fixed on our end. Andrew, I'm not sure I can mark
> this as fixed (doesn't seem like it), but feel free to verify and close out.

Thanks Steven! I just gave you super powers in bugzilla so you can edit bugs going forward, use them carefully :)

Comment 30

[Steven Saviano]

Thanks! Will only use as appropriate on the Google-specific bugs.