426713 - Style nsIAlertsService alerts on XP and Vista

Status: RESOLVED INVALID

(Firefox :: Theme, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

We should do some css tweaks to improve the appearance of system tray
notifications on XP to make them appear more native.  Probably giving them a
tooltip background, and curved corners to match other system tray notifications
will help.

Comment 1

[Dão Gottwald [:dao]]

Do you think the styling should be any different from bug 426712? If not, we shouldn't fork winstripe further.

(In reply to comment #0)
> We should do some css tweaks to improve the appearance of system tray
> notifications on XP to make them appear more native.  Probably giving them a
> tooltip background, and curved corners to match other system tray notifications
> will help.

Note that our alerts actually aren't /system tray/ notifications. And without the tip, I'm afraid the roundness won't look right. They also contain a link, thus a tooltip background might not feel right.

Comment 2

[Ryan A. C.]

Attachment: Patch v1

I agree with Dão on this one. Microsoft calls these ones "toast popups" because they don't have the arrows, on MSN they're square, on windows media player toolbar song change they're square. I've seen them styled everything from a full 3d border with inset pane (ugly imo), gradient with no border, and the firefox style with 2px flat border (google talk for one). I made this patch to look very neutral to windows so it will look good on vista or XP: window background with 2px windowframe, also changed the link colors to use -moz colors.

One point of interest (separate bug really): almost all of the iterations I've seen have a close button on the top right.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 313921 [details] [diff] [review]
Patch v1

>+  border-right: 2px solid WindowFrame;
>+  border-bottom: 2px solid WindowFrame;
>+  border-top: 2px solid WindowFrame;
>+  border-left: 2px solid WindowFrame;

border: 2px solid WindowFrame;?

I think I'd prefer a -moz-dialog background and threedshadow border, though.

Are you sure that -moz-hyperlinktext is right for this?

Comment 4

[Ryan A. C.]

Attachment: Patch v1.1

I actually thought of that shortly after attaching the patch, I wonder why the original was done that way in the first place?
-moz-hyperlinktext seems to be the user defined color (in the preferences>content>colors) for hyperlinks, which alertText[clickable="true"] seems to define only the hyperlink on the alert, hence the underline part. Also, the old color there was a bright blue that was clearly meant to look like a hyperlink, but it was seemingly chosen as a close-looking color.
-moz-activehyperlinktext completely changes that color from a strange dark-blue on click to (usually) red on click, so it acts like other hyperlinks as well.

I went with Window/WindowFrame partially because of -moz-hyperlinktext, since that's what you'll get if you make a webpage with a link that has no background defined, but really it could go either way, since the hyperlink will likely look good on -moz-Dialog too.

Comment 5

[Ryan A. C.]

Attachment: screenshot of v1.1

That's how it turns out (in vista anyways...)

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #4)
> -moz-hyperlinktext seems to be the user defined color (in the
> preferences>content>colors) for hyperlinks

Yes, preferences > *content*. Therefore I don't think it's right for chrome.

Comment 7

[Ryan A. C.]

I'm not really sure why -moz-hyperlinktext is exposed as the user-specified hyperlink color other than to be used by chrome, because regular pages get the color by not specifying one, and shouldn't be using -moz colors anyways. IMO if the chrome is trying to look like a link then it should look the same as other links in the browser. We all know of course that links are rarely ever default link-blue, but -moz-hyperlinktext is the closest thing we have to a 'native color' unless we do Bug 426732, and since -moz-Dialog and Window are both native colors, we should be using something native for the rest of the colors on the panel.

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

Here is a quick spec

-8 pixel corner radius
-color that is not clippy yellow (perhaps just the dialog background color?)

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #1)
> Do you think the styling should be any different from bug 426712? If not, we
> shouldn't fork winstripe further.

Yes, different corner radius on Vista.

(In reply to comment #2)
> I agree with Dão on this one. Microsoft calls these ones "toast popups"
> because they don't have the arrows, on MSN they're square, on windows media

We basically want this to look like the "thing in the bottom right that tries to get your attention". If you're asserting that square is the right way to go based on other XP applications, I'm willing to discuss that, but I think rounded corners ends up looking a lot nicer and fitting with the other "hey, I want to tell you something!" boites.

The yellow colour has been ruined for everyone, though, so I'm happy to keep a white background here.

I like your patch, mostly, but maybe instead make it more like...

.----------------------------------------------, 8px corner radius
| ICON   _Downloads Complete_                  | 10pt, bold, no underline linke
| ICON                                         |                     
| ICON   All files have finished downloading   | 10pt, normal text
'----------------------------------------------'

and ensure that the entire thing acts as a link target so that clicking anywhere in it takes the action.

Comment 10

[Ryan A. C.]

A small snag. We can certainly set the -moz-border-radius on the alertBox, and that works great, but the problem is windows is still providing a square window for us to draw on (or so i'm guessing, because I see nothing in XUL to explain it). I set visibility: hidden on alertNotification, and to my surprise I still got the popup container (blank, but still there)
It appears we're unable to make the window invisible/rounded corners.
Even if we set -moz-appearance: tooltip on the alertNotification the background panel still persists. 

Really, we should be doing SetWindowRgn on windows that define -moz-border-radius on window objects, but that's a little out of scope of this bug I think (can we even do that?) I'm not familiar enough with the code involved in window creation, but I think if we want to make the windows have rounded corners it's going to be a fairly large patch that we might not even want to take for 1.9, unless I'm just missing something here.

Comment 11

[Ryan A. C.]

hmm, could this be why I can't set hidechrome="true"
http://mxr.mozilla.org/mozilla/source/toolkit/components/alerts/src/nsAlertsService.cpp#145
"chrome,dialog=yes,titlebar=no,popup=yes"

Comment 12

[Ryan A. C.]

So I've narrowed it down to popup=yes
It seems to be incompatible with background-color: transparent
Problem with that of course is that the notification is not nearly as useful if it's not popup, so we can't change that. Instead I suppose we need to fix the bug that doesn't allow us to set transparent on a popup.

Comment 13

[Ryan A. C.]

Attachment: Everything but -moz-border-radius

-If we want the -moz-border-radius: Bug 428686
-All is clickable, I renamed a variable there to make it more appropriate to the fact that the entire alert is now clickable.
-I gave the alertTitle the active state, because I found it was nice to have a tactile feedback on that. It might even be nice to do a hover state, but I don't know what to use for that.
-This particular patch uses -moz-hyperlinktext, but we would probably be better off with -moz-nativelinktext from Bug 426732
-I added some left padding to the image there, the box looked kind of wrong without it.
-I went with background-color: Menu and a 1px ThreeDShadow on this one to look like the identity information window, but that could go either way.

Once this one is in the vista changes will be trivial.

I'm probably going to file a followup RFE for making the alert mouse-sticky with close box and dismissible on right-click (or I could do it in this bug, since I'm already touching the files anyways, and it's part of 'styling' it) This would make it act a little more like system native interactable alerts such as attachment 314655 [details], I don't know if that will make it for 1.9, but it doesn't seem like an overly invasive patch.

Comment 14

[Ryan A. C.]

Attachment: Improved nsIAlertsService alerts WIP

so, I decided to see just how much work it would be to do this, and surprisingly it's not bad.
-makes them mouse-sticky* and they unstick to a reset timer.
-adds a close button
-dismisses them on the right-click

For both the close button and the right click I decided on 'instant result' rather than sliding the window down since it seemed more natural that way. Upon thinking about it though, you couldn't slide the window down easily anyways, since the sticky mouse code would pop it back up.

*for some reason it continuously calls onmouseover and onmouseout while the mouse is moving over it, the 2 kind of balance each other out but it seems odd to me, known bug?

It's WIP because: 
-The alertTitle still acts like an active link when you click the close button. I can't think of how to stop this atm, so I'm just posting current progress.
-the close button works great on the download alerts, but I guess there's horizontal alerts (which I have never seen used) and while I think I accommodated for them on the mouse-sticky I don't know what the close button would do on them, nor do I know what would happen to this alert on rtl, but I might guess XUL handles some of that automagically.
-I haven't actually done a full build with these changes, I just modified the ones in the jar files in firefox, I'm building now and will check it when I get up in the morning.
-things like the instant-close which I'm trying for a while to see if I would prefer it to slide away.

Comment 15

[Ryan A. C.]

Attachment: Improved nsIAlertsService alerts WIP v2

Good thing I tested that, messed up an if there.

Comment 16

[Ryan A. C.]

notes/thoughts:
-perhaps I shouldn't remove 'clickable'=true on alertTextLabel to avoid late theme compat?
-consider bumping alerts.totalOpenTime to 5000ms, I've seena lot of comments that it seems 'just a little too short'

for the improvement patch:
-close button should probably have a tooltip, which is late l10n unless Thunderbird's alert close button tooltip is already appropriate?
-rather than sliding out it should fade out, Thunderbird does this already I think?
--^consider that for the simple patch if we don't want the improved one
-onmouseout could be a smaller timeout, maybe half/quarter normal, or just an arbitrary number like 500ms. After using it for a while it feels like the timeout is too long after moving the mouse off, and the mouse leaving the area IMO means the user has stopped interacting with the alert and probably wants it to go away.

Comment 17

[Ryan A. C.]

Attachment: Improved nsIAlertsService alerts v1

Fixes the close button.
Doesn't remove the clickable=true to the alertTextLabel for theme compat
onmouseout timeout time is 1/4 normal.

Decided against fade out, seems kind of odd to pop in then fade out, and I don't think it should fade in. If anyone prefers it to fade (i suppose beltzner would have final say on that) I could make it happen though.

Comment 18

[Ryan A. C.]

Attachment: Screen shot of the Improved nsIAlertsService

The Everything but -moz-border-radius looks the same, just no close box.

Comment 19

[Ryan A. C.]

Attachment: Screen shot of the Improved nsIAlertsService on XP

It occurs to me that my last screenshot was on vista, and of course that looks different. This one has both close.png, and closeSidebar.png screen shots, the X on closeSidebar is moved over a bit for some reason, intentional?

Thunderbird doesn't have a tooltip on their alert close button, and the closest appropriate tooltip text in the code is:
http://lxr.mozilla.org/seamonkey/source/toolkit/locales/en-US/chrome/global/notification.dtd#1
Which of course, it's not necessarily a good idea to use the tooltip text for something it wasn't made for.
Alternatively, we could just remove the close button entirely, I'd be happy with just the right-click dismissal myself.

requesting UI review on this one too, pick whichever you like :)

Comment 20

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: closeSidebar.png

>the X on closeSidebar is moved over a bit for some reason, intentional?

Seems lined up to me, can you take a screen shot of it being moved over a bit?

Comment 21

[Ryan A. C.]

Actually, it seemed to only happen the one time.. I have no idea why it happened, but it was a good 2-3 pixels to the left when I put it on there, and I never did save that screen shot (I replaced that one with the nativelinktext version). The image is small enough one can actually drag one on top of the other and see the that the X button lines up, so it doesn't seem like it moved at all. strange.

Comment 22

[Ryan A. C.]

Some missed polish from 3.0, might as well try for 3.1.

Comment 23

[Alex Faaborg [:faaborg] (Firefox UX)]

this bug is eligible for bug 462081

Comment 24

[Shawn Wilsher :sdwilsh]

gavin and beltzner - any chance of getting your reviews done on this bug?

Comment 25

[Alex Faaborg [:faaborg] (Firefox UX)]

Few notes on the current design:

-I don't think we should be bolding the title, since bold isn't commonly used on either platform

-For Aero use a tooltip background, and menu background on XP

-10pixels of padding seems to small, perhaps 12?  I might play around with these to see what looks right, either way the padding around all sides should be the same as the padding in between the icon and the text.

-We need to use the correct colors for the title for Aero and Luna. The ability to extract them from the system should not block this (but would be nice).

Comment 27

[Alex Faaborg [:faaborg] (Firefox UX)]

uiwanted: new mockups for both XP and Vista

Comment 28

[Alex Faaborg [:faaborg] (Firefox UX)]

Note that previous discussion of Vista was over in bug 426712

Comment 29

[Nochum Sossonko [:Natch]]

I _think_ this bitrotted with bug 342261 's checkin. Also that enabled clicking the whole box.

Comment 30

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 316696 [details] [diff] [review]
Improved nsIAlertsService alerts v1

Unfortunately I wasn't able to review this before it was bitrotten (it no longer applies to the trunk).

The changes look good though, and it should be a quicker review if this patch gets updated again.

Comment 31

[Dão Gottwald [:dao]]

Comment on attachment 317663 [details]
Screen shot of the Improved nsIAlertsService on XP

this is wrong since bug 342261 has been fixed

Comment 32

[Jim Mathies [:jimm]]

A fix for bug 488566, assuming we come up with something that works, would invalidate any work on styling the existing popup. This could probably be marked as a dupe of 488566?

Comment 33

[Dão Gottwald [:dao]]

Depends on how soon we get a fix for bug 488566. As long as it's not fixed, we could still tweak the custom alert.

Comment 34

[Alex Faaborg [:faaborg] (Firefox UX)]

Similar to bug 423744 I'm invalidating in favor of either bug 488566 (in this case) or a new notification UI for Firefox.next that is
currently being worked on by dolske and Matthew Noorenberghe at an API level. 
I'll be posting initial UI mockups to dev.apps.firefox soon.

Comment 36

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 316354 [details] [diff] [review]
Everything but -moz-border-radius

I am the wrong person to ask for this ui-review ...