Currently, migrating add-on buttons from the add-on bar to the toolbar is a one-way process. This is going to lead to quite a few fully justified complaints from people who didn't want all their add-on buttons in their toolbar. Since we fully expect to see add-ons which restore the add-on bar, we should make it easy for the add-ons to move whatever buttons we have moved to the toolbar back to the add-on bar they add. (As opposed to forcing the user to move everything off of the toolbar, one by one.) My initial thought was to add something to the button when it passes through the add-on bar stub, but mconley had the possibly better idea of keeping a snapshot of the user's previous setup before we run the migration, and exposing that snapshot as JSON to add-ons which can then do whatever they want with it.

(In reply to Blake Winton (:bwinton) from comment #1) > Currently, migrating add-on buttons from the add-on bar to the toolbar is a > one-way process. This is going to lead to quite a few fully justified > complaints from people who didn't want all their add-on buttons in their > toolbar. > > Since we fully expect to see add-ons which restore the add-on bar, we should > make it easy for the add-ons to move whatever buttons we have moved to the > toolbar back to the add-on bar they add. (As opposed to forcing the user to > move everything off of the toolbar, one by one.) > > My initial thought was to add something to the button when it passes through > the add-on bar stub, but mconley had the possibly better idea of keeping a > snapshot of the user's previous setup before we run the migration, and > exposing that snapshot as JSON to add-ons which can then do whatever they > want with it. The problem is that this is a continuous process. That is, if post-startup something else inserts stuff into the add-on bar (e.g. a restartless add-on) it'll also get migrated. Should we be exposing a continuous list? Maybe we should fire an event of sorts?

(In reply to Blake Winton (:bwinton) from comment #0) > mconley had the possibly better idea of keeping a > snapshot of the user's previous setup before we run the migration We already have that in localstore.rdf. Add-ons can access that directly, or they can create a toolbar with the id "addon-bar", which would automatically get the currentset attribute restored.

(In reply to Dão Gottwald [:dao] from comment #2) > (In reply to Blake Winton (:bwinton) from comment #0) > > mconley had the possibly better idea of keeping a > > snapshot of the user's previous setup before we run the migration > > We already have that in localstore.rdf. Add-ons can access that directly, or > they can create a toolbar with the id "addon-bar", which would automatically > get the currentset attribute restored. Uhm, they can in the sense that nobody stops them. But they shouldn't, because there's already a toolbar with the id "addon-bar" which we're using to automigrate things... And currentset won't be updated if add-ons get put in the shim at runtime, and immediately migrated (and arguably they shouldn't then be in the addon-bar's currentset attribute, either). I don't think this bug is invalid. I'm not sure we will be fixing it, but it's a valid request IMO.

And shazam! As predicted, someone wrote an add-on to re-add the add-on bar. https://addons.mozilla.org/en-US/firefox/addon/the-addon-bar/ :)

This was a lot simpler than I thought.

I'm curious how this will work from an bar-restoring add-on's perspective... So an add-on registers a new area with CustomizableUI, and then inserts a XUL toolbar with customizable="true" into the document. Then I guess they wait for the chrome window to finish loading, retrieve the add-on bar's exposed migrated items list, and then use CustomizableUI to insert each of those items into their new bar? Maybe I'm exposing some of my ignorance in how browser.uiCustomization.state interacts with localstore.rdf, but wouldn't this migrated items list be only available the first time - the time that the items are migrated? Since after that, those placements are persisted in browser.uiCustomization.state. If that's the case, that kind of makes this migrated items list useless (or just out of reach) for add-ons that require a restart... How's my thinking on that?

(In reply to Mike Conley (:mconley) from comment #6) > I'm curious how this will work from an bar-restoring add-on's perspective... > > So an add-on registers a new area with CustomizableUI, and then inserts a > XUL toolbar with customizable="true" into the document. > > Then I guess they wait for the chrome window to finish loading, retrieve the > add-on bar's exposed migrated items list, and then use CustomizableUI to > insert each of those items into their new bar? > > Maybe I'm exposing some of my ignorance in how browser.uiCustomization.state > interacts with localstore.rdf, but wouldn't this migrated items list be only > available the first time - the time that the items are migrated? Since after > that, those placements are persisted in browser.uiCustomization.state. > > If that's the case, that kind of makes this migrated items list useless (or > just out of reach) for add-ons that require a restart... > > How's my thinking on that? The only way around that would be persisting something, and I'm not really sure that's a good idea. It's possible, but I'm not convinced, is all. At what point would we clear such a list? Or do we just plan on keeping it around forever? Rock and a hard place and all that.

(In reply to :Gijs Kruitbosch from comment #7) > (In reply to Mike Conley (:mconley) from comment #6) > > I'm curious how this will work from an bar-restoring add-on's perspective... > > > > So an add-on registers a new area with CustomizableUI, and then inserts a > > XUL toolbar with customizable="true" into the document. > > > > Then I guess they wait for the chrome window to finish loading, retrieve the > > add-on bar's exposed migrated items list, and then use CustomizableUI to > > insert each of those items into their new bar? > > > > Maybe I'm exposing some of my ignorance in how browser.uiCustomization.state > > interacts with localstore.rdf, but wouldn't this migrated items list be only > > available the first time - the time that the items are migrated? Since after > > that, those placements are persisted in browser.uiCustomization.state. > > > > If that's the case, that kind of makes this migrated items list useless (or > > just out of reach) for add-ons that require a restart... > > > > How's my thinking on that? > > The only way around that would be persisting something, and I'm not really > sure that's a good idea. It's possible, but I'm not convinced, is all. At > what point would we clear such a list? Or do we just plan on keeping it > around forever? Rock and a hard place and all that. We could stash a snapshot of the add-on bar population (or really, all of the toolbar currentsets) in prefs somewhere... and leave them there for a few releases of Australis, to allow users the opportunity to use add-ons that can make use of them. After a few releases, we clear them out via migrateUI? I don't know. I think anything we do here is going to be a bit hacky.

Alright, but then we'd need something like this... Does this look better?

Comment on attachment 8341802 [details] [diff] [review] add API for items migrated out of the add-on bar by Australis, Review of attachment 8341802 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/customizableui/content/toolbar.xml @@ +475,5 @@ > aNode.setAttribute("removable", "true"); > + if (!this._currentSetMigrated.has(aNode.id)) { > + this._currentSetMigrated.add(aNode.id); > + this.setAttribute("migratedset", this.getMigratedItems().join(',')); > + this.ownerDocument.persist(this.id, "migratedset"); Not sure I'm too jazzed about synchronous flushing to RDF for each node we evict, especially on start-up. Could we perhaps hook the RDF flush into a setter for _isModifying, or something along those lines? Just do a single flush after all evictions have taken place?

Alright, bunched up writes + a test. Plus fixing a for loop without a variable declaration. :-)

Comment on attachment 8341837 [details] [diff] [review] add API and test for items migrated out of the add-on bar by Australis, Review of attachment 8341837 [details] [diff] [review]: ----------------------------------------------------------------- Oh yes, I think I'm happier with this. Thanks Gijs!

Adding dev-doc-needed so that we don't forget to document this new attribute for add-on bar replacement authors.

remote: https://hg.mozilla.org/integration/fx-team/rev/f99da61e4010

Just a thought, and maybe I misunderstand how this works, but what if we just made the addon-bar shim movable, so that it could be added to the new "addon-bar"? That way any new addons that add their icon to the old addon bar would automatically be put in the new addon bar.