632181 - "Write" button popup menu needs icon adaption to new Thunderbird default theme

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Stefan Sitter [:ssitter]]

Attachment: screenshot of error

Lightning 1.1a1pre (20110206) with Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110207 Thunderbird/3.3a3pre

Steps to reproduce:
1. Create clean Thunderbird profile, install Lightning, restart
2. Check the "Write" button popup menu

Actual results:
The icon for menu entry "Message" is incorrect and should be fixed.

)The icon is correct in Lightning 1.0b3pre with Lanikai/3.1.9pre due to a different Thunderbird default theme.)

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v1

Tested only on linux at the moment (old icon looks fine there). This needs additional testing on windows (to see if aero/non-aero works) and on mac (to see if it compiles and still works)

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Tested on mac, no side-effects.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 511703 [details] [diff] [review]
Fix - v1

Matthew, do you have a Windows Vista or later box you could test this with (both classic theme and aero) ?

Comment 4

[Matthew Mecca [:mmecca]]

Sorry, I only have access to XP at the moment.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

In that case a simple code review and short test on XP would be sufficient, I'll test on windows 7 before checkin.

Comment 6

[Matthew Mecca [:mmecca]]

Comment on attachment 511703 [details] [diff] [review]
Fix - v1


>     skin/lightning/mode-switch-icons.png                   (themes/common/images/mode-switch-icons.png)
> 
>+% skin lightning classic/1.0 %skin/lightning/aero os=WINNT osversion>=6
>+    skin/lightning/aero/lightning-aero.css                      (themes/winstripe/lightning-aero.css)

Just a small nit - the indentation of "(themes/...)" is inconsistent with the rest of the file.

Looks fine on Windows XP, still needs to be tested on Vista / Win 7. Otherwise, r=mmecca.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

It seems I got the path for the css file wrong, so it wouldn't have caused sideeffects anyway. Could you do a quick test of this patch on winxp?

I'll give it a spin on linux in a moment.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Please pretend your nit is fixed, sorry :)

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

No issues on linux

Comment 10

[Matthew Mecca [:mmecca]]

Attachment: screenshot - fix v2 XP

This breaks the icon in Win XP.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 517727 [details] [diff] [review]
Fix - v2

how annoying. I'll see what I can do. Thanks for testing!

Comment 12

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v3

This should finally fix it. It does add a new chrome registration namespace, but I think in our case thats a better option than duplicating things as done in Thunderbird land.

This should definitely work, I've tested by manually incrementing the osversion in which case the css is not applied, as expected.

Comment 13

[Matthew Mecca [:mmecca]]

Comment on attachment 519017 [details] [diff] [review]
Fix - v3

Looks good on Win XP and Vista. Same code nit ;)
r=mmecca

Comment 14

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/ab969d8f4f7e>
-> FIXED