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

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 User image Merike Sell (: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 is broken by
Comment 1 User image Richard Marti (:Paenglab) 2013-01-12 11:28:10 PST
Created attachment 701482 [details] [diff] [review]

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 User image Philipp Kewisch [:Fallen] 2013-02-07 06:58:56 PST
Comment on attachment 701482 [details] [diff] [review]

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 User image 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 User image 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 User image 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 User image Philipp Kewisch [:Fallen] 2013-02-08 07:04:19 PST
Pushed to comm-central changeset 76972e8b8432
Comment 7 User image Philipp Kewisch [:Fallen] 2013-02-08 07:04:37 PST
Backported to releases/comm-aurora changeset 82a84f2fa6c5
Comment 8 User image Philipp Kewisch [:Fallen] 2013-02-08 07:04:55 PST
Backported to releases/comm-beta changeset d2c6e7722663
Comment 9 User image :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 User image 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.