530106 - Lightweight theme is shown as enabled, despite personas extension changing the theme

Status: VERIFIED FIXED

(Mozilla Labs Graveyard :: Personas Plus, defect)

Description

[Tony Chung [:tchung]]

Attachment: Wrong used theme shown screenshot

Lightweight themes set as "used theme" will never get defaulted to what the personas extension theme is applied.  Is there a way to dynamically update this?

See screenshot

Repro:
1) install 192 nightly or b3.  
2) apply a lightweight theme, and verify theme manager has it set to "use theme" button
3) install personas extension 1.4
4) change themes using the extension
5) Verify theme manager still shows your first lightweight theme as "used theme", despite it differing from what the personas extension has overwritten.

Expected:
- some way to sync up the theme manager to know that personas extension is using its own theme.

Actual:
- lightweight theme gets "used" despite the persona extension overwriting it.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Sounds like something the Personas addon should be handling.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Agreed, it's the responsibility of the extension to keep the theme manager up-to-date with its changes to the lightweight theme.

Comment 3

[Juan Manuel Rodriguez Alvardo]

Attachment: proposed patch

This patch makes personas addon work in sync with addons/themes dialog. 

Now, changes in themes made using personas are reflected properly in the addons/themes dialog and viceverza.

There is one case where the current persona menuitem is not being updated properly. Howerver, we realized this issue will be addressed when the fix for Bug 530146 is landed, because the issue is caused by the lightweight-theme-changed topic being notified too many times. These are the steps to reproduce the problem:

- go to a light weight theme page at getpersonas (e.g http://www.getpersonas.com/en-US/persona/49)
- click wear this button

Current behavior

the current persona menuitem is not updated accordingly

Expected behavior

the current persona menuitem is updated accordingly

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 414405 [details] [diff] [review]
proposed patch

>+  /**
>+   * Notifies the persona changes or uses the lightweight theme manager
>+   * functionality for this purpose (if available)
>+   * @param aPersona the persona to be set as current if the lighweight theme
>+   * manager is available
>+   */

Nit: lighweight -> lightweight

This looks good and works well in my tests. r=myk

Comment 5

[Jose E. Bolanos]

Patch committed in changeset:
http://hg.mozilla.org/labs/personas/rev/9b88f78b0734

Comment 6

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b5pre) Gecko/20091202 Namoroka/3.6b5pre and personas 1.5 nightly.