SessionManager 0.8+ sometimes clears out the main toolbar

RESOLVED FIXED

Status

Tech Evangelism
Add-ons
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nmaier, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

4 years ago
Better track this in Bugzilla as well.

After installing the add-on (URL) and restarting the browser, all toolbar items will be removed from the main toolbar except for both Session Manager items.
STR:
- New and clean Firefox profile
- Install Session Manager
- Restart the browser after a few moments
-> toolbar gets cleaned out after restart and browser becomes essentially unusable unless you put all that stuff (navigation, urlbar, etc) back and know how to put back things in the first place. http://i.imgur.com/OiVkVyR.png
This does not happen when you Customize (with or without changing anything) before restarting.

I can reliably reproduce this on OSX/Win7.

If amo-admins can reproduce this, it might be a wise idea to downgrade 0.8+ for the time being.
(Reporter)

Updated

4 years ago
That's very odd because I have had no reports of this happening and 0.8.0.1 has been out for awhile and is currently in use by about 250,000 plus users. In fact the rejected review was the first I'd ever heard of this problem. The compatibility reports on 0.8.01+ are overwhelmingly positive. Obviously I never saw this or I wouldn't have submitted 0.8+ and 0.8.0.1 wouldn't have been accepted I don't think. Is there something special about your version of Firefox (locale maybe?).

What's odd is that Session Manager doesn't even make any changes to the existing toolbar when installed.  It simply adds two buttons to the Toolbar Pallette using Dmitry Gutov's buttons.js script with modifications by Erik Void (see include/buttons.js), but doesn't actually add them to the toolbar.  It definitely doesn't remove anything from the toolbar or Toolbar Pallete so I see know reason why all buttons except for the 2 Session Manager ones would disappear.
Flags: needinfo?
Okay I reproduced this.  The problem seems to only occur if the user has never customized the Toolbars before since first installing Firefox.  If the toolbars were customized any time in the past prior to installing Session Manager, the problem doesn't occur.  This explains why the vast majority of the people don't see this as I think most people who install add-ons have customized their toolbar at some point in the past.

I'll look into seeing why this occurs and how I can work around it, but just based on what I'm seeing this would seem like a Firefox bug.

There's a simple work around though, which is to right click on a toolbar, choose customize and restore the default set.  That will restore the Toolbars back to the default state.  Users won't lose any customizations, because if they had made any, the problem would not occur.
Flags: needinfo?
(Reporter)

Comment 3

4 years ago
(In reply to Michael Kraft [:morac] from comment #2)
> Okay I reproduced this.  The problem seems to only occur if the user has
> never customized the Toolbars before since first installing Firefox.  If the
> toolbars were customized any time in the past prior to installing Session
> Manager, the problem doesn't occur.  This explains why the vast majority of
> the people don't see this as I think most people who install add-ons have
> customized their toolbar at some point in the past.

Yep, it seems only to happen if the toolbar wasn't customized before.

> There's a simple work around though, which is to right click on a toolbar,
> choose customize and restore the default set.  That will restore the
> Toolbars back to the default state.  Users won't lose any customizations,
> because if they had made any, the problem would not occur.

Most savvy users will know how to reset it, either the way you describe it or by Help/Troubleshooting/Reset, but many users aren't savvy... Given the target audience a typical session manager user might be a bit more savvy, but that won't include all users. Searching for a solution is also problematic as the urlbar/searchbar are missing. And having users switching to another browser to fix a problem in Firefox is a horrible experience. :p
So it will likely affect only few users (non-savvy, not-customized-before), but these few users will be affected quite badly.

> I'll look into seeing why this occurs and how I can work around it, but just
> based on what I'm seeing this would seem like a Firefox bug.

It might be enough just to simulate a customization event?
Can this be reproduced without using the SDK or these libraries? Could this be a bug in either of those, rather than Firefox?
I know what the problem is.  When installing in the toolbar, Session Manager grabs the current "currentset" attribute from the toolbar, adds itself to the "currentset" and then sets the attribute back to the toolbar and persists it.

The problem is that until the user customizes the toolbars, the "currentset" attribute doesn't exist, nor is it defined in localstore.rdf.  As such there's no way to "get" the current toolbar configuration.  When Session Manager adds itself to the blank "currentset" and saves it the result is all the existing toolbar items disappear.

There is a "defaultset" attribute which contains the default toolbar icons, so I'll just read that if "currentset" doesn't exist.
This is fixed in Session Manager 0.8.0.7
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
I filed bug 869134 to look into the root causes of this.
(In reply to Michael Kraft [:morac] from comment #5)
> I know what the problem is.  When installing in the toolbar, Session Manager
> grabs the current "currentset" attribute from the toolbar, adds itself to
> the "currentset" and then sets the attribute back to the toolbar and
> persists it.
> 
> The problem is that until the user customizes the toolbars, the "currentset"
> attribute doesn't exist, nor is it defined in localstore.rdf.  As such
> there's no way to "get" the current toolbar configuration.  When Session
> Manager adds itself to the blank "currentset" and saves it the result is all
> the existing toolbar items disappear.

You should be reading the currentSet property, not the currentset
attribute. The latter is only used for persistence, the former
defines the current contents of the toolbar.
(Reporter)

Comment 9

4 years ago
> This is fixed in Session Manager 0.8.0.7

Actually, it is 0.8.0.6, which I just approved.

JFYI, fixed by
toolbar.currentSet.split(",") // was toolbar.getAttribute("currentset").split(",")
...
toolbar.setAttribute("currentset", toolbar.currentSet);
(In reply to Kris Maglione [:kmag] from comment #8)
> You should be reading the currentSet property, not the currentset
> attribute. The latter is only used for persistence, the former
> defines the current contents of the toolbar.

Actually I do need to know the persistence since I need to know where to insert the toolbar item (bootstrap addon).  The issue is I shouldn't have been using that when adding the toolbar item for the first time.  

As comment #9 mentioned, I switched to using the currentSet property on an install or more specifically when the toolbar item's ID can't be found in the persistent set.  I also use it when setting persistence now. 

Thanks for the fast approval.
(In reply to Michael Kraft [:morac] from comment #10)
> Actually I do need to know the persistence since I need to know where to
> insert the toolbar item (bootstrap addon).  The issue is I shouldn't have
> been using that when adding the toolbar item for the first time.  
> 
> As comment #9 mentioned, I switched to using the currentSet property on an
> install or more specifically when the toolbar item's ID can't be found in
> the persistent set.  I also use it when setting persistence now. 

It's irrelevant. The currentSet property defines the current state of the toolbar, which is all you need to know. Copying the result to the currentset property is necessary for persistence, but its state is only ever relevant at startup when the toolbar is first initialized.
You need to log in before you can comment on or make changes to this bug.