Last Comment Bug 771757 - Subdocument scrolling is broken on any page with touch events
: Subdocument scrolling is broken on any page with touch events
Status: VERIFIED FIXED
: regression
Product: Firefox for Android
Classification: Client Software
Component: Graphics, Panning and Zooming (show other bugs)
: 14 Branch
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
:
: away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
Mentors:
: 771750 772427 773696 782055 (view as bug list)
Depends on:
Blocks: google-evangelism
  Show dependency treegraph
 
Reported: 2012-07-06 21:38 PDT by Michael Comella (:mcomella) [not actively working on fennec: expect slow responses]
Modified: 2016-07-29 14:26 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
+
fixed
verified


Attachments
Unscrollable dropdown (Firefox) (133.79 KB, image/png)
2012-07-06 21:38 PDT, Michael Comella (:mcomella) [not actively working on fennec: expect slow responses]
no flags Details
Expanded dropdown (Stock browser - Tablet) (268.13 KB, image/jpeg)
2012-07-06 21:39 PDT, Michael Comella (:mcomella) [not actively working on fennec: expect slow responses]
no flags Details
Patch (4.89 KB, patch)
2012-07-09 09:33 PDT, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
wjohnston2000: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Michael Comella (:mcomella) [not actively working on fennec: expect slow responses] 2012-07-06 21:38:59 PDT
Created attachment 639929 [details]
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) [not actively working on fennec: expect slow responses] 2012-07-06 21:39:39 PDT
Created attachment 639930 [details]
Expanded dropdown (Stock browser - Tablet)
Comment 2 Aaron Train [:aaronmt] 2012-07-08 20:45:18 PDT
Kats/Matt, thoughts?
Comment 3 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-09 07:13:36 PDT
(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 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-09 08:11:57 PDT
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 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-09 08:14:25 PDT
Likely a regression from bug 744518 which made the browser.js Panning:Override code run earlier (before the PanZoomController.onTouchStart code).
Comment 6 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-09 09:33:52 PDT
Created attachment 640248 [details] [diff] [review]
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 7 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-09 09:34:22 PDT
*** Bug 771750 has been marked as a duplicate of this bug. ***
Comment 8 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-10 09:48:35 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5990b7ced37d
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-07-10 20:33:29 PDT
https://hg.mozilla.org/mozilla-central/rev/5990b7ced37d
Comment 10 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-11 07:22:21 PDT
Would like to see this go into 14 and 15 as well, requesting tracking.
Comment 11 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-11 07:23:29 PDT
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 12 :Margaret Leibovic 2012-07-11 11:11:58 PDT
*** Bug 772427 has been marked as a duplicate of this bug. ***
Comment 13 Alex Keybl [:akeybl] 2012-07-11 14:04:57 PDT
(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) 2012-07-12 00:02:59 PDT
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 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-12 06:32:57 PDT
(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] 2012-07-12 13:00:42 PDT
(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 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-12 13:32:27 PDT
Yeah, makes sense.
Comment 18 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-12 13:35:57 PDT
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 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-13 06:23:43 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/32af216468ee
Comment 20 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-13 12:28:31 PDT
*** Bug 773696 has been marked as a duplicate of this bug. ***
Comment 21 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-07-16 13:09:47 PDT
*** Bug 773696 has been marked as a duplicate of this bug. ***
Comment 22 Ed Morley [:emorley] 2012-08-08 09:33:55 PDT
https://hg.mozilla.org/mozilla-central/rev/fd673fa4d1d7
Comment 23 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-08-08 09:47:34 PDT
Apparently I'm dsylexic and landed the above patch with the wrong bug number. fd673fa4d1d7 actually belongs to bug 771575, not this bug.
Comment 24 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2012-08-13 08:44:07 PDT
*** Bug 782055 has been marked as a duplicate of this bug. ***

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