Closed Bug 985867 Opened 11 years ago Closed 11 years ago

Regression: Tapping in the add-ons page opens the same page in a new tab, unable to search for add-ons

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox28 wontfix, firefox29 wontfix, firefox30 verified, firefox31 verified, firefox32 verified, fennec31+)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- verified
firefox31 --- verified
firefox32 --- verified
fennec 31+ ---

People

(Reporter: csuciu, Assigned: wesj)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached file log.txt (deleted) —
Nightly 31.0a1 (2014-03-19) Samsung Galaxy Tab (4.0) Asus Transformer Pad (4.2) HTC Desire HD (2.3) 1. Start Nightly with a clean profile 2. Open add-ons page via Menu > Tools > Add-ons 3. Tap anywhere on the page Result: A new add-ons page opens in a new tab
Last good revision: 8abc76dedec2 (2014-03-01) First bad revision: 0085a162499f (2014-03-02) Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8abc76dedec2&tochange=0085a162499f
I'm not able to reproduce this on my Nexus 5.
I can reproduce this on LG Nexus 4 (Android 4.4.2). For example. I wanted to install "Stylish" and after tapping it, a new tab opened with "addons.mozilla.org/en-US/android". In the end I had over 20 tabs opened and couldn't install it, only after closing the tabs opened.
Flags: needinfo?(teodora.vermesan)
Regression window wanted: good build: 01-03-2014 bad build: 02-03-2014 pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8abc76dedec2&tochange=0085a162499f
Flags: needinfo?(teodora.vermesan)
tracking-fennec: --- → ?
Summary: Tapping in the add-ons page opens the same page in a new tab → Regression: Tapping in the add-ons page opens the same page in a new tab, unable to search for add-ons
Assigning to Wes
Assignee: nobody → wjohnston
Wes - This is already on Aurora. Is bug 973348 to blame? If so, why is it happening with about:addons? Are we doing something in about:addons that triggers this?
The regressionwindow was already found. Please see comment#1 and comment#4
tracking-fennec: ? → 30+
Blocks: 973348
This was not a regression from bug 973348.
No longer blocks: 973348
AFAICT, This is happening in the platform. For some reason the target of the first touchstart is set to this div (even though you are far away from it), which we then use as our touch event target. Trying to figure out why...
Two things here. We're opening a new tab on "click". Something's changed recently so that we're dispatching events as: touchstart mousemove mousedown mouseup touchend That means the click fires before the touchend. Then, when touchend does run the getContentFrame() at: http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7117 returns null. I'm not sure why exactly. Still looking. But that causes it to never really fire and the event isn't removed from our list of pending events. When you return to the page and click again we fire the event (that happens to have the same id) at the same target. I suspect the change in event ordering is because of the new doubletap handling, but I'm not quite sure why. You know kats? Our tests for touchevents this fire events use DOMWindowUtils, so we don't really test this code. We could and should though :)
Flags: needinfo?(bugmail.mozilla)
Ahh, so to reproduce this my STR are: 1.) Open about:apps 2.) Tap the "Browse all Add-ons" row 3.) Wait for the page to load. Hit back. 4.) Tap anywhere on the screen. I can reproduce with those steps even on release builds. I think the problem with click events firing between touchevents has been present ever since we supported quick clicking in pages with user scalable turned off (our GestureDetectors fire before we send the events to Gecko). In onclick, we open a new tab (via BrowserApp.addTab()). The touchend event then fires after that new tab has opened, so it fires in the new tab, which angers the PresShell because the event is trying to fire in a different frame than its target. I think we need to fix this event ordering bug. I can reproduce it with a test page, but 1.) Any touch listeners in the page make it disappear, and 2.) the page uses window.open, which doesn't trigger the same bad behavior in presshell. I'm still not sure why these events fire in order sometimes and out-of-order others. I don't see any way that events can get sent in the right order...
Thanks, that explanation makes sense, somewhat. Within the regression range, it was probably introduced by the backout of bug 960146, because that backout fixed touch listeners. Likely another bug (maybe 941995) which landed while 960146 was still in the tree caused this to happen in this case. But yes, I agree that we need to fix the event ordering. There was a similar issue on B2G a few months ago (bug 965381) where I did the equivalent of setTimeout(,0) so that the touch event was guaranteed to be handled before the click event. We can do that here too.
Flags: needinfo?(bugmail.mozilla)
Attached patch Patch (deleted) — Splinter Review
This posts the click as a new task to the PanZoomTarget (which should post to the UI Thread). I really wanted to post to the "current" thread here, but I think this code should be running on the UIThread, so this is the same.
Attachment #8405533 - Flags: review?(bugmail.mozilla)
Attachment #8405533 - Flags: review?(bugmail.mozilla) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Flags: needinfo?(catalin.suciu)
Keywords: verifyme
I can not reproduce the bug on Nightly 31.0a1 (2014-04-13) using the steps from comment #0 or comment #13 The bug is present on all the other branches and can be reproduced only by following the steps from comment #13.
Status: RESOLVED → VERIFIED
Flags: needinfo?(catalin.suciu)
Depends on: 997289
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(In reply to Wesley Johnston (:wesj) from comment #19) > backed out while I try to repro/fix bug 997289: > https://hg.mozilla.org/integration/fx-team/rev/1c075c91c7bf FYI, this had the wrong bug number in the commit message (bug 985967). If you aren't using the qbackout extension for these kind of things, I recommend you give it a try :)
Backout fixed bug 997289.
Keywords: verifyme
Is this a dupe of bug 890237?
Status: REOPENED → ASSIGNED
Do we have an update here? This is currently on Beta 4.
I am able to reproduce this on Marketplace too at first time I try to install a webapp (make a tap in the page) PS: This was fixed with the previous patch in April 14 that was back out .
> 11:43 sawrubh: are addons supported for Fx-Android? > 11:43 andym: sawrubh: yes > 11:43 sawrubh: I try installing, get directed to the addons site, when I tap in the search box, the page just keeps on reloading > 11:44 sawrubh: also when I do Tools->Addons->Your Addons, I still get directed to the addons website > 11:45 sawrubh: andym: and I'm on the latest Nightly > 11:46 sawrubh: and in fact the page doesn't reload when I tap on the search box on the addons website, it opens a new tab :o User's are running into this still
Any news on this, wes?
Flags: needinfo?(wjohnston)
Comment 13 mentions some things that make this bug go away. We could add a hack to the about:addons page to at least make this issue go away for the one case where it is currently happening. We can keep a testcase around to fix the event ordering. Wes?
An in-page workaround: long-press select the text in the button you're about to click, then tap the arrow on the right hand side. Reliably causes AMO to function for me.
(In reply to Mark Finkle (:mfinkle) from comment #29) > Comment 13 mentions some things that make this bug go away. We could add a > hack to the about:addons page to at least make this issue go away for the > one case where it is currently happening. We can keep a testcase around to > fix the event ordering. > > Wes? The workaround is basically to throw yourself out of the fast-click path. i.e. either Registering touch event listeners means we have to listen for them to reply before clicking or changing the viewport to... whatever it is now that disables fast-clicks. None of them are particularly great. This bug is already shipping on release and has been since NativeFennec 1.0. I'd prefer not to push for a hack to fix it. I let it slip off my radar, but I can bump it up in priority. I haven't been able to reproduce the Google bugs on any devices I use regularly. Digging into my stache today to see if it happens on any of them.
Flags: needinfo?(wjohnston)
Attached patch Hack fix (deleted) — Splinter Review
Here's the hacky fix. Touch event listeners should be slightly faster than just a viewport fix... I think.
Attachment #8430292 - Flags: review?(mark.finkle)
Attachment #8430292 - Flags: review?(mark.finkle) → review+
tracking-fennec: 30+ → 31+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Comment on attachment 8430292 [details] [diff] [review] Hack fix [Approval Request Comment] Bug caused by (feature/regressing bug #): Always. Lucas showed me this today. I think on release it presents slightly differently because of the change in bug 973348. User impact if declined: Clicks just open amo Testing completed (on m-c, etc.): Landed on mc last week. Risk to taking this patch (and alternatives if risky): This is the simplest solution I could come up with. Its very low risk. It basically does nothing, but it does throw us out of our fast click path on about:addons into a slower, but not really slow, path String or IDL/UUID changes made by this patch: none
Attachment #8430292 - Flags: approval-mozilla-beta?
Attachment #8430292 - Flags: approval-mozilla-aurora?
Comment on attachment 8430292 [details] [diff] [review] Hack fix Since it's super confusing to try to hit a text entry field and get a hyper link reaction I think we should take this low risk patch into our final beta.
Attachment #8430292 - Flags: approval-mozilla-beta?
Attachment #8430292 - Flags: approval-mozilla-beta+
Attachment #8430292 - Flags: approval-mozilla-aurora?
Attachment #8430292 - Flags: approval-mozilla-aurora+
Verified as fixed in builds: 32.0a1 (2014-06-03) 31.0a2 (2014-06-03) 30.0b10 Device: Asus Transformer Pad TF300T (Android 4.2.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: