747023 - Install software notification doorhanger popup can appear from page in background tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Steps to reproduce:
- Long tap on this link and open in new tab: http://people.mozilla.org/~mwargers/tests/dom/multiple_doorhangers.htm

Expected result:
- No notification doorhanger appears

Actual result:
- Install software notification doorhanger appears

Tested on the Samsung Galaxy Nexus, Android4.0.

Comment 1

[Aaron Train [:aaronmt]]

Reproduced (04/30)

Comment 2

[:Margaret Leibovic]

Attachment: patch

Problem here is that we aren't passing a tab id to doorhanger.show(), so it's just picking the current one. This patch just ports over logic from desktop:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#715

(Desktop uses a browser to keep track of doorhangers, we use a tab)

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 619732 [details] [diff] [review]
patch

I assume that we couldn't do something like:

+        let win = installInfo.originatingWindow;
+        let tab = BrowserApp.getTabForWindow(win.top);

If not, I am fine with the docShell based code

Comment 4

[:Margaret Leibovic]

Attachment: simpler patch

(In reply to Mark Finkle (:mfinkle) from comment #3)

> I assume that we couldn't do something like:
> 
> +        let win = installInfo.originatingWindow;
> +        let tab = BrowserApp.getTabForWindow(win.top);

Yeah, this works. I looked into why desktop does this docShell business, and that was added way long ago (2004) before there was an installInfo interface.

Comment 5

[:Margaret Leibovic]

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

Comment 6

[:Margaret Leibovic]

Comment on attachment 619961 [details] [diff] [review]
simpler patch

[Approval Request Comment]
Regression caused by (bug #): n/a
User impact if declined: add-on install doorhanger can appear in wrong tab
Testing completed (on m-c, etc.): just landed on m-c
Risk to taking this patch (and alternatives if risky): low risk, only affects add-on install doorhaner
String changes made by this patch: n/a

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d77293605e32

Comment 8

[Paul Feher]

Verified/Fixed on:
Nightly Fennec 15.0a1 (2012-05-30)
Aurora Fennec 14.0a2 (2012-05-30)
Devices: HTC Desire Z (Android 2.3.3)

Verified using the STR from comment 0.