Closed
Bug 1229123
Opened 10 years ago
Closed 10 years ago
Support removal of built-in buttons
Categories
(Firefox :: Toolbars and Customization, defect)
Firefox
Toolbars and Customization
Tracking
()
RESOLVED
FIXED
Firefox 45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
Details
Attachments
(1 file, 1 obsolete file)
5.98 KB,
patch
|
mixedpuppy
:
review+
|
Details | Diff | Splinter Review |
This is important for hello and pocket functionality to be moved into addons.
If you remove a built in button, and add it into an addon, there is no way for the addon or CUI to determine whether the button had been placed into the palette. built-in buttons are not added to the "seen" state. This patch adds an obsolete object that will be checked on startup, if a button is removed in that version then it is added to the "seen" state. With this, createWidget will properly place the button into the palette if that was the prior location.
This patch was developed and tested as part of bug 1215694 but is separated out to allow hello to address the same issue. As such, the obsolete object is currently empty and will be addressed in the respective bugs for pocket and hello.
Attachment #8693727 -
Flags: review?(gijskruitbosch+bugs)
Comment 1•10 years ago
|
||
Comment on attachment 8693727 [details] [diff] [review]
cui-obsolete
Review of attachment 8693727 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/components/customizableui/CustomizableUI.jsm
@@ +370,5 @@
> + return;
> + // we're upgrading, update state if necessary
> + for (let id in ObsoleteBuiltinButtons) {
> + let version = ObsoleteBuiltinButtons[id]
> + if (version == kVersion && !gSeenWidgets.has(id)) {
Nit: don't need to check gSeenWidgets.has(), it's a set, so .add() will just no-op if the item exists already.
@@ +374,5 @@
> + if (version == kVersion && !gSeenWidgets.has(id)) {
> + gSeenWidgets.add(id);
> + }
> + }
> + },
I wonder if you need to set gDirty to true, and if that will be enough to ensure the state is saved. Can you check what happens if you:
1) create new profile without this patch
2) remove pocket
3) run on a build with pocket in the list of obsolete thingies in here
4) restart on that build without entering customization mode or whatever
do we save the state back to the pref? How?
I think right now we won't, and so we should make sure that that happens one way or another.
r=me with that addressed, if it's much more work than just adding gDirty (or if my assumption that we're not saving is totally wrong) it might be good for me to have another look, but up to you.
Attachment #8693727 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 2•10 years ago
|
||
comments addressed.
Assignee: nobody → mixedpuppy
Attachment #8693727 -
Attachment is obsolete: true
Attachment #8693767 -
Flags: review+
Assignee | ||
Comment 3•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/97258d49fa0184f98d1b730b463e0d7c3de4145e
Bug 1229123 support for obsoleted buttons, r=Gijs
Comment 4•10 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
You need to log in
before you can comment on or make changes to this bug.
Description
•