1059927 - Extend the inverted icon logic from bug 1046563 to AB, Composer and Lightning

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Richard Marti (:Paenglab)]

Bug 1046563 introduced the logic to use the inverted icons depending of the toolbar color. This bug is to implement this logic to the AB, Composer, Message header buttons and Lightnings event dialog and task toolbar.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: bug1059927.patch

Similar to bug 1046563.

The mail-toolbar-aero-inverted.png was missing two white lines on archive icon. I added them.

Josiah, you can test this patch with the build you downloaded from me. The easiest way to test is to switch to a dark HC theme in Win 8.

Philipp, I've added inverted icons for the event dialog toolbar and the task actions buttons. The new event button in today pane and the new task button aren't inverted because they aren't in a toolbar to detect the color. Maybe I can solve this in a other bug.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: calendar-event-dialog.png

Screenshot of the calendar event dialog toolbar with the inverted icons on top and the normal icons (actual state) on bottom.

Comment 3

[Josiah Bruner [:jsbruner]]

FYI, My review probably won't come until next week as my main machine is currently non-operational. I'm taking it in to get a part replaced today, so I should have it by next week.

Comment 4

[Josiah Bruner [:jsbruner]]

Comment on attachment 8480753 [details] [diff] [review]
bug1059927.patch

Review of attachment 8480753 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, this looks fine to me, just one nit.

::: calendar/base/content/dialogs/calendar-event-dialog.xul
@@ +54,5 @@
>      <script type="application/javascript"
>              src="chrome://global/content/globalOverlay.js"/>
>      <script type="application/javascript"
>              src="chrome://global/content/printUtils.js"/>
>      <script type="application/javascript" 

There's trailing white space here, could you remove that since you're touching the file? Unfortunately my white space killer didn't touch stuff in calendar/

Comment 5

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in (checked-in)

Removed the white space.

Comment 6

[Patrick Cloke [:clokep]]

https://hg.mozilla.org/comm-central/rev/3cf5411b9643

Comment 7

[Stefan Sitter [:ssitter]]

It seems that Lightning now calls some file/method that only exists in mail/Thunderbird, i.e. Lightning will be broken in SeaMonkey?

Comment 8

[Richard Marti (:Paenglab)]

Attachment: 1059927removeCall.patch

While checking to not calling the function on SM, I saw this call isn't needed in calendar-event-dialog.js.

With removing it we should be fine with SM again. Sorry to not seeing this earlier.

Comment 9

[Josiah Bruner [:jsbruner]]

https://hg.mozilla.org/comm-central/rev/e77c13e9691c