Last Comment Bug 722687 - Restart message looks unstyled
: Restart message looks unstyled
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-31 06:42 PST by Andreas Nilsson (:andreasn)
Modified: 2012-02-21 07:35 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Firefox vs Thunderbird (152.24 KB, image/png)
2012-01-31 06:42 PST, Andreas Nilsson (:andreasn)
no flags Details
Make the notification look as a part of the page (3.38 KB, patch)
2012-02-04 04:19 PST, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
Notification under Win7 (134.35 KB, image/png)
2012-02-04 04:20 PST, Richard Marti (:Paenglab)
no flags Details
Notification under OSX (142.28 KB, image/png)
2012-02-04 04:21 PST, Richard Marti (:Paenglab)
no flags Details
Make the notification look as a part of the page v2 (3.31 KB, patch)
2012-02-16 09:36 PST, Richard Marti (:Paenglab)
bugs: ui‑review+
Details | Diff | Splinter Review
Make the notification look as a part of the page v3 (4.50 KB, patch)
2012-02-17 11:22 PST, Richard Marti (:Paenglab)
bugs: review+
richard.marti: ui‑review+
Details | Diff | Splinter Review

Description Andreas Nilsson (:andreasn) 2012-01-31 06:42:37 PST
Created attachment 593079 [details]
Firefox vs Thunderbird

When installing an addon, the message we're displaying for restarting Thunderbird doesn't look good. It is gray and looks unstyled. This is probably quite easy to fix.

Another option is to have a very nice restart message like Firefox does, but that probably requires some more hacking.
Comment 1 Jim Porter (:squib) 2012-01-31 14:17:40 PST
We have the Firefox-style message already. Maybe the notification bar is redundant at this point.
Comment 2 Richard Marti (:Paenglab) 2012-01-31 23:14:47 PST
I don't think it's redundant. If you are on 'Appearance' and install a 'Extensions' Add-on you wouldn't see a restart message without the notification bar. This also applies to an Add-on not in the visible scroll area.

A problem to style the notification bar could be not having a special ID or class.
Comment 3 Jim Porter (:squib) 2012-01-31 23:25:04 PST
It looks like Firefox uses a doorhanger for this. Maybe we should try to switch to that if it's not too much effort? (Though I seem to recall this being non-trivial...)
Comment 4 Andreas Nilsson (:andreasn) 2012-02-01 06:07:57 PST
Yeah, a doorhanger thing sounds like a good idea!
Comment 5 Richard Marti (:Paenglab) 2012-02-04 04:19:45 PST
Created attachment 594424 [details] [diff] [review]
Make the notification look as a part of the page

I'm using the selector .contentTabInstance[disablechrome] to style the notification. This works for the Add-ons but I don't know if other disablechrome content also uses notifications. If yes, then this approach would fail.

Squib, do know about such content?

If only the Add-ons are using this notification, here is the patch. Under OSX I restyled the reboot button to look similar to the other buttons in this page.
Comment 6 Richard Marti (:Paenglab) 2012-02-04 04:20:23 PST
Created attachment 594425 [details]
Notification under Win7
Comment 7 Richard Marti (:Paenglab) 2012-02-04 04:21:27 PST
Created attachment 594426 [details]
Notification under OSX
Comment 8 Richard Marti (:Paenglab) 2012-02-04 04:24:34 PST
If desired I can also style the restart button under Windows. Under Linux it is the same appearance.
Comment 9 Andreas Nilsson (:andreasn) 2012-02-15 18:32:35 PST
(In reply to Richard Marti [:paenglab] from comment #5)
> Created attachment 594424 [details] [diff] [review]
> Make the notification look as a part of the page
> 
> I'm using the selector .contentTabInstance[disablechrome] to style the
> notification. This works for the Add-ons but I don't know if other
> disablechrome content also uses notifications. If yes, then this approach
> would fail.

Seems it's used in five files right now.
http://mxr.mozilla.org/comm-central/search?string=contentTabInstance
However, I think it probable that others could in the future.
Is the javascript that triggers this something we control in Thunderbird, or is it something we share with Firefox? It might be a good idea to add a class if we can.

Visually it looks excellent, even though I would like a doorhanger notification even more :)
Comment 10 Andreas Nilsson (:andreasn) 2012-02-15 18:46:12 PST
notification[value="addon-install-complete"] seems pretty unique on the other hand.
Comment 11 Richard Marti (:Paenglab) 2012-02-16 09:36:32 PST
Created attachment 597867 [details] [diff] [review]
Make the notification look as a part of the page v2

(In reply to Andreas Nilsson (:andreasn) from comment #10)
> notification[value="addon-install-complete"] seems pretty unique on the
> other hand.

You're right. I changed the '.contentTabInstance[disablechrome] notification' to 'notification[value^="addon-install-"]'. This works then also for incompatible Add-ons etc. Everything else is unchanged.
Comment 12 Andreas Nilsson (:andreasn) 2012-02-16 14:04:34 PST
Some docs on how this selector works: https://developer.mozilla.org/en/CSS/Attribute_selectors
I haven't seen selectors being used like this before in the mozilla tree, it seems more common to print them all out:
http://hg.mozilla.org/mozilla-central/file/78fde7e54d92/browser/themes/gnomestripe/browser.css#l1184 and that makes it slightly easier to read (but also less forward compatible I guess).
How many different variations are there of addon-install-? Is it only 2 more?
Comment 13 Andreas Nilsson (:andreasn) 2012-02-16 15:47:35 PST
Comment on attachment 597867 [details] [diff] [review]
Make the notification look as a part of the page v2

Might be nice with a specific style just for classic, but it doesn't really stick out right now on that theme and in general I like it, so giving the patch ui-review+
Comment 14 Richard Marti (:Paenglab) 2012-02-17 11:22:30 PST
Created attachment 598306 [details] [diff] [review]
Make the notification look as a part of the page v3

Exchanged the 'notification[value^="addon-install-"]' with the three values.
Under Aero I moved the rules to the -moz-windows-default-theme. Now it looks also under classic like under XP.

Carrying over the ui-r from previous patch.
Comment 15 Andreas Nilsson (:andreasn) 2012-02-20 10:09:50 PST
Comment on attachment 598306 [details] [diff] [review]
Make the notification look as a part of the page v3

css looks good!
Comment 16 Mark Banner (:standard8) (afk until 26th July) 2012-02-21 07:35:57 PST
Checked in: http://hg.mozilla.org/comm-central/rev/9a19110c96cc

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