Closed Bug 1059927 Opened 7 years ago Closed 7 years ago

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

Categories

(Thunderbird :: Theme, defect)

All
Windows 7
defect
Not set
normal

Tracking

(thunderbird36 fixed)

RESOLVED FIXED
Thunderbird 36.0
Tracking Status
thunderbird36 --- fixed

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(3 files, 1 obsolete file)

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.
Attached patch bug1059927.patch (obsolete) — Splinter Review
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.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8480753 - Flags: review?(philipp)
Attachment #8480753 - Flags: review?(josiah)
Screenshot of the calendar event dialog toolbar with the inverted icons on top and the normal icons (actual state) on bottom.
Attachment #8480753 - Flags: review?(philipp) → review+
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 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/
Attachment #8480753 - Flags: review?(josiah) → review+
Removed the white space.
Attachment #8480753 - Attachment is obsolete: true
Attachment #8498373 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/3cf5411b9643
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
It seems that Lightning now calls some file/method that only exists in mail/Thunderbird, i.e. Lightning will be broken in SeaMonkey?
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.
Attachment #8507090 - Flags: review?(philipp)
Attachment #8507090 - Flags: review?(philipp) → review+
Attachment #8498373 - Attachment description: Patch for check-in → Patch for check-in checked-in)
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Attachment #8498373 - Attachment description: Patch for check-in checked-in) → Patch for check-in (checked-in)
https://hg.mozilla.org/comm-central/rev/e77c13e9691c
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.