Closed Bug 1229123 Opened 10 years ago Closed 10 years ago

Support removal of built-in buttons

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch cui-obsolete (obsolete) — 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 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+
Attached patch cui-obsoleteSplinter Review
comments addressed.
Assignee: nobody → mixedpuppy
Attachment #8693727 - Attachment is obsolete: true
Attachment #8693767 - Flags: review+
Blocks: 1229351
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Blocks: 1236014
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: