1229123 - Support removal of built-in buttons

Status: RESOLVED FIXED

Description

[Shane Caraveo (:mixedpuppy)]

Attachment: cui-obsolete

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.

Comment 1

[:Gijs (he/him)]

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.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: cui-obsolete

comments addressed.

Comment 3

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/fx-team/rev/97258d49fa0184f98d1b730b463e0d7c3de4145e
Bug 1229123 support for obsoleted buttons, r=Gijs

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97258d49fa01