1315236 - Handle popup notifications with no actions

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P2)

Description

[:Paolo Amadini]

We should determine how a popup notification should look when PopupNotifications.show is invoked with no actions. For example, this is currently done for the completion notification of restartless add-ons.

Comment 1

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

Philipp or Markus, can one of you weigh in here? The two options I can see are:
1. Hide the button row entirely but keep the x button for closing the prompt.
2. Provide a single OK button (or Dismiss, etc.) to close the notification and remove the x as in all the other new notifications.

The otherwise automatically-downloaded Valence add-on is a good test to see the current behavior:
https://ftp.mozilla.org/pub/labs/fxdt-adapters/

Comment 2

[Markus Jaritz [:designakt] (UX)]

Option 2 is what we implemented as a confirmation for installs via disco pane, so I would recommend doing the same thing here. Single "OK!" Button, no X, and the panel is also dismissed on click outside of it.

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Bug 1315236 - Handle popup notifications with no actions.

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

Comment 5

[Johann Hofmann [:johannh]]

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

Comment 6

[:Paolo Amadini]

Comment on attachment 8813670 [details]
Bug 1315236 - Handle popup notifications with no actions.

https://reviewboard.mozilla.org/r/95080/#review95244

::: browser/base/content/browser-addons.js:239
(Diff revision 1)
>      var notificationID = aTopic;
>      // Make notifications persistent
>      var options = {
>        displayURI: installInfo.originatingURI,
>        persistent: true,
> +      hideClose: true,

I'm not sure whether we decided to remove the "X" in the add-on installation flow. On the other hand, I see no harm in removing it.

::: toolkit/locales/en-US/chrome/global/notification.dtd:12
(Diff revision 1)
>  <!ENTITY checkForUpdates "Check for updates…">
>  
>  <!ENTITY learnMore "Learn more…">
> +
> +<!ENTITY defaultButton.label "OK!">
> +<!ENTITY defaultButton.accesskey "o">

I believe this needs to be uppercase.

Comment 7

[Johann Hofmann [:johannh]]

> I'm not sure whether we decided to remove the "X" in the add-on installation flow. On the other hand, I see no harm in removing it.

From comment 2:

> Single "OK!" Button, no X, and the panel is also dismissed on click outside of it.

That's what I went with. There is still an X in the confirmation popup (do you really want to install this addon?) but I wrote to the addon folks that they might want to consider removing that one as well. I think that should be covered in a different bug though, since the user implications are a bit bigger than for this one.

> I believe this needs to be uppercase.

Indeed, good catch. :)

Comment 8

[Johann Hofmann [:johannh]]

Comment on attachment 8813670 [details]
Bug 1315236 - Handle popup notifications with no actions.

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

Comment 9

[:Paolo Amadini]

(In reply to Johann Hofmann [:johannh] from comment #7)
> > Single "OK!" Button, no X, and the panel is also dismissed on click outside of it.
> 
> That's what I went with.

I thought this referred only to the popup with no buttons that are dismissed when clicking outside them, but as I said it probably works for the others too.

Comment 10

[:Paolo Amadini]

Comment on attachment 8813670 [details]
Bug 1315236 - Handle popup notifications with no actions.

https://reviewboard.mozilla.org/r/95080/#review95306

Thanks!

::: toolkit/themes/shared/popupnotification.inc.css:107
(Diff revision 2)
> +.popup-notification-button[anonid="secondarybutton"][hidden="true"] ~ .popup-notification-button[default] {
> +  flex: 1;
> +}
> +

I still slightly prefer the default button to be half the width, but if you expand it to take all the available space I guess you can remove the "justify-content: flex-end;" property on the parent.

Comment 11

[Johann Hofmann [:johannh]]

Comment on attachment 8813670 [details]
Bug 1315236 - Handle popup notifications with no actions.

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

Comment 12

[Johann Hofmann [:johannh]]

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/375051992261
Handle popup notifications with no actions. r=Paolo

Comment 15

[Phil Ringnalda (:philor)]

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

Comment 17

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170115030210

Verified that a completion popup notification is displayed for restartless add-ons (after installing a restartless add-on from addons.mozilla.org a panel with single "OK!" Button is displayed, no X, and the panel is dismissed when clicking the OK! button or when clicking outside of the panel). 

Tested on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.11.

But during testing, 2 questions came up:

1. Is it intended for the completion popup notification to be dismissed when changing the focus to a different tab? (for example, right after you install the Adblock Plus add-on you are redirected to the Adblock Plus first run page in a new tab and you only get to see a glance of the completion popup notification)

2. When an add-on can't be verified, the completion popup notification can't be dismissed by clicking outside the panel. Is this the intended behavior? (situation encountered when installing an unverified .xpi from: https://ftp.mozilla.org/pub/labs/fxdt-adapters/win32/ )

Comment 18

[Johann Hofmann [:johannh]]

Thanks for testing!

(In reply to Simona B [:simonab ] from comment #17)
> Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0
> Build ID: 20170115030210
> 
> Verified that a completion popup notification is displayed for restartless
> add-ons (after installing a restartless add-on from addons.mozilla.org a
> panel with single "OK!" Button is displayed, no X, and the panel is
> dismissed when clicking the OK! button or when clicking outside of the
> panel). 
> 
> Tested on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.11.
> 
> But during testing, 2 questions came up:
> 
> 1. Is it intended for the completion popup notification to be dismissed when
> changing the focus to a different tab? (for example, right after you install
> the Adblock Plus add-on you are redirected to the Adblock Plus first run
> page in a new tab and you only get to see a glance of the completion popup
> notification)
> 

Yes, at least that's how I implemented it on purpose.

> 2. When an add-on can't be verified, the completion popup notification can't
> be dismissed by clicking outside the panel. Is this the intended behavior?
> (situation encountered when installing an unverified .xpi from:
> https://ftp.mozilla.org/pub/labs/fxdt-adapters/win32/ )

I'm not sure to be honest, it should probably also auto-dismiss. let's ask Markus.

Comment 19

[Markus Jaritz [:designakt] (UX)]

(In reply to Johann Hofmann [:johannh] from comment #18)
> Thanks for testing!
> 
> (In reply to Simona B [:simonab ] from comment #17)
> > Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0
> > Build ID: 20170115030210
> > 
> > Verified that a completion popup notification is displayed for restartless
> > add-ons (after installing a restartless add-on from addons.mozilla.org a
> > panel with single "OK!" Button is displayed, no X, and the panel is
> > dismissed when clicking the OK! button or when clicking outside of the
> > panel). 
> > 
> > Tested on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.11.
> > 
> > But during testing, 2 questions came up:
> > 
> > 1. Is it intended for the completion popup notification to be dismissed when
> > changing the focus to a different tab? (for example, right after you install
> > the Adblock Plus add-on you are redirected to the Adblock Plus first run
> > page in a new tab and you only get to see a glance of the completion popup
> > notification)
> 
> Yes, at least that's how I implemented it on purpose.

I think it is fine as a first version, but it would be an improvement if people could get back to our confirmation message with closing the automatically opened tab. (I worry that people might see it as a glitch, or are confused by the dialog that appears but does not allow for time to be read. )

> > 2. When an add-on can't be verified, the completion popup notification can't
> > be dismissed by clicking outside the panel. Is this the intended behavior?
> > (situation encountered when installing an unverified .xpi from:
> > https://ftp.mozilla.org/pub/labs/fxdt-adapters/win32/ )
> 
> I'm not sure to be honest, it should probably also auto-dismiss. let's ask
> Markus.

I can't reproduce this. For me it automatically dismisses when clicking outside. Which is what should happen. (Win10)

Comment 20

[Simona Badau, Desktop QA]

Attachment: popupCompletion.mp4

(In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #19)
> I can't reproduce this. For me it automatically dismisses when clicking
> outside. Which is what should happen. (Win10)

Markus, please see the attached screen cast. (Nightly 53.0a1, Build ID: 20170116030326)

Comment 21

[Markus Jaritz [:designakt] (UX)]

(In reply to Simona B [:simonab ] from comment #20)
> Created attachment 8827353 [details]
> popupCompletion.mp4
> 
> (In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #19)
> > I can't reproduce this. For me it automatically dismisses when clicking
> > outside. Which is what should happen. (Win10)
> 
> Markus, please see the attached screen cast. (Nightly 53.0a1, Build ID:
> 20170116030326)

Thanks. That does not look like the behavior we want. It definitely should auto-dismiss.

Comment 22

[Simona Badau, Desktop QA]

(In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #21)
> (In reply to Simona B [:simonab ] from comment #20)
> > Created attachment 8827353 [details]
> > popupCompletion.mp4
> > 
> > (In reply to Markus Jaritz [:designakt prev :maritz] (UX) from comment #19)
> > > I can't reproduce this. For me it automatically dismisses when clicking
> > > outside. Which is what should happen. (Win10)
> > 
> > Markus, please see the attached screen cast. (Nightly 53.0a1, Build ID:
> > 20170116030326)
> 
> Thanks. That does not look like the behavior we want. It definitely should
> auto-dismiss.

Markus, would you prefer me to open a new bug for this behavior or to Reopen this one?

Comment 23

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

Please file followup bugs when we don't intend to back out a patch. This makes tracking our work easier.

Comment 24

[Simona Badau, Desktop QA]

Sure, filed bug 1331918.

Comment 25

[Simona Badau, Desktop QA]

Bases on Comments 17 and 18 and on the fact Bug 1331918 was fixed and verified, setting status-firefox53: to verified.