(In reply to Luís Miguel [:Quicksaver] from bug 947987 comment #3) > [We should ensure that the non-removable widget ends up in its > defaultArea if it has no placement. > > For example (I haven't tested this, just throwing it out of my head from > reading the code) if I create a custom aToolbar, then create a non-removable > widget defaulting to aToolbar, then unregisterArea with aDestroyPlacements = > true and then destroy the widget. At this point the widget placement is > destroyed, but if I re-create aToolbar and the widget (in that order) the > widget won't be placed in its defaultArea because gSeenWidgets will have the > widget, and so its defaultArea won't be used for placing it. Note that in this case we're assuming the new widget isn't listed in the defaultPlacements for the new aToolbar... which sounds somewhat implausible until you realize we persist gSeenWidgets forever-and-ever. So this kind of permanently nukes them. That is kind of sadfaces, also because they're non-removable so they'll always be unhappy widgets. Oops.

Test + fix

Comment on attachment 8346196 [details] [diff] [review] ensure non-removable API-based widgets with a defaultArea work after destruction, Review of attachment 8346196 [details] [diff] [review]: ----------------------------------------------------------------- Not sure which is more confusing to actually use - having them show, or having them not show. But implementation wise, this is definitely the simplest - so lets go with this patch for now, and re-visit in the future if needed. ::: browser/components/customizableui/src/CustomizableUI.jsm @@ +1693,5 @@ > // seen before, then add it to its default area so it can be used. > + // If the widget is not removable, we *have* to add it to its default > + // area here. > + if (autoAdd && !widget.currentArea && > + (!gSeenWidgets.has(widget.id) || !widget.removable)) { Non-removable widgets should *not* depend on autoAdd being true - that'll end up being another manifestation of this bug.

Good point. Now with test. :-)

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

I've verified this in my add-on with a custom toolbar and a non-removable widget in it. The widget is successfully placed back in the toolbar when I click the reset defaults button, and when I delete its placement directly in the prefs.js file. (Not marking as Verified yet because I'm unsure of how to test the "Non-removable widgets should *not* depend on autoAdd being true" part of the patch)