1455269 - Switching tabs while an install doorhanger is open breaks the install flow

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Andreea Cupsa [:acupsa], Experiments QA]

Attachment: Switch tabs 2.gif

Version: 61.0a1
Build ID: 20180418230818
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0

[Affected Platforms]:
- Win 10 x64
- Ubuntu 14.04 x64
- Mac 10.13.3
[Steps to reproduce]:
1. Start Firefox latest Nightly and navigate on any website in a tab.
2. Open a new tab and and navigate to https://addons.mozilla.org/en-US/firefox/ page.
3. Navigate to one of the extensions and click on "Add to Firefox" button. 
4. Wait for the "Add" doorhanger to appear and switch to previous page tab.
5. Return to the Add-ons tab and click on "Add" button from displayed doorhanger.
6. Observe the page's behaviour.

[Expected results]:
- A confirmation of the installation doorhanger is being displayed.

[Actual results]:
- The doorhanger is dismissed and the add-on is not installed.

[Regression]:
- This is not reproducible on the Latest Release Firefox version, and after further investigation seems to be caused by a change occured in recent Nightly builds. After searching for regression window, I have found that this was caused by Bug 1446686.

Last good revision: 22cd4c5fcb397d80fc7f693737dc1c827ce61d74
First bad revision: 7e5efe49f034c364260d5f1e0964060693c50d7e
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=22cd4c5fcb397d80fc7f693737dc1c827ce61d74&tochange=7e5efe49f034c364260d5f1e0964060693c50d7e

Comment 1

[Andreea Cupsa [:acupsa], Experiments QA]

@:kmag can you please take a look since you have worked on that issue?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Not a blocker, but not great either.

Comment 3

[Marion Daly [:mdaly]]

David: Triage/Assign?

Comment 4

[Ryan VanderMeulen [:RyanVM]]

David, we've realistically got about two weeks left in this cycle to get this fixed for 61. Is there anybody on your team you can assign to look into this?

Comment 5

[Tomislav Jovanovic :zombie]

I think I have a simple fix for 61, later today or tomorrow.

Comment 6

[Tomislav Jovanovic :zombie]

Attachment: bug 1455269 - Cancel instalation on notification dismissal

Review commit: https://reviewboard.mozilla.org/r/245796/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245796/

Comment 7

[Tomislav Jovanovic :zombie]

Comment on attachment 8979649 [details]
bug 1455269 - Cancel instalation on notification dismissal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245796/diff/1-2/

Comment 8

[Tomislav Jovanovic :zombie]

(In reply to Tomislav Jovanovic :zombie from comment #5)
> I think I have a simple fix for 61, later today or tomorrow.

Calling this a "fix" might be generous, it's a simple workaround for the uplift.  The proper fix is probably something like [1], but I can't figure out the cause of intermittent test failures with that applied, and will investigate outside any time pressure.

Comment 9

[Tomislav Jovanovic :zombie]

> something like [1]

1) https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4452774a509bb7eb45263be4952075daea55182

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8979649 [details]
bug 1455269 - Cancel instalation on notification dismissal

https://reviewboard.mozilla.org/r/245796/#review252330

thanks, don't forget to either set leave-open or file a separate follow-up

::: browser/base/content/test/webextensions/browser_permissions_dismiss.js:7
(Diff revision 2)
> +  let testURI = makeURI("https://example.com/");
> +  Services.perms.add(testURI, "install", Services.perms.ALLOW_ACTION);

The test uses mozAddonManager, which shouldn't need this...

Comment 11

[Tomislav Jovanovic :zombie]

Comment on attachment 8979649 [details]
bug 1455269 - Cancel instalation on notification dismissal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245796/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/755e1ebfde37
Cancel instalation on notification dismissal r=aswan

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/755e1ebfde37

Comment 14

[Tomislav Jovanovic :zombie]

Can we please get QE to verify this doesn't disrupt any existing installation workflow?

Comment 15

[Andreea Cupsa [:acupsa], Experiments QA]

Version: 62.0a1
Build ID: 20180528220216
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0

I have verified this issue on Windows 10 x64, Mac 10.13 and Arch Linux on the latest Nightly(62.0a1), now when navigating to another page tab during an installation flow and returning to the page tab with the installation doorhanger,  the doorhanger is no longer displayed. 

@zombie, is this the intended fix for this issue?

Comment 16

[Tomislav Jovanovic :zombie]

Yes, this is the simple workaround intended for uplift, the full restoration of previous functionality is a followup bug 1464459.

I was just wondering if QA could verify this didn't regress any other existing install workflows.

Comment 17

[Andreea Cupsa [:acupsa], Experiments QA]

This fix did no regress any other existing install workflows. 

Methods tested:

- Go to the Firefox Add-ons site and select the extension or theme that you would like to install, click on the Add to Firefox button and then click on Install Now.
- Open the Add-ons Manager > (Alt + t > a) click on the gear icon (Tools for all add-ons) > Install Add-on From File.. > browse to the add-on > double click the add-on (or select Open) > Install Now
- With Firefox open, hit Ctrl+ o (or Cmd+ o) > browse to the extension > double click the extension (or select Open) > Install Now. 
- Press Alt + f > o > browse to the extension > double click the extension (or select Open) > Install Now. 
- Drag the extension to the "about:addons" page, Extensions section.
- Drag the extension to the instance of Firefox on the taskbar > wait for Firefox to appear > drop onto the browser window > Install Now.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Thanks for the verification, Andreea!

Please request uplift on this when you get a chance, zombie. Thanks :)

Comment 19

[Tomislav Jovanovic :zombie]

Comment on attachment 8979649 [details]
bug 1455269 - Cancel instalation on notification dismissal

> Approval Request Comment
> [Feature/Bug causing the regression]:
Bug 1446686.

> [User impact if declined]:
An uncommon addon install flow leaves installs in a broken state.

> [Is this code covered by automated tests?]:
Yes.

> [Has the fix been verified in Nightly?]:
Yes.

> [Needs manual test from QE? If yes, steps to reproduce]: 
Done.

> [List of other uplifts needed for the feature/fix]:
None.

> [Is the change risky?]:
Somewhat.

> [Why is the change risky/not risky?]:
This workaround is only making explicit that we are already canceling the install on tab switching (from bug 1446686).

> [String changes made/needed]:
None.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8979649 [details]
bug 1455269 - Cancel instalation on notification dismissal

Fix for poor UX when switching tabs with an open install doorhanger. Approved for 61.0b11.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/251c38d653d0