Closed
Bug 730078
Opened 9 years ago
Closed 9 years ago
Remove the QFB-button from defaultset
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: Paenglab, Assigned: Paenglab)
References
Details
Attachments
(1 file, 3 obsolete files)
1.23 KB,
patch
|
Fallen
:
review+
Fallen
:
approval-calendar-aurora+
Fallen
:
approval-calendar-beta+
|
Details | Diff | Splinter Review |
Now the QFB-button is moved to the mail-toolbar. To not blocking the migration the defaultset in messenger-overlay-sidebar.xul have to be removed.
Assignee | ||
Comment 1•9 years ago
|
||
Concerning Mike Conley the migration problem is the defaultset. I'm asking for approval‑calendar‑beta because this needs to land in beta.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #600145 -
Flags: review?(philipp)
Attachment #600145 -
Flags: approval-calendar-beta?
Attachment #600145 -
Flags: approval-calendar-aurora?
Comment 2•9 years ago
|
||
I took a different approach to this. The problem with hardcoding defaultset in an overlay is that it can break migrations like this, but it can also conflict with other addons if 2 addons try to override the defaultset
Assignee | ||
Comment 3•9 years ago
|
||
Comment on attachment 600352 [details] [diff] [review] Don't hardcode defaultset in an overlay, as it can break migrations and conflict with other addons This looks better than my amateurish approach. So asking for review.
Attachment #600352 -
Flags: review?(philipp)
Attachment #600352 -
Flags: approval-calendar-beta?
Attachment #600352 -
Flags: approval-calendar-aurora?
Assignee | ||
Updated•9 years ago
|
Attachment #600145 -
Attachment is obsolete: true
Attachment #600145 -
Flags: review?(philipp)
Attachment #600145 -
Flags: approval-calendar-beta?
Attachment #600145 -
Flags: approval-calendar-aurora?
Comment 4•9 years ago
|
||
Comment on attachment 600352 [details] [diff] [review] Don't hardcode defaultset in an overlay, as it can break migrations and conflict with other addons >+ try { >+ currentUIVer = Services.prefs.getIntPref(UI_VERSION_PREF); >+ } catch(ex) {} you could use cal.getPrefSafe(UI_VERSION_PREF, -1) here >+ <toolbarpalette id="MailToolbarPalette"> > <toolbarbutton id="calendar-tab-button" We already have buttons in the Mail Toolbar, here: http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/lightning-toolbar.xul#93 Please adapt accordingly. Also please use {} even for one-line if's to match calendar style r=philipp and approval with those changes.
Attachment #600352 -
Flags: review?(philipp)
Attachment #600352 -
Flags: review+
Attachment #600352 -
Flags: approval-calendar-beta?
Attachment #600352 -
Flags: approval-calendar-beta+
Attachment #600352 -
Flags: approval-calendar-aurora?
Attachment #600352 -
Flags: approval-calendar-aurora+
Comment 5•9 years ago
|
||
Chris, do you have time to update this patch? This should go in soon since I'd like to create a new beta build this week.
Comment 6•9 years ago
|
||
Yes, sorry, will do that now. Thanks!
Comment 7•9 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #4) Ok, I've updated this now > Comment on attachment 600352 [details] [diff] [review] > Don't hardcode defaultset in an overlay, as it can break migrations and > conflict with other addons > > > >+ try { > >+ currentUIVer = Services.prefs.getIntPref(UI_VERSION_PREF); > >+ } catch(ex) {} > you could use cal.getPrefSafe(UI_VERSION_PREF, -1) here > > > >+ <toolbarpalette id="MailToolbarPalette"> > > <toolbarbutton id="calendar-tab-button" > We already have buttons in the Mail Toolbar, here: > > http://mxr.mozilla.org/comm-central/source/calendar/lightning/content/ > lightning-toolbar.xul#93 > > Please adapt accordingly. > I've dropped one set of buttons. I hope I understood correctly, and apologise if I've misunderstood what you were asking. > Also please use {} even for one-line if's to match calendar style > > r=philipp and approval with those changes.
Attachment #600352 -
Attachment is obsolete: true
Attachment #601581 -
Flags: review?(philipp)
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Chris Coulson from comment #7) > Created attachment 601581 [details] [diff] [review] > Don't hardcode defaultset in an overlay, as it can break migrations and > conflict with other addons (v2) This patch would remove lightning-button-calendar and lightning-button-tasks completely? I think this isn't what Philipp wants now. lightning-button-calendar and lightning-button-tasks should stay in the MailToolbarPalette or if customized on a toolbar. calendar-tab-button and task-tab-button should be irremovable (until a decision is made what's the future of this buttons) in tabbar-toolbar. Is this correct Philipp?
Comment 9•9 years ago
|
||
Right, hence my last comment :)
Comment 10•9 years ago
|
||
Ok, so it was pointed out to me on IRC that the buttons on the tabbar aren't currently removable, which I hadn't realized. This makes things much simpler. All we need to do now is to just not set defaultset at all. The only benefit of doing that previously is that it defined the placement of the buttons wrt the QFB for an unmodified tabbar, but as the defaultset is empty now in Thunderbird, we don't need to do this.
Attachment #601581 -
Attachment is obsolete: true
Attachment #601734 -
Flags: review?(philipp)
Attachment #601581 -
Flags: review?(philipp)
Comment 11•9 years ago
|
||
Comment on attachment 601734 [details] [diff] [review] Don't hardcode defaultset in an overlay, as it can break migrations and conflict with other addons (v3) Ooh, simple! I like! Please push to aurora and beta too.
Attachment #601734 -
Flags: review?(philipp)
Attachment #601734 -
Flags: review+
Attachment #601734 -
Flags: approval-calendar-beta+
Attachment #601734 -
Flags: approval-calendar-aurora+
Comment 12•9 years ago
|
||
Pushed to comm-central changeset d3f4108df62e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
Comment 13•9 years ago
|
||
Backported to releases/comm-aurora changeset 810dff72d281
Target Milestone: 1.5 → 1.4
Comment 14•9 years ago
|
||
Backported to releases/comm-beta changeset 503595f53b2e
Target Milestone: 1.4 → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•