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

RESOLVED FIXED in Thunderbird 36.0

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 36.0
All
Windows 7
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

(thunderbird36 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Posted 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)
(Assignee)

Comment 2

5 years ago
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+
(Assignee)

Comment 5

5 years ago
Removed the white space.
Attachment #8480753 - Attachment is obsolete: true
Attachment #8498373 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/3cf5411b9643
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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?
(Assignee)

Comment 8

5 years ago
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+
(Assignee)

Updated

5 years ago
Attachment #8498373 - Attachment description: Patch for check-in → Patch for check-in checked-in)
(Assignee)

Updated

5 years ago
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
(Assignee)

Updated

5 years ago
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
Last Resolved: 5 years ago5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.