The default bug view has changed. See this FAQ.

Add a "not now" choice to all doorhanger notification split buttons

VERIFIED FIXED

Status

()

Firefox
General
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: faaborg, Assigned: Margaret)

Tracking

(Blocks: 1 bug, {late-l10n})

unspecified
late-l10n
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [strings][target-betaN][doorhanger])

Attachments

(1 attachment, 4 obsolete attachments)

We would like to teach users that doorhanger notifications are ignorable, and that dismissing them is actually the equivalent of "not now" (as opposed to "no").  We should add a "Not now" command to the split button of every doorhanger (even if it otherwise only has a single command), which contains explanatory text in the spot that usually contains keyboard shortcuts:

[action][v]
Action
---------------------------------
Not now    Click outside to close

This way users can both have access to a way of specifically indicating not now, and in the process they can also pick up the message that it is actually more efficient to simply ignore the message.
(Assignee)

Updated

6 years ago
Assignee: nobody → margaret.leibovic

Comment 1

6 years ago
I think having a separate "not now" button rather than adding it to the menu would be much more discoverable. Some less savvy users may not know to view the menu and only see the split button as a single button. In this instance I would relegate the "Click outside to close" to the tooltip, as that information isn't as directly necessary if the user does have a direct "not now" button.
(Reporter)

Comment 2

6 years ago
Note that we are going to use this our our only approach for "not now" to see how well it works.  Worst case scenario we move forward with bug 615315 as well.
(Reporter)

Comment 3

6 years ago
>I think having a separate "not now" button rather than adding it to the menu
>would be much more discoverable.

If we present the user with two buttons, they are going to interpret it as a forced choice modal dialog (even though it isn't actually forced choice, and it isn't actually modal).  Also, they are more likely to think "I should actually read and decide what I want to do with this" when we are actually hoping that they ignore the prompt in certain situations where the user wasn't expecting it (random forum web site would like to know your physical location, etc.)
(Assignee)

Comment 4

6 years ago
Created attachment 493858 [details] [diff] [review]
WIP

This is going to require updating the tests in browser_popupNotification.js and get approval for string changes. I'll let Alex fight for the strings if this is really what we want to do ;)
Attachment #493858 - Flags: feedback?(gavin.sharp)
(Reporter)

Comment 5

6 years ago
>[Strings]

UI change based on the feedback coming in.  It's not ideal that we have to break string freeze to add this, but I think it does improve the usability of the control.
blocking2.0: --- → ?
Whiteboard: [strings]

Comment 6

6 years ago
(In reply to comment #3)
I see your point. The problem is "click outside to close" is an alien concept to the bulk of users and unfortunately even split buttons are so to many as well. Most users will probably need bug 615315 in some form. (see also bug 615315 comment 2)

It might be good to have both, such that if the user does select the menu they see the explicit choice to ignore always being available.
Comment on attachment 493858 [details] [diff] [review]
WIP

Did you consider adding the item to notification.xml#popup-notification's anonymous content, rather than adding it dynamically? I think that might be simpler.

That said, I'm not really happy about forcing this on all popup-notifications. It places some weird constraints on the general notification API and probably isn't something we'd be able to undo later. I'm also not sure it would be a very effective teaching aid... Explanatory-text-in-accelText just seems kind of strange, and it seems to me that users confused by the interaction model are likely to also be unfamiliar with menu-buttons, which would hurt discoverability. I'd be much more comfortable with bug 615315, which I don't think suffers from any of those problems.
Attachment #493858 - Flags: feedback?(gavin.sharp) → feedback-
(Assignee)

Comment 8

6 years ago
(In reply to comment #7)
> Comment on attachment 493858 [details] [diff] [review]
> WIP
> 
> Did you consider adding the item to notification.xml#popup-notification's
> anonymous content, rather than adding it dynamically? I think that might be
> simpler.

I agree that sounds like a better idea. I'm not sure why I decided not to do that.

> That said, I'm not really happy about forcing this on all popup-notifications.
> It places some weird constraints on the general notification API and probably
> isn't something we'd be able to undo later. I'm also not sure it would be a
> very effective teaching aid... Explanatory-text-in-accelText just seems kind of
> strange, and it seems to me that users confused by the interaction model are
> likely to also be unfamiliar with menu-buttons, which would hurt
> discoverability. I'd be much more comfortable with bug 615315, which I don't
> think suffers from any of those problems.

The mockup in Alex's bug tracker mockup (http://people.mozilla.com/~faaborg/files/firefox4Mockups/polishNotifications-i1/polishNotifications-i1.htm) adds more to the menu popup than my patch, and I think that might make the user's options clearer. I don't think it will be too hard to make a patch, so maybe this is one of those things that we can get in front of nightly users and see what they think?
(Assignee)

Comment 9

6 years ago
Created attachment 494046 [details] [diff] [review]
WIP v2

This patch adds the close menuitem to the binding. However, the dynamic menuitems are being inserted after it when the notification is re-opened (they are inserted before it the first time the popup appears). Do you know what xbl issue would cause this?
Attachment #493858 - Attachment is obsolete: true
Attachment #494046 - Flags: feedback?(gavin.sharp)
(Assignee)

Updated

6 years ago
Blocks: 615441
(Assignee)

Comment 10

6 years ago
Created attachment 494889 [details] [diff] [review]
patch

I haven't tested how this looks on Linux yet. 

Alex, do we want the "x" icon in the menuitem on Linux? I didn't include it on OSX, since other bugs have talked about how there should not be icons in menuitems on OSX.
Attachment #494046 - Attachment is obsolete: true
Attachment #494889 - Flags: review?(gavin.sharp)
Attachment #494046 - Flags: feedback?(gavin.sharp)
Linux yes, OS X no.
(Assignee)

Updated

6 years ago
Whiteboard: [strings] → [strings][needs review gavin]
(Assignee)

Comment 12

6 years ago
Oops, I used the wrong icon url in the gnomestripe css. However, I still can't get the icon to appear in the menu, and I'm not seeing other menuitem icons appear. Is there some pref that shows/hides menuitem icons on Linux?
(In reply to comment #12)
> Oops, I used the wrong icon url in the gnomestripe css. However, I still can't
> get the icon to appear in the menu, and I'm not seeing other menuitem icons
> appear. Is there some pref that shows/hides menuitem icons on Linux?

Yes, that is a gnome system pref. They are off by default. I think there is a checkbox under the Appearance Preferences > Interface tab.
(Assignee)

Comment 14

6 years ago
Created attachment 495572 [details] [diff] [review]
patch (with updated gnomestripe styles)

I got rid of the color: graytext; style for the gnomestripe acceltext because it was impossible to read when highlighed. I could also set a [_moz-menuactive="true"] color style to make it readable, but I feel like Linux themes vary so much that we're best off if we just use the default system colors.
Attachment #494889 - Attachment is obsolete: true
Attachment #495572 - Flags: review?(gavin.sharp)
Attachment #494889 - Flags: review?(gavin.sharp)
>I could also set a
>[_moz-menuactive="true"] color style to make it readable, but I feel like Linux
>themes vary so much that we're best off if we just use the default system
>colors.

Given the placement on a second line, it's going to look like the main command if we don't provide some way of de-emphasizing it.  Let's go with the [_moz-menuactive="true"] approach.
Whiteboard: [strings][needs review gavin] → [strings][needs review gavin][target-betaN]
blocking2.0: ? → betaN+
(Assignee)

Comment 16

6 years ago
Johnath, this has strings. Should it block beta9?
blocking2.0: betaN+ → beta9+
(Assignee)

Comment 17

6 years ago
(In reply to comment #15)
> >I could also set a
> >[_moz-menuactive="true"] color style to make it readable, but I feel like Linux
> >themes vary so much that we're best off if we just use the default system
> >colors.
> 
> Given the placement on a second line, it's going to look like the main command
> if we don't provide some way of de-emphasizing it.  Let's go with the
> [_moz-menuactive="true"] approach.

What color should we do for the "Click Outside to Close" text when it's highlighted? Using -moz-menubarhovertext will guarantee it's readable, but then it will be the same color as the "Not Now" text.
(Assignee)

Comment 18

6 years ago
Created attachment 497923 [details] [diff] [review]
patch v2

After talking with Alex, we decided to get rid of the "Click Outside to Close" text.
Attachment #495572 - Attachment is obsolete: true
Attachment #497923 - Flags: review?(gavin.sharp)
Attachment #495572 - Flags: review?(gavin.sharp)
Attachment #497923 - Flags: review?(gavin.sharp) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [strings][needs review gavin][target-betaN] → [strings][can land][target-betaN]
Keywords: late-l10n
(Assignee)

Comment 19

6 years ago
http://hg.mozilla.org/mozilla-central/rev/545fcdd02090
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [strings][can land][target-betaN] → [strings][target-betaN]

Comment 20

6 years ago
Sorry if this is the wrong place to discuss about this, but this makes it impossible to have notifications that are just used for informing about something that has happened. I mean notification with only an OK-button or something similar. The choices now are either to have an OK-button, with "Not now" -option (doesn't make much sense to "not now" on something that has already happened) OR no button's at all which means no way to dismiss the panel, unless user knows to click outside of it, and no way to completely remove the notification.
Tuomas: that's a good point. Can you file that as a separate bug, and CC me?

Comment 22

6 years ago
Done. Bug 622969

Comment 23

6 years ago
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
No longer blocks: 615441
blocking2.0: beta9+ → betaN+
Keywords: late-l10n

Comment 24

6 years ago
(In reply to comment #23)
Christian, you somehow wiped the entire CC list for this bug... fixing...

Updated

6 years ago
Blocks: 615441
Keywords: late-l10n

Updated

6 years ago
Depends on: 626793
Whiteboard: [strings][target-betaN] → [strings][target-betaN][doorhanger]
Status: RESOLVED → VERIFIED
(Assignee)

Updated

6 years ago
Blocks: 638568

Comment 25

6 years ago
Could this be the cause of the regression in bug 650247? The geolocation API no longer triggers the error handler when selecting "Now now" since Firefox 4. (Or a recent Aurora, for that matter. Build ID 20110704042003.)

Related: bug 591745.
You need to log in before you can comment on or make changes to this bug.