829962 - Lightning breaks mail toolbar delete button icon

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Merike Sell (:merike)]

This is 719050 again. Affects at least gnomestripe and possibly also pinstripe and win classic.

For gnomestripe http://mxr.mozilla.org/comm-central/source/mail/themes/gnomestripe/mail/primaryToolbar.css#161 is broken by http://mxr.mozilla.org/comm-central/source/calendar/base/themes/gnomestripe/dialogs/calendar-event-dialog.css#141

Comment 1

[Richard Marti (:Paenglab)]

Attachment: patch

I see this only under Linux in Customize window. This patch is only the minimal change to fix the issue. Maybe we should think about a id change to be safe and no more affecting TB or SM. Maybe a id like button-delete-event (I know the button also deletes tasks) could be better.

Comment 2

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

Comment on attachment 701482 [details] [diff] [review]
patch

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

I'm fine with an id change, I'd suggest button-delete-item or maybe button-delete-calitem. r+ for this patch too though.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: New ID

This patch changes the button ID. I've also added a migration code but it isn't working. Philipp, please can you check what's wrong? Best would be if it checks if button-delete is in the toolbar and only then exchanges the IDs.

Comment 4

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

Comment on attachment 711403 [details] [diff] [review]
New ID

The code will not work since calendar-chrome-startup is loaded on the main window, not the event dialog. We would have to add extra code to the event dialog to migrate.

I missed the fact that this means migration code. Do you really think its worth it?

Comment 5

[Richard Marti (:Paenglab)]

With the first patch we should also be safe. Please can you land it, I'm at work? should this also go to aurora/beta to stop breaking SM?

Comment 6

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

Pushed to comm-central changeset 76972e8b8432

Comment 7

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

Backported to releases/comm-aurora changeset 82a84f2fa6c5

Comment 8

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

Backported to releases/comm-beta changeset d2c6e7722663

Comment 9

[:aceman]

Are the other selectors in the modified file OK? Should e.g. '#button-delete[disabled="true"]:hover' NOT take the .cal-event-toolbarbutton class?

Comment 10

[Richard Marti (:Paenglab)]

As I wrote in comment 1, this affected only the icon in customize window, and where is no disabled state and so no need to change this.