631803 - Personas Plus for Firefox 4 can not rotate Personas Skin

Status: RESOLVED FIXED

(Mozilla Labs Graveyard :: Personas Plus, defect)

Description

[valentinatatya]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: 

Personas Plus for Firefox 4 can not rotate Personas Skin which used to work.

Reproducible: Always

Steps to Reproduce:
1. Install Personas Plus 1.6.2 from below URL. You may need to de-activate compability check of FF4 or download and edit the install.rdf to install the XPI.

https://bugzilla.mozilla.org/attachment.cgi?id=502806&action=edit

2. After Personas Rotator is installed type about:config in the address bar and press ENTER

3. Change the pref:

extensions.personas.category

value to:

Firefox

4. Change the pref:

extensions.personas.selected

value from

current

to:

random

Actual Results:  
The extensions.personas.selected value reverts back to current again. A notification bar is also shown for Personas change.

Expected Results:  
The extensions.personas.selected value should stay as random to activate the Personas rotation. If the value resets back to the current then the rotation stops. Also because we want to rotate the Personas automatically, the notification bar should not be shown to the user which may distract the users attention and closing the notification bar again and again may irritate the user.

This problem occurs on Firefox 4.0b10.
This problem does not occur on Firefox 3.6.13 even with the same XPI.

Comment 1

[Deb Richardson [:dria] (plz NEEDINFO)]

Jose: could you take a look at this?  If it's not caused by the add-on, I'll have to track down a regression range.

Comment 2

[Jose E. Bolanos]

There appears to be a difference in the Lightweight Theme Manager (LTM) in Firefox 3 and 4.
When a persona is changed, the add-on changes the currentTheme of the LTM. This causes the LTM to fire the "lightweight-theme-changed" notification, and so the add-on updates itself.

In Firefox 3, the "lightweight-theme-changed" notification is fired by the LTM once with the new selected theme, as it probably should be. Instead, in Firefox 4, the notification is being fired twice: first with a null theme, immediately followed by the selected them.

The latter causes the add-on to think that the default persona was selected, i.e. no persona, and resets the "selected" setting to "default". Then comes the second notification and the add-on sets the new theme as "current", losing the "random" setting.

Here are two case scenarios:

Firefox 3
1. "Random selection" is chosen on the add-on
2. The add-on sets its selected setting to "random", sets persona X, and changes the currentTheme of the LTM.
3. The LTM fires back the "lightweight-theme-changed" notification, passing persona X.
4. Since the add-on already has persona X set, the chain of events stops here.

Firefox 4
1. "Random selection" is chosen on the add-on
2. The add-on sets its "selected" setting to "random", sets persona X, and changes the currentTheme of the LTM.
3. The LTM fires back the "lightweight-theme-changed" notification, passing NULL.
4. The add-on currently has persona X set. Because the notification contains a NULL persona, the add-on changes the "selected" setting to "default", and removes the persona.
5. The LTM fires back the "lightweight-theme-changed" notification a second time, now passing persona X.
6. The add-on currently has no persona set. It takes persona X from the notification, and sets it as "current".


The idea solution would be to fix this on the LTM. If this is not possible, we would have to do a workaround on the add-on side.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Mossop/Blair: can you guys see if this is something you think we can get in Firefox 4, or if the Personas+ folks should consider a workaround?

Comment 4

[Dave Townsend [:mossop]]

I can see why this is happening but I wouldn't be confident of getting a safe fix in for Firefox 4. I've filed bug 634342 for our side but I'd recommend a workaround for the extension for now.

One simple option would be to make the call to set currentTheme to the random choice and then afterwards re-set the random option, the notifications should all get fired before the call to set currentTheme returns.

Comment 5

[Jose E. Bolanos]

As a workaround, we're not delaying the first notification just a little bit to allow any possible follow-up notification to cancel it. This way, the "random" setting is not lost.

In addition, the "Random from New & Featured" and "Random from Most Popular" actions were broken and have been fixed too.

We've committed the change here:
http://hg.mozilla.org/labs/personas/rev/daf6a9a8b754

Comment 6

[Deb Richardson [:dria] (plz NEEDINFO)]

What needs to be done to roll out another RC with these fixes included?

Comment 7

[Jose E. Bolanos]

The best would be to include these as part of a 1.6.2 RC2

Comment 8

[Deb Richardson [:dria] (plz NEEDINFO)]

Sounds perfect :)  Is there anything I can do to help with that?

Comment 9

[Jose E. Bolanos]

We'll create the RC, and let you know so you can push it to the distribution servers again. Thanks.