771757 - Subdocument scrolling is broken on any page with touch events

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Unscrollable dropdown (Firefox)

1) Open Firefox.
2) Open the URL bar and go to news.google.com.
2a) If you are served the mobile site, scroll to the bottom and select "Classic" to get the desktop site.
3) Select the "U.S. edition" dropdown under the search bar (the dropdown text might vary based upon your location).
4) Attempt to scroll the dropdown.

Expected: The dropdown scrolls.
Actual: The dropdown is unscrollable and the entire page moves instead.

Both dropdowns on the page do not work.

The content appears to be div based, using custom css - a "goog-menu".

This is problematic on tablet devices which are served the desktop site by default.

Stock browser and Chrome are also unable to scroll. Chrome (phone) displays a improperly placed scrollbar but does not scroll. Tablet versions of both browsers expand the dropdown to be visible all at once.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Expanded dropdown (Stock browser - Tablet)

Comment 2

[Aaron Train [:aaronmt]]

Kats/Matt, thoughts?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

(In reply to Michael Comella (:mcomella) from comment #0)
> Stock browser and Chrome are also unable to scroll. Chrome (phone) displays
> a improperly placed scrollbar but does not scroll.

On a Galaxy Nexus with ICS it works in both the stock browser and Chrome, for me. The stock browser glitches while scrolling it but it's generally usable. I got it to scroll the first time I loaded it in nightly, but it hasn't worked since then, which is odd. I'll debug it to see what's going on.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

It looks like this happens on any subdocument scrolling when the page has touch listeners registered. The code from PanZoomController.onTouchStart gets run after the Panning:Override message comes in from browser.js. One of the first things PanZoomController.onTouchStart does is call mSubscroller.cancel() which cancels the panning override behaviour, which aborts subdocument scrolling.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Likely a regression from bug 744518 which made the browser.js Panning:Override code run earlier (before the PanZoomController.onTouchStart code).

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

We need to make sure we do subscroller.cancel() earlier on the touch event handling, so this code makes it happen before we even dispatch the event to gecko for processing.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5990b7ced37d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5990b7ced37d

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Would like to see this go into 14 and 15 as well, requesting tracking.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Actually I guess it won't into 14 unless we're doing another point release for that, which I don't think we are.

Comment 13

[Alex Keybl [:akeybl]]

(In reply to Kartikaya Gupta (:kats) from comment #10)
> Would like to see this go into 14 and 15 as well, requesting tracking.

Is this a regression from 14.0? If bug 744518 is the culprit, it wouldn't appear so.

Comment 14

[Cristian Nicolae (:xti)]

This issue was fixed on the latest Nightly build.
Closing bug as verified fixed on:

Firefox 16.0a1 (2012-07-11)
Device: Galaxy Nexus
OS: Android 4.0.4

Comment 15

[Kartikaya Gupta (email:[e-mail])]

(In reply to Alex Keybl [:akeybl] from comment #13)
> Is this a regression from 14.0? If bug 744518 is the culprit, it wouldn't
> appear so.

I'm not sure what you mean by "regression from 14.0". This bug occurs in 14 (all released versions), 15, and 16. We didn't catch it before shipping but I think it's severe enough to uplift to any more releases we do on 14.

Comment 16

[Alex Keybl [:akeybl]]

(In reply to Kartikaya Gupta (:kats) from comment #15)
> (In reply to Alex Keybl [:akeybl] from comment #13)
> > Is this a regression from 14.0? If bug 744518 is the culprit, it wouldn't
> > appear so.
> 
> I'm not sure what you mean by "regression from 14.0".

Is this bug present on 14.0, or has it regressed since? It sounds like it was present on 14.0. Given that, the fact that this hasn't been a major pain point for users, and we're past our last planned beta, I'm wontfixing for FF14.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Yeah, makes sense.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 640248 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 744518
User impact if declined: panning subdocuments (iframes, overflow:scroll, etc) on pages which have touch event listeners doesn't work
Testing completed (on m-c, etc.): verified on m-c
Risk to taking this patch (and alternatives if risky): mobile-only. i'd say low risk but this code is fairly brittle and hard to get right.
String or UUID changes made by this patch: none

Comment 19

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/32af216468ee

Comment 22

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fd673fa4d1d7

Comment 23

[Kartikaya Gupta (email:[e-mail])]

Apparently I'm dsylexic and landed the above patch with the wrong bug number. fd673fa4d1d7 actually belongs to bug 771575, not this bug.