1335697 - No pop-ups displayed while installing from Addons Manager search list

Status: VERIFIED FIXED

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

Description

[Vasilica Tamas, QA [:vtamas]]

[Affected versions]:
Firefox 53.0a2 (2017-01-31)
Firefox 54.0a1 (2017-01-31)

[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit


[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Create extensions.webextPermissionPrompts and set it to true.
3.Restart the browser.
4.Navigate to about:addons -> Extensions tab.
5.Search for a webextension (e.g Awesome Screenshot) using the search bar displayed in the top right corner of the page.
6.Choose one item from the list and click “Install” button.


[Expected Results]:
The add-ons/webextensions are installed following the new Permissions install flow.


[Actual Results]:
- There are no doorhangers (permissions) displayed while installing from about:addons search list.
- See screencast: https://www.screencast.com/t/pSAv4JA9P

Comment 1

[Andrew Swan [:aswan]]

Attachment: bug 1335697 Display permissions and post-install notifications from about:addons search

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

Comment 2

[Andrew Swan [:aswan]]

I think this is close but I'm mildly stuck on getting the test to pass reliably.  It works locally with two magic setTimeout calls but I'm pretty certain there's a better approach, I just don't know what it is.  Dave, can you take a glance at the XXX comments in the test in the attached patch?  If the issue isn't obvious, I can keep digging, but I'm hoping something will leap right out at you...

Comment 3

[Andrew Swan [:aswan]]

Also, this flow got overlooked when UX did the original install flows.  We don't currently show any doorhangers for installs from the search page, so I opted to only show the post-install notification for webextensions (i.e., not for legacy add-ons).  Markus, can you confirm this is okay?

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8835750 [details]
bug 1335697 Display permissions and post-install notifications from about:addons search

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

Comment 5

[Markus Jaritz [:markus] (UX)]

For WebExtensions we need to make sure to show the permissions the users is agreeing to.
(if reducing the number of different cases, I think Andy filed a bug to replace this search with a re-direct to an AMO search. That might omit that bug altogether.)

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 8835750 [details]
bug 1335697 Display permissions and post-install notifications from about:addons search

https://reviewboard.mozilla.org/r/111368/#review112658

Comment 7

[Andrew Swan [:aswan]]

(In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #5)
> For WebExtensions we need to make sure to show the permissions the users is
> agreeing to.
> (if reducing the number of different cases, I think Andy filed a bug to
> replace this search with a re-direct to an AMO search. That might omit that
> bug altogether.)

Sorry my question wasn't clear, this patch adds permissions prompts for webextensions installed from the search page.  It also adds post-install notifications, but only for webextensions.  I think we're good for webextensions, the question was about legacy add-ons -- I didn't add any post-install notifications for them.  This maintains the current behavior, but it is different than the other install flows (disco pane, AMO, etc) for legacy add-ons that do show a post-install notification.

Comment 8

[Markus Jaritz [:markus] (UX)]

(In reply to Andrew Swan [:aswan] from comment #7)
> the question was about legacy add-ons -- I didn't add any
> post-install notifications for them.  This maintains the current behavior,
> but it is different than the other install flows (disco pane, AMO, etc) for
> legacy add-ons that do show a post-install notification.

If it makes implementation easier this is OK as legacy will not be around for much longer. 
If easy to change, having a similar behavior for every extension would be preferred.

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8835750 [details]
bug 1335697 Display permissions and post-install notifications from about:addons search

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

Comment 10

[Andrew Swan [:aswan]]

Mossop, the version that you was full of leaks in debug builds.  This version is clean on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdf16a5178f542dd34b63279bb8d2c7cefc149e6
But the changes feel a little gross, mind taking a look again, perhaps there's some way to make it a little neater...

Comment 11

[Dave Townsend [:mossop]]

Looks fine to me

Comment 12

[Andrew Swan [:aswan]]

Comment on attachment 8835750 [details]
bug 1335697 Display permissions and post-install notifications from about:addons search

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33d14dcc5ca1
Display permissions and post-install notifications from about:addons search r=mossop

Comment 14

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/ccd16d3e22b6 for a 50% chance of exceeding the timeout threshold on Linux32 debug, https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=33d14dcc5ca1b9fff0ecfc0bd0a12e105f896156&filter-searchStr=9a7d6324891dd0d7bae38b0cd55ae4f604f47647&group_state=expanded

Comment 15

[Andrew Swan [:aswan]]

Comment on attachment 8835750 [details]
bug 1335697 Display permissions and post-install notifications from about:addons search

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/111368/diff/4-5/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b49222f97169
Display permissions and post-install notifications from about:addons search r=mossop

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 18

[Vasilica Tamas, QA [:vtamas]]

Clicking on “Cancel” button will remove the webextension from the search results list. See screencast: https://www.screencast.com/t/jngZ3muVgd
Is this an intended behavior?

Are there any plans to implement install notifications from about:addons search for legacy add-ons too?

Comment 19

[Andrew Swan [:aswan]]

(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #18)
> Clicking on “Cancel” button will remove the webextension from the search
> results list. See screencast: https://www.screencast.com/t/jngZ3muVgd
> Is this an intended behavior?

Yeah, I forgot to mention that, its probably worthy of a follow-up bug but I don't think it is high priority.

> Are there any plans to implement install notifications from about:addons
> search for legacy add-ons too?

This was discussed above, see comment 3, comment 7, comment 8.  In short, no.

Comment 20

[Vasilica Tamas, QA [:vtamas]]

(In reply to Andrew Swan [:aswan] from comment #19)
> Yeah, I forgot to mention that, its probably worthy of a follow-up bug but I
> don't think it is high priority.

Filled Bug 1342350.

> This was discussed above, see comment 3, comment 7, comment 8.  In short, no.

Thanks. I wasn't sure about the conclusion.

Verified fixed on Firefox 54.0a1 (2017-02-26) under Windows 10 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.12.1. Permissions pop-up is successfully displayed while installing from Addons Manager search list: https://www.screencast.com/t/i9dZqJz62