Closed Bug 426714 Opened 14 years ago Closed 13 years ago

Style nsIAlertsService alerts on Linux

Categories

(Firefox :: Theme, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.6a1

People

(Reporter: faaborg, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Keywords: fixed1.9.1, polish, Whiteboard: [polish-easy][polish-visual][polish-p2])

Attachments

(4 files)

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.
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...
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?
(In reply to comment #2)
Because it's not a tooltip. See bug 426712 and bug 426713 for related discussions.
Summary: Style "system tray" notifications on Linux → Style nsIAlertsService alerts on Linux
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.
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.
Bug 404738 is really the only way to fix this properly though.
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 :) .
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...
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).
this bug is eligible for bug 462081
Keywords: polish
Whiteboard: [polish-easy][polish-visual]
Whiteboard: [polish-easy][polish-visual] → [polish-easy][polish-visual][polish-high-visibility]
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.
Any takers? I'll review quickly!
Attached image a
Attachment #350218 - Flags: ui-review?(faaborg)
Attached image b
Attachment #350219 - Flags: ui-review?(faaborg)
/me votes for b, even if it is tooltip-styled though
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?
(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.
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.
B looks the most similar to other system tray alerts, at least on Ubuntu. I believe system tray notifications also use the tooltip colours.
...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.
Making the whole alert clickable is bug 342261.
(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?
(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...
(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.
(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.
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 on attachment 350218 [details]
a

see comment #26
Attachment #350218 - Flags: ui-review?(faaborg) → ui-review+
Attachment #350219 - Flags: ui-review?(faaborg) → ui-review-
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
(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
Attached patch patchSplinter Review
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;
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #351207 - Flags: review?(ventnor.bugzilla)
Attachment #351207 - Flags: review?(ventnor.bugzilla) → review+
Comment on attachment 351207 [details] [diff] [review]
patch

Looks good, thanks.
http://hg.mozilla.org/mozilla-central/rev/9e3d35aea6ee
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b3
Target Milestone: Firefox 3.1b3 → Firefox 3.2a1
Attachment #351207 - Flags: approval1.9.1?
Attachment #351207 - Flags: approval1.9.1? → approval1.9.1+
Keywords: checkin-needed
Blocks: 418044
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.
Whiteboard: [polish-easy][polish-visual][polish-high-visibility] → [polish-easy][polish-visual][polish-p2]
You need to log in before you can comment on or make changes to this bug.