1320406 - Add-on installation's "Restart Now" notification no longer has a "Not Now" button.

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P2)

Description

[Chris Peterson [:cpeterson]]

STR:
1. In Nightly 53, load https://addons.mozilla.org/en-US/firefox/addon/pinboard-extension/
2. Click the page's "Add to Firefox" button.
3. Click the popup notification's "Install" button.

RESULT:
Firefox will show a "Restart Now" popup notification but there is no "Now Now" button. "Not Now" was a drop-down option on the old popup notification. You can click the popup's tiny X button, but as obvious as a the previous "Not Now" option.

Comment 1

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

Markus, what are your thoughts on this? The current options to dismiss the popup if the user doesn't want to restart are to click the X, click outside the doorhanger or hit the Esc key. Should we add a Not Now button to the doorhanger as well?

Comment 2

[Markus Jaritz [:markus] (UX)]

If it is not a lot of effort, please add it as a secondary button to the dialog.
If it is a lot of work, we can live without it, as extensions that require a restart will gradually become less and less.

Comment 3

[Johann Hofmann [:johannh]]

Attachment: Bug 1320406 - Add a "Not now" button to the addon installation restart notification.

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

Comment 4

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96685998896d

Comment 5

[Florian Quèze [:florian]]

Comment on attachment 8822196 [details]
Bug 1320406 - Add a "Not now" button to the addon installation restart notification.

https://reviewboard.mozilla.org/r/101178/#review101668

Reluctant r+, because:
- no test (but I'm not sure a test would be really helpful; if I was I would r- on this).
- I haven't tested the patch, and I think the secondary action here will remove the notification, which means that clicking "Not Now" takes away the affordance that lets users restart later (I would prefer if the notification was just dismissed and the user could reopen it and click 'restart now' later).

Comment 6

[Johann Hofmann [:johannh]]

Hm good point about the dismissal, I'll take another look.

I can add a test but I'm not really sure it needs one either. :)

Comment 7

[Johann Hofmann [:johannh]]

Taking another look, the addons code actually passes "removeOnDismissal" in its options object with the following explanation: http://searchfox.org/mozilla-central/rev/51aa673e28802fe6ea89f4793721fc55334a6ac8/browser/base/content/browser-addons.js#449

I think I'd like to keep this patch simple and not question their decision here. So I'll go ahead and check this in if you're ok with that.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2b07396e446
Add a "Not now" button to the addon installation restart notification. r=florian

Comment 9

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

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

Comment 10

[Md.Tarikul Islam Oashi]

I have to reproduced this bug with Nightly 53.0a1 on Windows 7 64 bit;

The Bug's fix is verified on Latest Nightly 

Build ID 	20170104030214
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170104]

Comment 11

[Md Maruf Rahman]

I have reproduced this bug with Nightly 53.0a1 on ubuntu 16.04 LTS !

This bug's fix is verified on latest Nightly 

Build ID   :   20170104030214
User Agent :   Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
[bugday-20170104]

Comment 12

[Johann Hofmann [:johannh]]

Thanks! :)

Comment 13

[Mihai Boldan, Desktop QA [:mboldan]]

I've also manage to verify this issue on Firefox 53.0a1 (2017-01-05), under Windows 10x64, Mac OS X 10.12.1, Ubuntu 14.04 x64.
The 'Not Now' button is correctly displayed, but I noticed a discordance between OSs, about the order of the 'Not Now' and 'Restart Now' buttons (see https://i.imgur.com/pW9tSNe.png).  

Should I open a new bug for the issue mentioned above?

Comment 14

[Johann Hofmann [:johannh]]

It's a discordance but it's on purpose. See bug 1315269. So no bug necessary.

Thanks :)

Comment 15

[Surentharan]

[bugday-20170118] Bug verified