532741 - Customizing a persona requires a header and footer for the changes to stick

Status: VERIFIED FIXED

(Mozilla Labs Graveyard :: Personas Plus, defect)

Description

[Tony Chung [:tchung]]

When customizing a persona, the user needs to include a header or footer before their persona becomes visible. Simply editing the Text or Accent color isn't enough.

Repro:
1) install personas 1.5 extension
2) enable custom persona setting
3) Edit the Text or accent color to something other then default
4) Verify unless you apply a header or footer, the custom persona will not be applied.

Expected:
- Editing the text or accent color should be applied, minus having a custom header or footer or not.

Comment 1

[Jose E. Bolanos]

This happens because the code now relies on the LightweightThemeManager object (when using Personas in Firefox 3.6+) to apply and preview personas, even custom ones. This object is probably enforcing all these properties to be set, instead of just applying the ones which are available (e.g. accent color without header image).

Comment 2

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

IMHO, custom persona bugs should in general not block the release of Personas 1.5, since custom personas are a relatively hidden feature that relatively few users use.

Comment 3

[Suneel]

(In reply to comment #1)
> This happens because the code now relies on the LightweightThemeManager object
> (when using Personas in Firefox 3.6+) to apply and preview personas, even
> custom ones. This object is probably enforcing all these properties to be set,
> instead of just applying the ones which are available (e.g. accent color
> without header image).

Jose, is there a workaround for this problem? Agree with Myk that this is not a blocker for 1.5, so targeting this fix to 1.6.

Comment 4

[Jose E. Bolanos]

(In reply to comment #3)
> (In reply to comment #1)
> > This happens because the code now relies on the LightweightThemeManager object
> > (when using Personas in Firefox 3.6+) to apply and preview personas, even
> > custom ones. This object is probably enforcing all these properties to be set,
> > instead of just applying the ones which are available (e.g. accent color
> > without header image).
> 
> Jose, is there a workaround for this problem? Agree with Myk that this is not a
> blocker for 1.5, so targeting this fix to 1.6.

The workaround would be to fall back to the old code when a custom persona is set. Ideally the change should be done in the lightweight themes code instead of the add-on code, but I guess for now we can do this.

Comment 5

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

Could we perhaps specify a data: URL containing a 1px by 1px transparent PNG for images that are missing?

Comment 6

[Jose E. Bolanos]

(In reply to comment #5)
> Could we perhaps specify a data: URL containing a 1px by 1px transparent PNG
> for images that are missing?

That's a great idea, definitely better than using the old code.

Comment 7

[Jose E. Bolanos]

Attachment: Transparent 1x1px image patch

This effectively fixes the issue, as Myk cleverly suggested. I submitted a patch for him to review because I'm not sure whether the placement and naming of the image constant is correct.

Comment 8

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

Comment on attachment 416097 [details] [diff] [review]
Transparent 1x1px image patch

This looks good, and the placement and naming seems fine. r=myk

Comment 9

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

Committed in changeset http://hg.mozilla.org/labs/personas/rev/0914c5ac2779.

Comment 10

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

Note: I included this fix in Personas 1.5rc2, which I spun earlier today:

https://ftp.mozilla.org/pub/mozilla.org/labs/personas/personas-1.5rc2.xpi

Comment 11

[Tony Chung [:tchung]]

Thanks for the quick fix guys.  Looks to be working now on Personas 1.5rc2.