Handle popup notifications with no actions

VERIFIED FIXED in Firefox 53

Status

()

defect
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Paolo, Assigned: johannh)

Tracking

unspecified
Firefox 53
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox53 verified)

Details

(Whiteboard: [fxprivacy])

Attachments

(2 attachments)

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.
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/
Flags: needinfo?(philipp)
Flags: needinfo?(mjaritz)
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.
Flags: needinfo?(philipp)
Flags: needinfo?(mjaritz)
Priority: -- → P2
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Blocks: 1319419
Duplicate of this bug: 1319471
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.
> 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. :)
Attachment #8813670 - Flags: review?(past)
(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 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.
Attachment #8813670 - Flags: review?(paolo.mozmail) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/375051992261
Handle popup notifications with no actions. r=Paolo
Duplicate of this bug: 1319447
https://hg.mozilla.org/mozilla-central/rev/375051992261
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Duplicate of this bug: 1320407
Flags: qe-verify+
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/ )
Flags: needinfo?(jhofmann)
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.
Flags: needinfo?(jhofmann) → needinfo?(mjaritz)
(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)
Flags: needinfo?(mjaritz)
Posted video 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)
Flags: needinfo?(mjaritz)
(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.
Flags: needinfo?(mjaritz)
(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?
Flags: needinfo?(mjaritz)
Please file followup bugs when we don't intend to back out a patch. This makes tracking our work easier.
Flags: needinfo?(mjaritz)
Depends on: 1331918
Sure, filed bug 1331918.
Bases on Comments 17 and 18 and on the fact Bug 1331918 was fixed and verified, setting status-firefox53: to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.