Restart message looks unstyled

RESOLVED FIXED in Thunderbird 13.0

Status

Thunderbird
Theme
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: andreasn, Assigned: Paenglab)

Tracking

Trunk
Thunderbird 13.0
x86_64
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
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

6 years ago
We have the Firefox-style message already. Maybe the notification bar is redundant at this point.
(Assignee)

Comment 2

6 years ago
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

6 years ago
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...)
(Reporter)

Comment 4

6 years ago
Yeah, a doorhanger thing sounds like a good idea!
(Assignee)

Comment 5

6 years ago
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.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #594424 - Flags: ui-review?(nisses.mail)
Attachment #594424 - Flags: review?(nisses.mail)
(Assignee)

Comment 6

6 years ago
Created attachment 594425 [details]
Notification under Win7
(Assignee)

Comment 7

6 years ago
Created attachment 594426 [details]
Notification under OSX
(Assignee)

Comment 8

6 years ago
If desired I can also style the restart button under Windows. Under Linux it is the same appearance.
(Reporter)

Comment 9

6 years ago
(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 :)
(Reporter)

Comment 10

6 years ago
notification[value="addon-install-complete"] seems pretty unique on the other hand.
(Assignee)

Comment 11

6 years ago
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.
Attachment #594424 - Attachment is obsolete: true
Attachment #594424 - Flags: ui-review?(nisses.mail)
Attachment #594424 - Flags: review?(nisses.mail)
Attachment #597867 - Flags: ui-review?(nisses.mail)
Attachment #597867 - Flags: review?(nisses.mail)
(Reporter)

Comment 12

6 years ago
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?
(Reporter)

Comment 13

6 years ago
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+
Attachment #597867 - Flags: ui-review?(nisses.mail) → ui-review+
(Assignee)

Comment 14

6 years ago
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.
Attachment #597867 - Attachment is obsolete: true
Attachment #597867 - Flags: review?(nisses.mail)
Attachment #598306 - Flags: ui-review+
Attachment #598306 - Flags: review?(nisses.mail)
(Reporter)

Comment 15

6 years ago
Comment on attachment 598306 [details] [diff] [review]
Make the notification look as a part of the page v3

css looks good!
Attachment #598306 - Flags: review?(nisses.mail) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/9a19110c96cc
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
You need to log in before you can comment on or make changes to this bug.