1331918 - The completion popup panel is not auto-dismissed by clicking outside of it

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Comment 1

[Simona Badau, Desktop QA]

Attachment: popupCompletion.mp4

[Affected versions]:
- Nightly 53.0a1

[Affected platforms]:
- All 

[Steps to reproduce]:
1. Install an unverified add-on, you can choose the fist .xpi from:
https://ftp.mozilla.org/pub/labs/fxdt-adapters/win32/
2. After the completion popup panel is displayed, dismiss it by clicking outside the panel

[Expected result]:
- The panel is dismissed 

[Actual result]:
- The panel is not dismissed. Please see the screen-cast.

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Bug 1331918 - Make addon install failure messages non-persistent.

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

Comment 5

[Johann Hofmann [:johannh]]

Comment on attachment 8828252 [details]
Bug 1331918 - Make addon install failure messages non-persistent.

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

Comment 6

[Johann Hofmann [:johannh]]

I'm not sure if this needs tests and I wouldn't be sure where to put them. If you think it does, feel free to r-

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8828252 [details]
Bug 1331918 - Make addon install failure messages non-persistent.

https://reviewboard.mozilla.org/r/105724/#review106628

r=me with the removal of the hunk below or a clarification about its necessity.

::: toolkit/modules/PopupNotifications.jsm:1034
(Diff revision 2)
> +      if (notificationsToShow.some(n => n.options.persistent)) {
> +        this.panel.setAttribute("noautohide", "true");
> +      } else {
> +        this.panel.removeAttribute("noautohide");
> +      }
> +

Why is this necessary? We are calling _showPanel() right above this block, which will eventually run the same code and race with this, since it runs after the _hidePanel() promise has been fulfilled.

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8828252 [details]
Bug 1331918 - Make addon install failure messages non-persistent.

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

Comment 9

[Johann Hofmann [:johannh]]

Yes, you're right, actually _showPanel() is the much better place to put this. I had to do this since on already open panels with updated notifications the noautohide attribute is not updated according to the new panel's specifications. So if you have a persistent panel that becomes a non-persistent one, it would still be incorrectly persistent.

But yeah, that was definitely the wrong place.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61dfd2f2ae3d
Make addon install failure messages non-persistent. r=past

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/61dfd2f2ae3d

Comment 12

[Simona Badau, Desktop QA]

Verified as fixed using the latest Aurora 53.0a2 (Build ID: 20170202084301)and the latest Nightly 54.0a1 (Build ID: 20170202030211) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.