940974 - Australis: Customizations are not remembered after the restart

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Krzysztof]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20131119030202

Steps to reproduce:

1. Press Customize button
2. Customize your toolbar and menu
3. Restart the browser


Actual results:

Customizations are not remembered, "hamburger" menu and urlbar are going back to the default state. 


Expected results:

Customizations should be remembered after the restart.

Comment 1

[:Gijs (out for now; he/him)]

Are you moving add-on icons? What about the builtin widgets, do those persist? So far we've only seen problems with add-ons that restore their icons to their default locations after a restart.

Comment 2

[Krzysztof]

None of them persist. I've tried to put Subscribe, Developer icons on the menu, also icons of WebDeveloper extension, without luck.

Comment 3

[:Gijs (out for now; he/him)]

(In reply to Krzysztof from comment #2)
> None of them persist. I've tried to put Subscribe, Developer icons on the
> menu, also icons of WebDeveloper extension, without luck.

This is very strange. Can you reproduce on an empty profile or in safe mode? We just write stuff to the prefs file, so it sounds like somehow preferences aren't being saved correctly.

Comment 4

[Krzysztof]

I restarted in safe-mode twice, doing the Customizations in-between. Icons of widgets have survived, thanks. 

However, still no luck with add-on icons.

Comment 5

[:Gijs (out for now; he/him)]

(In reply to Krzysztof from comment #4)
> I restarted in safe-mode twice, doing the Customizations in-between. Icons
> of widgets have survived, thanks. 
> 
> However, still no luck with add-on icons.

Can you detail which add-ons these are?

Comment 6

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Krzysztof from comment #4)
> > I restarted in safe-mode twice, doing the Customizations in-between. Icons
> > of widgets have survived, thanks. 
> > 
> > However, still no luck with add-on icons.
> 
> Can you detail which add-ons these are?

Comment 8

[Krzysztof]

Adblock+, Stylish, WebDeveloper, Print Edit, Download Helper.

Comment 9

[Fyren]

I seem to be experiencing exactly this, too.  I'm on the 20th's nightly (20131120062258) on Windows 7.  After restarting Firefox, any toolbar modifications didn't save.  Starting in safe mode did allow me to modify the default widgets.  I can modify the default widgets in a new profile as well.

The addons I have with toolbar buttons are ABP, Pano, Cookie Manager, and Scriptish.

Comment 10

[Dave Camp (:dcamp)]

I had some customization revert after a restart:

I had previously moved the Developer button to my toolbar.  I then moved the search bar, developer button, and downloads button to the popup.  

I also have on addon button (OnePassword) that I can't remember if I decided to leave on the toolbar or put in the popup.

After a quit/restart my most recent customizations had been reverted, but the Developer button was still on the toolbar.

Comment 11

[:Gijs (out for now; he/him)]

So, the weirdest about all of these reports is that it seems like somehow the add-ons are also impacting whether normal icons get saved correctly.

This is strange because we just write to prefs to save all this info, so I don't see how any add-ons would interfere with this. If we had consistent steps to reproduce from a clean profile, that would be very helpful in figuring out what is going on here.

The add-ons themselves not saving their positions correctly is generally because the add-ons have custom-written code that inserts themselves into a toolbar if they don't exist somewhere. So you remove them, and they go "hey, my icon is gone" and go and add themselves to the toolbar again. That's wrong, but there's not really anything we can do against that.

I'm confirming this because it's clear that enough people are running into this that it's a real issue, even if it might be (partly?) caused by add-ons.

Comment 12

[:Gijs (out for now; he/him)]

Note that if you can reproduce this with builtin icons on your regular profile, it'd probably be helpful if you checked the browser console (ctrl-shift-j on windows) to see if there are any errors that show up there.

Comment 13

[Fyren]

(In reply to :Gijs Kruitbosch from comment #12)
I tried adding a default widget to my toolbar and the console reported the following:

Error finding I: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/ShortcutUtils.jsm :: <TOP_LEVEL> :: line 88"  data: no] ShortcutUtils.jsm:90

When I removed it, I received a different error that mentioned one of my user scripts in Scriptish.  I removed the script and Firefox will now remember what I do to default widgets.  Unfortunately, I didn't copy the second error, but it was complaining about the script referring to an undefined function.  Even more unfortunate is that I don't know which script it was now that I removed it.

Comment 14

[:Gijs (out for now; he/him)]

(In reply to Fyren from comment #13)
> (In reply to :Gijs Kruitbosch from comment #12)
> I tried adding a default widget to my toolbar and the console reported the
> following:
> 
> Error finding I: [Exception... "Component returned failure code: 0x80004005
> (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult:
> "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
> resource://gre/modules/ShortcutUtils.jsm :: <TOP_LEVEL> :: line 88"  data:
> no] ShortcutUtils.jsm:90
> 
> When I removed it, I received a different error that mentioned one of my
> user scripts in Scriptish.  I removed the script and Firefox will now
> remember what I do to default widgets.  Unfortunately, I didn't copy the
> second error, but it was complaining about the script referring to an
> undefined function.  Even more unfortunate is that I don't know which script
> it was now that I removed it.

Do you still have the script so you can re-add it? :-)

As for the first error, that's unrelated, but I've been meaning to file that for like forever, so thank you for reminding me!

(in the meantime, this is going to be a P1 because we need to get this sorted)

Comment 15

[Fyren]

(In reply to :Gijs Kruitbosch from comment #14)
> Do you still have the script so you can re-add it? :-)

I don't think so.  I wasn't the author.  In my profile, it's not next to the other user scripts still installed.  I don't know if traces of it might be sitting around somewhere in my profile.

If it helps any, the function in the error message was, I think, "GM_info".  That identifier doesn't show up on MXR, but Google says Greasemonkey's API has a function of that name and that Scriptish doesn't implement it.

Comment 16

[:Gijs (out for now; he/him)]

(In reply to Fyren from comment #15)
> (In reply to :Gijs Kruitbosch from comment #14)
> > Do you still have the script so you can re-add it? :-)
> 
> I don't think so.  I wasn't the author.  In my profile, it's not next to the
> other user scripts still installed.  I don't know if traces of it might be
> sitting around somewhere in my profile.
> 
> If it helps any, the function in the error message was, I think, "GM_info". 
> That identifier doesn't show up on MXR, but Google says Greasemonkey's API
> has a function of that name and that Scriptish doesn't implement it.

Yeah, it's a greasemonkey thing. https://github.com/greasemonkey/greasemonkey/blob/master/modules/script.js#L455

I don't see anything in there that'd obviously spectacularly start failing, but then again, we don't know what the script did.

At the minute we probably need more info as to stuff that breaks (other than SDK add-ons, where we'll still waiting on an SDK uplift - although they shouldn't affect the rest of your customizations being saved). :-(

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Gijs, did you intend to needinfo a specific person?

Comment 18

[:Gijs (out for now; he/him)]

Not really; this bug isn't really actionable without more specific STR. I'll try to reproduce with the add-ons listed in comment #8 tomorrow and see if I can (so I'll leave the needinfo for me). If not, I'll probably close as WFM, to be reopened if someone has specific STR. :-(

Comment 19

[:Gijs (out for now; he/him)]

Sometimes really complicated problems turn out to be really simple. Patch in a second.

Comment 20

[:Gijs (out for now; he/him)]

Attachment: in Australis' CustomizableUI, don't call beginBatchUpdate before potentially bailing and throwing, leading to the batch update stack being forever non-0,

So the issue is, the web dev toolbar isn't uptodate yet and we will probably change some of our toolbar registration stuff to try and help it work, but in the meantime, its added toolbar has our binding, which calls registerToolbarNode, which throws. That in itself isn't such a terrible problem. The problem is that before it throws, we call beginBatchUpdate, and then we never call endBatchUpdate. Then whenever we call saveState, we actually no-op because our batch update stack is non-zero. Simple fix is just not calling beginBatchUpdate before throwing...

Comment 21

[Mike de Boer [:mikedeboer]]

Comment on attachment 8339894 [details] [diff] [review]
in Australis' CustomizableUI, don't call beginBatchUpdate before potentially bailing and throwing, leading to the batch update stack being forever non-0,

Review of attachment 8339894 [details] [diff] [review]:
-----------------------------------------------------------------

Ough, that `throw` was indeed in a wicked position!

Comment 22

[:Gijs (out for now; he/him)]

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

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e95f769be558

Comment 24

[Krzysztof]

Confirming it as 'fixed'. ABP icon still on the toolbar.