426714 - Style nsIAlertsService alerts on Linux

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

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

Attachment: Screen shot of a notification on Linux

Until we get dbus messaging support working (bug 404738), we should at least style the current notifications (which appear where there would be a system tray on windows) to visually integrate with the surrounding OS.  Currently I believe these notifications look pretty much identical to how they appear on Windows.

Comment 1

[Michael Monreal [:micmon]]

It would be nice to make the border thinner (1px) and perhaps make the whole thing slightly larger (more space between elements) but as for colors, we can't do much because dbus notifications can be themed...

Comment 2

[Ian Spence]

Why not just throw in a -moz-appearance: tooltip, add some margins/padding to separate content, and modify it to use the correct tooltip text color?

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #2)
Because it's not a tooltip. See bug 426712 and bug 426713 for related discussions.

Comment 4

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

I'm not totally familiar with what the correct appearance should be on linux (although odds are it isn't just like what we look like on Windows), so I'll need some feedback from linux users on how we should redesign these.

Comment 5

[Michael Ventnor]

The background colour should be changed to match that of a tooltip (which I believe is InfoBackground) since that matches the alerts on my system. I'm not certain of the padding values. Aside from that, things should be OK.

Comment 6

[Michael Monreal [:micmon]]

Bug 404738 is really the only way to fix this properly though.

Comment 7

[u294409]

yep, libnotify is great idea. and xfce users will not be against, since they have notification-daemon-xfce.

the only group against would be kde users, but qt port is in works - they will have to do this on their own :) .

Comment 8

[Michael Monreal [:micmon]]

Speaking of the Firefox Qt port here I guess? Well, there has been news that this port will be supported officially at some point, but what's the status atm? Is Firefox/Qt likely to be shipped with 3.1? This would probably make things easier here but I don't yet see this happening in the given timeframe...

Comment 9

[u294409]

yep, I was talking about firefox qt port :) .

it will probably be not ready for 3.1 release, but you need to check this on your own or search for some release schedule (if any).

Comment 10

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

this bug is eligible for bug 462081

Comment 11

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

Ok, let's just set the border to 1 pixel, increase the padding between elements (border, between icon and message), and use a native color for the border instead a hard coded border color.

Comment 12

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

Any takers? I'll review quickly!

Comment 13

[Dão Gottwald [:dao]]

Attachment: a

Comment 14

[Dão Gottwald [:dao]]

Attachment: b

Comment 15

[u294409]

/me votes for b, even if it is tooltip-styled though

Comment 16

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

I would like someone on the tango team to make the call between A and B.  Otherwise, we shouldn't really be styling the text as a hyperlink here since we try to make sure that hyperlinks in Firefox always resolve to Web content as opposed to chrome.

Another question for people more familiar with Gnome conventions: is the bold title native-ish, or should we be using a larger font size?

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #16)
> Otherwise, we shouldn't really be styling the text as a hyperlink here

Then the whole box should respond to the click, right? That's a bit beyond this bug.

Comment 18

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

Oh, I thought it already did, but yeah, we would need to make the whole box respond to the click.  Let's do that as a followup.

Comment 19

[Michael Ventnor]

B looks the most similar to other system tray alerts, at least on Ubuntu. I believe system tray notifications also use the tooltip colours.

Comment 20

[Michael Ventnor]

...and if someone could file a bug or already knows of a bug to make the whole box respond to the click, I'll happily take that.

Comment 21

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

Making the whole alert clickable is bug 342261.

Comment 22

[Dão Gottwald [:dao]]

(In reply to comment #19)
> B looks the most similar to other system tray alerts, at least on Ubuntu. I
> believe system tray notifications also use the tooltip colours.

But don't these other alerts just close when clicking on them, and perform the action only when clicking the system tray icon?

Comment 23

[Michael Monreal [:micmon]]

(In reply to comment #19)
> B looks the most similar to other system tray alerts, at least on Ubuntu. I
> believe system tray notifications also use the tooltip colours.

This is not the case. Ubuntu (and only ubuntu iirc) uses a special notification bubble theme. Yeah, _notification bubble theme_. The default theme actually looks closer to proposal A. But there's really not point point in trying a close match as this will break for *some* people in either case.

Personally I don't see a reason to use tooltip color so my vote would be for A, but I don't really feel strongly about it...

Comment 24

[Michael Monreal [:micmon]]

(In reply to comment #22)
> But don't these other alerts just close when clicking on them, and perform the
> action only when clicking the system tray icon?
IIRC that is the default but you can basicly embed any gtk widget (I think) with any associated action into them.

Comment 25

[Dão Gottwald [:dao]]

(In reply to comment #24)
> (In reply to comment #22)
> > But don't these other alerts just close when clicking on them, and perform the
> > action only when clicking the system tray icon?
> IIRC that is the default but you can basicly embed any gtk widget (I think)
> with any associated action into them.

Yeah, but in the end we want bug 342261 rather than a certain widget within the alert.

Comment 26

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

Ok, our friendly linux folks didn't agree on an appearance, so unless anyone else wants to weigh in I suggest we go with A, since I think it looks nicer and saturated tool tip colors generally don't work too well when applied to large areas.  Let's land the update and we can change it again in a follow up if we want to.

Also, I'm under the impression that all of these changes will ultimately be irrelevant if bug 404738 (dbus notifications) lands.

Comment 27

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

Comment on attachment 350218 [details]
a

see comment #26

Comment 28

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

Comment on attachment 350219 [details]
b

see comment #26

Comment 29

[Michael Ventnor]

I'm happy with that, A looks like the current alert on my machine anyway. I never knew Ubuntu had that kind of deviation anyway (comment 23).

/shameless plug: I really need to get gavin to review bug 342261

Comment 30

[Michael Monreal [:micmon]]

(In reply to comment #29)
> I never knew Ubuntu had that kind of deviation anyway (comment 23).

IIRC you can change /apps/notification-daemon/theme to "standard" for the upstream look btw

Comment 31

[Dão Gottwald [:dao]]

Attachment: patch

Here's a diff between winstripe's old alert.css and the new one for gnomestripe:

@@ -44,17 +44,13 @@
 @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
 
 .alertBox {
-  border-right: 2px solid #7B969C;
-  border-bottom: 2px solid #7B969C;
-  border-top: 2px solid #7B969C;
-  border-left: 2px solid #7B969C;
+  border: 1px solid ThreeDShadow;
   background-color: -moz-Dialog;
   min-height: 50px;
 }
 
 .alertBox[orient="horizontal"] > .alertImageBox {
-  -moz-margin-start: 4px;
-  -moz-margin-end: 6px;
+  margin: 0 8px;
   min-height: 46px;
 }
 
@@ -74,14 +70,10 @@
 
 .alertText[clickable="true"] {
   cursor: pointer;
-  color: #1455D6;
+  color: -moz-nativehyperlinktext;
   text-decoration: underline;
 }
 
-.alertText[clickable="true"]:hover:active {
-  color: #424F63;
-}
-
 .alertBox[orient="horizontal"] > .alertTextBox {
   -moz-padding-end: 10px;
   padding-top: 5px;

Comment 32

[Michael Ventnor]

Comment on attachment 351207 [details] [diff] [review]
patch

Looks good, thanks.

Comment 33

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/9e3d35aea6ee

Comment 34

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

Comment on attachment 351207 [details] [diff] [review]
patch

a191=beltzner

Comment 35

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b38557a7f18c

Comment 36

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

This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.