1320421 - We fail to properly detect a scroll target in split Google Docs spreadsheet when scrolling horizontally

Status: RESOLVED WORKSFORME

(Core :: DOM: Events, defect)

Description

[Stephen A Pohl [:spohl]]

(Split out from bug 1267445 to focus on the first issue described in bug 1267445 comment 14.)

In the right section of the split Google Docs spreadsheet[1] we fail to initiate horizontal scrolls without first scrolling vertically. If a horizontal scroll is attempted, there is either no feedback or we perform a history swipe. Google Docs currently uses legacy mouse scroll events.

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

Comment 1

[Stephen A Pohl [:spohl]]

Attachment: Patch

This patch fixes the issue because we now get a scroll target returned by the call to ComputeScrollTarget and no longer set wheelEvent->mViewPortIsOverscrolled to true. We then properly initiate horizontal scrolls. Markus, this was introduced in bug 1016035. Could this be the proper fix, or is there a deeper issue why COMPUTE_DEFAULT_ACTION_TARGET doesn't return a scroll target?

History swipes still work if a horizontal scroll is initiated on sections of the page that are not part of the actual spreadsheet on the page.

If this is the right approach I'm wondering if we should also change the flag for the ComputeScrollTarget call that's used to set wheelEvent->mViewPortIsOverscrolled to true about 50 lines earlier.

Comment 2

[Markus Stange [:mstange]]

(In reply to Stephen A Pohl [:spohl] from comment #0)
> In the right section of the split Google Docs spreadsheet[1] we fail to
> initiate horizontal scrolls without first scrolling vertically. If a
> horizontal scroll is attempted, there is either no feedback or we perform a
> history swipe. Google Docs currently uses legacy mouse scroll events.

I cannot confirm any of these statements. Maybe things have changed in the meantime.

What I see is:
 - During loading, there is a phase during which the table contents are visible, but the menu at the top is greyed out. Call this the "Loading" phase.
 - At some point, it briefly displays "Working..." at the top and then the menu becomes available. Call the period after this moment the "Ready" phase.
 - During the "Loading" phase, scrolling horizontally in any part of the spreadsheet causes history swiping.
 - During the "Ready" phase, scrolling horizontally in any part of the spreadsheet causes the spreadsheet to scroll, and no history swiping.
 - During the "Loading" phase, there is no event listener set in the DOM on the ancestor chain of the spreadsheet content.
 - During the "Ready" phase, there is a "wheel" event listener set on the .grid-container element, and it correctly preventDefault()s all wheel events.
 - There is no "DOMMouseScroll" event listener anywhere. So they're not using legacy wheel events.
 - In Chrome, if you scroll horizontally during the "Loading" phase, you also end up doing a history swipe.
 - In fact, in Chrome, if you attempt a history swipe during the "Loading" phase, and then abort it by scrolling back before you lift your fingers, then the page is stuck in "swipe mode" even during the "Ready" phase. In my tests the page stayed stuck in that state for multiple minutes but eventually recovered.

Comment 3

[Stephen A Pohl [:spohl]]

(In reply to Markus Stange [:mstange] from comment #2)
> (In reply to Stephen A Pohl [:spohl] from comment #0)
> > In the right section of the split Google Docs spreadsheet[1] we fail to
> > initiate horizontal scrolls without first scrolling vertically. If a
> > horizontal scroll is attempted, there is either no feedback or we perform a
> > history swipe. Google Docs currently uses legacy mouse scroll events.
> 
> I cannot confirm any of these statements. Maybe things have changed in the
> meantime.

Yes, things have definitely changed. I'm unable to reproduce with a local build off of current m-c or the most recently released version of Firefox (50.1.0), so this change must have occurred on Google Docs' side.