1393661 - Resetting customization reshows add-ons after a restart because the new set of 'seen' widgets isn't saved

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Ted Campbell [:tcampbell]]

If one resets the toolbar from the customizations view, it does not show add-on buttons (such as screenshots or test pilot). When browser is restarted, the layout adds default add-on buttons. I expect resetting layout should generate a stable layout that is consistent with layout after restarting browser.

Comment 1

[Ted Campbell [:tcampbell]]

Gijs, perhaps you can look at this or redirect it. Thanks

Comment 2

[Mike Conley (:mconley) (:⚙️)]

For this one, if Gijs is on PTO next week, maybe mixedpuppy knows what's going on here - I think he's worked in this area before.

Comment 3

[:Gijs (he/him)]

Yeah, I looked at this a bit earlier. Not really done figuring this out - the expected behaviour is that buttons stay out. The bug is that they don't seem to be in gSeenWidgets. I think this because in reset() we set gDirty to true but don't force a save if gSeenWidgets is not empty. So if you follow these steps:

1. open firefox
2. use restore defaults
3. restart

the buttons come back

but if you do:

1. open firefox
2. use restore defaults
3. move the home button in the toolbar
4. restart

they don't.

I suspect just calling saveState() in this case, too, will fix this, but I don't really know why that is necessary / wasn't necessary before. Need to dig into that bit a bit more. Leaving needinfo for now, I *might* have time tomorrow, otherwise maybe Shane can figure it out in my absence, otherwise I'll do it when I get back. I somewhat doubt this bit is related to any photon changes, though it could be, of course...

Comment 4

[:Gijs (he/him)]

Not a (recent) regression, this just never worked quite right, AFAICT (checked with Firefox 49).

Comment 5

[:Gijs (he/him)]

Relevant code has been there since bug 1096763, the test of which claims that we expect there to be no saved state after a reset "otherwise undo wouldn't work right". That is not (or at least no longer) true, I think, but the test will need updating regardless (and we should be careful about not breaking undo).