Last Comment Bug 829962 - Lightning breaks mail toolbar delete button icon
: Lightning breaks mail toolbar delete button icon
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: 2.1
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-12 09:15 PST by Merike (:merike)
Modified: 2013-02-11 00:01 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.14 KB, patch)
2013-01-12 11:28 PST, Richard Marti (:Paenglab)
philipp: review+
philipp: approval‑calendar‑aurora+
philipp: approval‑calendar‑beta+
Details | Diff | Splinter Review
New ID (7.95 KB, patch)
2013-02-07 10:07 PST, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review

Description Merike (:merike) 2013-01-12 09:15:10 PST
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) 2013-01-12 11:28:10 PST
Created attachment 701482 [details] [diff] [review]
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] 2013-02-07 06:58:56 PST
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) 2013-02-07 10:07:48 PST
Created attachment 711403 [details] [diff] [review]
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] 2013-02-08 00:36:54 PST
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) 2013-02-08 00:52:56 PST
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] 2013-02-08 07:04:19 PST
Pushed to comm-central changeset 76972e8b8432
Comment 7 Philipp Kewisch [:Fallen] 2013-02-08 07:04:37 PST
Backported to releases/comm-aurora changeset 82a84f2fa6c5
Comment 8 Philipp Kewisch [:Fallen] 2013-02-08 07:04:55 PST
Backported to releases/comm-beta changeset d2c6e7722663
Comment 9 :aceman 2013-02-10 23:49:44 PST
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) 2013-02-11 00:01:16 PST
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.

Note You need to log in before you can comment on or make changes to this bug.