Last Comment Bug 520124 - Redesign theme/background Ul, and allow applying a background (lightweight theme, Persona) to any theme (complete theme)
: Redesign theme/background Ul, and allow applying a background (lightweight th...
Status: ASSIGNED
[workaround see comment #123] p=0
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal with 44 votes (vote)
: ---
Assigned To: Blair McBride [:Unfocused] (UNAVAILABLE)
:
Mentors:
: 535526 541187 541648 542594 576713 647507 702927 (view as bug list)
Depends on: 574970
Blocks: 511104 530167 859729 902921 550048 577063
  Show dependency treegraph
 
Reported: 2009-10-01 23:52 PDT by Alfred Kayser
Modified: 2016-01-27 21:10 PST (History)
64 users (show)
mmucci: firefox‑backlog+
dtownsend: wanted1.9.2-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
personas listitem (98.75 KB, image/png)
2010-01-24 17:07 PST, Pardal Freudenthal (:ShareBird)
no flags Details
mockup (423.79 KB, image/png)
2010-08-10 08:11 PDT, Dave Townsend [:mossop]
no flags Details
backend patch rev 1 (64.67 KB, patch)
2010-08-22 16:29 PDT, Dave Townsend [:mossop]
no flags Details | Diff | Splinter Review
backend patch rev 1 (64.67 KB, patch)
2010-08-22 16:47 PDT, Dave Townsend [:mossop]
robert.strong.bugs: review-
Details | Diff | Splinter Review
browser patch rev 1 (12.95 KB, patch)
2010-08-22 16:48 PDT, Dave Townsend [:mossop]
gavin.sharp: review+
Details | Diff | Splinter Review
UI patch rev 1 (92.94 KB, patch)
2010-08-22 17:07 PDT, Dave Townsend [:mossop]
blair: review-
Details | Diff | Splinter Review
browser patch rev 2 (12.67 KB, patch)
2010-08-26 16:35 PDT, Dave Townsend [:mossop]
gavin.sharp: review-
Details | Diff | Splinter Review
backend patch rev 2 (71.11 KB, patch)
2010-08-26 16:46 PDT, Dave Townsend [:mossop]
robert.strong.bugs: review+
Details | Diff | Splinter Review
browser patch rev 3 (12.62 KB, patch)
2010-08-30 15:44 PDT, Dave Townsend [:mossop]
no flags Details | Diff | Splinter Review
UI patch rev 2 (92.43 KB, patch)
2010-08-30 16:14 PDT, Dave Townsend [:mossop]
no flags Details | Diff | Splinter Review

Description Alfred Kayser 2009-10-01 23:52:15 PDT
With the new lwtheme support, when selecting a 'persona', Firefox switch back to the default theme. However, I would like to keep my theme (e.g. Nautipolis) even when a lwtheme is enabled.
Comment 1 Alfred Kayser 2009-12-17 01:01:23 PST
The best way to do this is for a theme to indicate somehow that it supports Personas (either in chrome.manifest or in install.rdf).
Comment 2 Dão Gottwald [:dao] 2010-01-21 13:15:07 PST
*** Bug 541187 has been marked as a duplicate of this bug. ***
Comment 3 Dão Gottwald [:dao] 2010-01-21 16:06:17 PST
*** Bug 541187 has been marked as a duplicate of this bug. ***
Comment 4 Ryan Doherty (:rdoherty) 2010-01-22 14:19:03 PST
+1 for this, gotten a lot of feedback for this in firefox & personas newsgroups.
Comment 5 Dave Townsend [:mossop] 2010-01-22 14:25:52 PST
The biggest problem I have with this right now is making the UI explanatory enough so that users aren't confused over why they have two different ways to modify their appearance. We already have an overload of too many different add-on types some of which are mostly overlapping as far as the user is concerned, I think this would just make things worse.
Comment 6 mick1 2010-01-22 16:41:11 PST
+1 for this
Comment 7 Dave Townsend [:mossop] 2010-01-22 17:03:51 PST
Please note that saying "+1" in a bug report will not make it more likely to be fixed, just clog up the report with spam and make it harder to work on.
Comment 8 Ryan Doherty (:rdoherty) 2010-01-22 17:10:54 PST
(In reply to comment #5)
> The biggest problem I have with this right now is making the UI explanatory
> enough so that users aren't confused over why they have two different ways to
> modify their appearance. We already have an overload of too many different
> add-on types some of which are mostly overlapping as far as the user is
> concerned, I think this would just make things worse.

I agree, we do have many add-on types, a lot of work is necessary to make add-ons (and themes/personas) easier to understand. We may need to re-label themes into add-ons and/or personas to themes along with changing the add-ons manager UI. (this is outside my area of expertise/control). 

Technically, a flag in a theme that states it's compatible with personas seems like a good idea.
Comment 9 Alfred Kayser 2010-01-23 01:54:52 PST
The solution about making a more clear distinction is simple.
Place Personas in a 'Personas' tab,
and keep Themes in the 'Themes' tab.

Any other change is only about more change, and change is always confusing.
Note that in all Firefox 3.6 news, Personas are called Personas, and in the newsgroup they are still referred to as Personas.
So, keep Personas as Personas, and Themes as Themes, both are now know names/concepts, and switching things around only makes it more confusing...
Comment 10 Dão Gottwald [:dao] 2010-01-23 04:27:25 PST
*** Bug 541648 has been marked as a duplicate of this bug. ***
Comment 11 hiryu 2010-01-23 13:35:57 PST
(In reply to comment #9)
> The solution about making a more clear distinction is simple.
> Place Personas in a 'Personas' tab,
> and keep Themes in the 'Themes' tab.


Totally bad idea for me. Previously most theme authors posted info about compatibility with Personas (similiar to Ryan idea) and most users knew what is what. Creating TWO ways of changing appearance will be super confusing.
Either create a flag for themes which supports Personas or mark it as experimental and allow only advanced users to change how LW & HW motives work together.
Comment 12 Dave Townsend [:mossop] 2010-01-23 14:20:15 PST
(In reply to comment #11)
> (In reply to comment #9)
> > The solution about making a more clear distinction is simple.
> > Place Personas in a 'Personas' tab,
> > and keep Themes in the 'Themes' tab.
> 
> 
> Totally bad idea for me. Previously most theme authors posted info about
> compatibility with Personas (similiar to Ryan idea) and most users knew what is
> what. Creating TWO ways of changing appearance will be super confusing.
> Either create a flag for themes which supports Personas or mark it as
> experimental and allow only advanced users to change how LW & HW motives work
> together.

Even if we make themes have a flag marking them as compatible with personas (as opposed to just assuming all work with personas presumably), we would still need to separate them out in the UI some way that makes it clear to users that they do indeed have two different ways to change the appearance. Without that we won't be adding this capability to Firefox.
Comment 13 Dave Townsend [:mossop] 2010-01-23 14:21:18 PST
(In reply to comment #9)
> The solution about making a more clear distinction is simple.
> Place Personas in a 'Personas' tab,
> and keep Themes in the 'Themes' tab.

That is the most obvious solution perhaps, but I don't like it since it just leads us back down the path of having so many different types of add-ons that users are overwhelmed. I'm willing to hear a UX person say it is a good idea though.
Comment 14 hiryu 2010-01-23 16:07:51 PST
Let's try to introduce a way to handle this all.
Technically "Theme" changes whole Firefox look, from bars to buttons and even menu look. Personas on the other hand simply adds transparency or something called Glass effect in Vista/7. Personas is just background image.
In short we could say that Themes are hardware change while Personas are only software changes (it's a metaphor ofc).

It's only proposition but maybe Mozilla should make sure ALL themes are working with Personas and those which are not should be removed from database ?
Or maybe merge themes with personas as Enhanced themes where users have the option to choose background image (and font color) ?
Then we have one Themes section but not all of them would be enhanced.
What do you think ?
Comment 15 hiryu 2010-01-23 16:09:29 PST
Or simply there could be themes and themes + of course all inside Themes catalog and without changing how add-on manager looks since we all know changes are confusing and Mozilla is starting to pull-off way too much changes without asking users what they feel about it.
Comment 16 Dave Townsend [:mossop] 2010-01-23 16:12:08 PST
(In reply to comment #14)
> Let's try to introduce a way to handle this all.
> Technically "Theme" changes whole Firefox look, from bars to buttons and even
> menu look. Personas on the other hand simply adds transparency or something
> called Glass effect in Vista/7. Personas is just background image.
> In short we could say that Themes are hardware change while Personas are only
> software changes (it's a metaphor ofc).
> 
> It's only proposition but maybe Mozilla should make sure ALL themes are working
> with Personas and those which are not should be removed from database ?
> Or maybe merge themes with personas as Enhanced themes where users have the
> option to choose background image (and font color) ?
> Then we have one Themes section but not all of them would be enhanced.
> What do you think ?

I don't think that solves the problem at hand. The issue is how to represent in the user interface that the user can choose from one theme and one persona (background image if you will).

Of course this becomes even more complicated if we actually go where we are talking about and make it possible for personas to change the toolbar icons as well.
Comment 17 mcdavis941 (sporadically reading bugmail) 2010-01-23 16:38:26 PST
The way it works right now makes the persona preview function modal (depending on whether you have a traditional theme active or not).  It seems like users are lead to expect non-modal, always-work previews.  (I haven't seen anything from Mozilla telling them otherwise.)  The number of users impacted is a minority of the overall user base, but still millions of people.

On the plus side, it makes persona-plus-traditional-theme compatibility problems a non-issue, because you never have both at the same time.  It's simple and it works.  It keeps users from ever seeing Firefox looking bad when they're previewing a persona with a theme not written to support previews.  (That last bit could be helped if AMO editors doing theme reviews check persona compatibility, or with a flag like Alfred is talking about.)

For reference, was there a discussion (e.g., in a bug) in the run-up to 3.6 that discussed pros and cons of making traditional themes and personas work at the same time?
Comment 18 mcdavis941 (sporadically reading bugmail) 2010-01-23 16:48:01 PST
(In reply to comment #17)
> The number of users impacted is a
> minority of the overall user base, but still millions of people.


No wait, that's not right.  Millions of theme users, but not yet using 3.6.
Comment 19 Pardal Freudenthal (:ShareBird) 2010-01-23 16:50:47 PST
I think we could add a kind of personas icon to the lwthemes to distinguish them from full themes.

I've written a little extension, that allows switch full themes without a restart with this modifications on the UI (it's a WIP):
www.silvermel.net/dev/xpi/switchthemes_0.1.0pre_trunk_r3246.xpi

But I am still thinking, if Personas will work with third party themes, the best place for it is still the menu. Now we have the Personas listed two times: at menu and at Addons Manager.

But, if Personas will be able to change also the toolbar icons, the whole thing will be more complicated. Even with this ability, Personas will not substitute third-party themes. And I can't see how third-party themes will be able to support changes on toolbar icons. Themes are also a platform to authors to present their icons artwork. In this case I think the best is to Personas become a feature from the default theme (and having a UI like proposed on Bug 59729) and then another extension like Personas 1.4 could supply the old behavior for using with third-party themes.
Comment 20 hiryu 2010-01-23 17:18:03 PST
No No NO - adding Personas ability to change icons is against it's main advantage - small, light themes/background images or whatever we call it.

Reply to Comment #17 :
" It's simple and it works.  It keeps users from ever seeing Firefox looking bad when they're previewing a persona with a theme not written to support previews. "
Wouldn't be easier that author of theme which doesn't support Personas simply block the preview function ?

Personas should stay as they're now - it's like changing wallpaper, while themes are changing everything EXCEPT wallpaper.
Maybe the same way of functionality should be proposed to users ? It would be rather easy for them since all users changes their wallpapers and most changed folder icon at least once.
Comment 21 Alfred Kayser 2010-01-24 06:00:50 PST
A couple of points here:
1. Personas are something else than Themes, because "they are simpler". 
2. Themes can support Personas (or block it quite easily).
3. Themes can change much more in the UI.

My take on this:
A. Separate Themes and Personas, with a separate tab in Addons window.
B. Allow Themes to support Personas (and vice versa!).
Comment 22 hiryu 2010-01-24 15:20:45 PST
It's quite odd for me to just a plugin/add-on/extension or whatever Personas is should get own tab in addons window. Sorry but PErsonas it's not the only add-on which can change firefox look without completelly changing FF appearance and os it simply doesn't "deserve" to have a tab.

I'm still voting to mark themes with personas support as Themes+ , Enhanced themes or Customizable themes. 

P.S - What's Mozilla doing anyway ??!! Attacking themes authors is not the way to go - Yo Moz do you really want to make us Chrome users ?
We the community of users & authors are your advantage - without us your income from google search would be really small !!
I'm really disgraced how Mozilla is starting to behave - maybe you feel to comfortable ?
Comment 23 Pardal Freudenthal (:ShareBird) 2010-01-24 17:06:20 PST
I can't see a not confusing way to make an easily understandable UI for the case "Personas working together with third-party themes" if Personas will be listed together with full themes. This scenario, personas + full themes will imply on something like a multiple choice: aTheme + aPersona (or aTheme without any persona, the "old" Default by Personas).

As it is now, the UI doesn't permit a multiple choice, the personas are listed exactly like the other themes (BTW, I'm attaching a img with an approach I'm using on the extension that differentiates a little personas from full themes). The button "Use Theme" implies on a single choice.

This means we have only two choices (I'm unable to imagine another one): either a new tab with personas that the user can choose a persona secondarily after choose a preferred full theme, or, sorry if I repeat myself, the menu. I know that we are trying to make the menu less populate, but we don't need to install twenty extensions to get ten more items on menu at all...
Comment 24 Pardal Freudenthal (:ShareBird) 2010-01-24 17:07:28 PST
Created attachment 423290 [details]
personas listitem
Comment 25 hiryu 2010-01-24 18:18:07 PST
Then we should stick to previous version when Personas was on Extension Tab - I didn't heard many complaints about that.

btw when we are talking about confusing - just look at Add-on window.
Tab 1: Get Add-ons. So users enters it and chooses some add-on to install.
After this he is moved to Tab 2: Extenstion where choosen add-on asks for a restart :D So in short - user click on install Add-on but installs extension - is this a confusion ? ;)
Comment 26 Dave Townsend [:mossop] 2010-01-24 21:57:08 PST
(In reply to comment #19)
> But I am still thinking, if Personas will work with third party themes, the
> best place for it is still the menu. Now we have the Personas listed two times:
> at menu and at Addons Manager.

Can you elaborate on this, what menu are you referring to?

(In reply to comment #24)
> Created an attachment (id=423290) [details]
> personas listitem

I hope you agree that this is not something that makes sense to new users.
Comment 27 Alfred Kayser 2010-01-25 00:10:41 PST
The last attachment (id=423290) [details] [details]
shows that Personas are a different thing from Themes, and putting them in their own list in their tab makes it more clear to users.
This also allows an user to select a theme in the Themes tab, and a personas in the Personas tab.
Comment 28 MoJo Butters 2010-01-25 01:37:06 PST
Someone (Tiribulus on MozillaZine) mentioned reverting back to an older version of the personas add on (1.4) and for mouse overs it worked perfect... Not sure about the themes and personas working together problem but he seemed to think it would work. Give it a go...

https://addons.mozilla.org/en-US/firefox/addons/versions/10900#version-1.4
Comment 29 Pardal Freudenthal (:ShareBird) 2010-01-25 05:33:22 PST
(In reply to comment #26)
> (In reply to comment #19)
> > But I am still thinking, if Personas will work with third party themes, the
> > best place for it is still the menu. Now we have the Personas listed two times:
> > at menu and at Addons Manager.
> 
> Can you elaborate on this, what menu are you referring to?
> 
Sorry for this, Dave... My mistake. Testing one of the betas, I was asked to install the Personas extension to get the ability to use it on 3.6 beta. I erroneously thought this would be the default behavior. 

> (In reply to comment #24)
> > Created an attachment (id=423290) [details] [details]
> > personas listitem
> 
> I hope you agree that this is not something that makes sense to new users.
This is a good point. And I guess the matter here is not on how discoverable is to know what personas are, but to know what full themes are, on a new user perspective.
Comment 30 Joe 2010-01-25 12:14:20 PST
Here are some ideas: What about a button that appears only on Persona-compatible themes, when you click it it brings up a popup that allows you to select a persona? 

Or if that is too complicated, what about multiple tabs under the "Themes" tab? 

The default tab would be "Full Themes", but there would be other tabs like "Personas" and "Icons" for users to tweak. 

In any case, I think we might be going at things the wrong way, trying to figure out through discussion what would be the best UI. Let's create an addon that allows users to mix and match themes and personas under a special menu; that way no new users will be confused, and users who can deal with the extra complication can mix and match themes with personas. Let the community and the users of the addon propose ideas for better UI options; in my opinion, it's not necessary to make Themes + Personas capabilities a default for firefox just yet. But if we allow people to test that functionality out, they will undoubtedly come up with a better UI for it.
Comment 31 Alfred Kayser 2010-01-25 13:00:58 PST
Or, as Personas are now a standard feature/functionality of Firefox, any theme that themes Firefox 3.6 simply has to support it (or override it, if really needed).

Even my Walnut themes with wood background can work with Personas (such as Deep Forest, achieving great effect).
Comment 32 Bob Primak 2010-01-26 21:08:44 PST
My $.02 worth:

Alfred Kayser is on to something. Personas now seem to want to override the current Theme, no matter which Theme it is. Sometimes this works seamlessly, but other times... !!!

Let me relate a disaster which occurred because Personas tried to override a Theme (Nautipolis) which was not yet ready for the new Personas Extension for Firefox 3.6.

I upgraded from FF 3.5.7 to FF3.6, but did not stick with the Default Theme. I updated all my Themes, uninstalled incompatible ones (flagged), and selected Nautipolis. When I then selected a Persona to "wear", it took over my Theme. But not completely!  From then onwards I could not see the contents of the Downloads, History, or Add-ons windows. I could not uninstall Personas because the Add-ons window was empty of contents. I had to completely remove Firefox (using Revo Unninstaller) and start over (with a new, default profile) because of this event. 

My issue went away only after a full uninstall and reinstall of Firefox,
effectively creating a new Profile. There appears to be a conflict between some of the Firefox Themes and the new Firefox 3.6 Personas features. The Themes feature has to be completely disabled before Personas will work properly, and the upgrade installer did not fully disable the Themes feature before installing the new Personas. Hence the conflict which messed up the entire Firefox graphical interface. Or so I have concluded so far. Thank God for the FEBE extension, or else I'd still be rebuilding my Profile. Needless to say, I made a new FEBE backup right away after cleaning up this mess! 

The bug showed up immediately after a Firefox 3.5.7 to Firefox 3.6 upgrade in an Administrator Account under Windows XP Professional. 

I assure you that this issue is very real, and did involve the Personas plug-in creating a conflict with a Firefox 3.6 "compatible" Theme (Nautipolis) which did in fact produce some sort of corruption of the current user's Firefox Profile when any Persona was superimposed upon it. This issue is, unfortunately, easily reproducible until Firefox is restarted after the Persona is selected. 

My workaround is to leave the default theme intact, as FF 3.6 starts us out when we upgrade. First select to wear a Persona, and then look to see if there is an available Theme. Usually, there is not, and clicking on "Get Themes" takes us only to the Personas pages. I believe this is by design, and is a good idea. Once Personas are established, the Themes no longer even show a "Use This" or "Uninstall This" button in my FF 3.6. But a right-click on any Theme allows uninstallation only, which I did for all my Themes so as to concentrate on using Personas instead. To get a Compact Theme, I used the Customize Toolbars pop-up window, and selected at the lower-left the Small Icons With Text option. This narrows the Toolbars almost as much as any compact Theme ever did, and allows Personas to be installed at safely and will. This is a good workaround, as long as Personas vs. Themes has this potential for conflict and Profile corruption.

By the way, I do prefer Personas over Themes, if I have to choose one or the
other. Selecting ANY Persona while Themes is still enabled can cause at
least some Profile corruption, in my observations so far. It's the features
themselves which are not always compatible with each other, I think.
Comment 33 Joe 2010-01-27 12:16:51 PST
I tried to replicate your problem by installing Firefox 3.6, installing Nautipolis, and then using a Persona, but the addons and downloads windows display fine.  Is there anything else you might have done that could have triggered the problem?
Comment 34 Bob Primak 2010-01-27 13:01:52 PST
The initial upgrade to Firefox 3.6 may have been involved. This was immediately after upgrading -- since restoring the Profile, I have not been able to select any non-default Theme when Personas was active. The buttons were simply not there. I do have security programs which limit programs' ability to modify certain System Files and Registry Entries in Windows XP Prop SP3. But it looks from various BugZilla reports as if the bug I have experienced is reproducible on a variety of Windows PCs in a variety of configurations. This thread contains lesser problems, but the profile corruption issue does have several BugZilla threads of its own. 

I am not saying this bug will always show up in every conceivable hardware and software configuration -- only that it is widespread and fatal, so everyone should be on the lookout in case they might be vulnerable and not know it from the FF 3.6 Release Notes. Those Notes are not explicit about the profile corruption risk when first applying Firefox Personas under Firefox 3.6 on top of a non-Default Theme.  

Joe, you do not say which Windows version you are using, so I cannot evaluate your reply further. 

My principle security software is Avast 5.0 (also happens with Version 4.8) and the Comodo Firewall with Defense+. Disabling security programs just to get Firefox to work is not an option.
Comment 35 hiryu 2010-01-27 13:53:09 PST
(In reply to comment #34)
> The initial upgrade to Firefox 3.6 may have been involved. This was immediately
> after upgrading -- since restoring the Profile, I have not been able to select
> any non-default Theme when Personas was active. The buttons were simply not
> there. I do have security programs which limit programs' ability to modify
> certain System Files and Registry Entries in Windows XP Prop SP3. But it looks
> from various BugZilla reports as if the bug I have experienced is reproducible
> on a variety of Windows PCs in a variety of configurations. This thread
> contains lesser problems, but the profile corruption issue does have several
> BugZilla threads of its own. 
(...)
> My principle security software is Avast 5.0 (also happens with Version 4.8) and
> the Comodo Firewall with Defense+. Disabling security programs just to get
> Firefox to work is not an option.

It's quite odd since I run much advanced protection - Kaspersky Internet Security 2010 which sometimes is really killin' me as it's able to block installation of add-ons and there is no information about block. Simply add-on won't install and you left without any help. Nonethless that KIS blocked installing FoxyTunes and all add-on's which somehow go outside Firefox (foxy tunes contacts with Winamp) are blocked it doesn't gave me the error you said. Immidiatelly after I choosen Personas I received info that I need to restart FF cause of theme change (on the beggining I though this behaviour is due to KIS weird protection system but sadly it's not) but after restart theme indeed returned to default one without any problems. But it's possible that my profile was quite clean since after format I only installed FF 3.5.7 and then after few days FF 3.6 so maybe that's the reason.
Comment 36 hiryu 2010-01-27 13:55:37 PST
(In reply to comment #31)
> Or, as Personas are now a standard feature/functionality of Firefox, any theme
> that themes Firefox 3.6 simply has to support it (or override it, if really
> needed).
> 
> Even my Walnut themes with wood background can work with Personas (such as Deep
> Forest, achieving great effect).

Sadly Alfred you're copied my idea and now it's your idea ? Quite odd.
Reminder - Comment#14 "It's only proposition but maybe Mozilla should make sure ALL themes are working
with Personas and those which are not should be removed from database ?"

We both talking about exactly the same feature
Comment 37 mcdavis941 (sporadically reading bugmail) 2010-01-27 16:36:45 PST
*** Bug 542594 has been marked as a duplicate of this bug. ***
Comment 38 Bob Primak 2010-01-28 00:44:29 PST
@hiryu -- 

Sorry to read that KIS is causing such problems. But that is irrelevant to my issues with Themes and Personas.(blocking of add-ons) That's one big reason I dumped Norton 360, v.2.0 two years ago. The Big Guys do not understand the difference between friend and foe. And they leave the End User with very few options to allow temporary or permanent exceptions. That is a Security Vendor issue, not a Firefox issue. 

But this is why I finally decided to stick with a free, limited anti-virus -- Avast Free blocks only what it has to to keep me safe, if I exercise reasonable cautions when at questionable web sites. Same with the Firefox NoScript plug-in. They do not block what I want to download -- only what I DO NOT want. Alongside of a good third-party firewall (Comodo with Defense+), Avast Free is about all the protection I need -- without blocking Firefox Personas or Add-ons. And if I need to make exceptions, both Avast and Comodo will let me do so -- permanently or temporarily. Not much digging needed in either case.

Now let's get back on topic, OK?
Comment 39 Bob Primak 2010-01-28 00:47:06 PST
To All --
 BTW, it's the fact that Firefox Add-ons are usually unsigned which sets off the Security Suites. Even Comodo Defense+ makes me dismiss two Warnings before I can install even a Persona. FF Add-on developers really need to get on track with this issue.
Comment 40 Bob Primak 2010-01-28 00:53:23 PST
(In reply to comment #33)
> I tried to replicate your problem by installing Firefox 3.6, installing
> Nautipolis, and then using a Persona, but the addons and downloads windows
> display fine.  Is there anything else you might have done that could have
> triggered the problem?

I should not accuse Nautipolis of not being ready for Personas in FF 3.6. That goes beyond the scope of my observations so far. But the bug I found is still very real, and is not restricted to just one Theme.
Comment 41 Alfred Kayser 2010-01-28 01:24:38 PST
Two comments:
1. I have always tried to make my themes compatible with a lot of extensions, and Personas is one of them (So, Hiru, comment #36, this is not stealing your idea, just allowing an extension to work with my themes).
2. The problems described are NOT caused by Nautipolis, but by Firefox trying to switch back to the default theme, which failed somehow.
Comment 42 Bob Primak 2010-01-28 01:42:02 PST
(In reply to comment #41)
> Two comments:
> 1. I have always tried to make my themes compatible with a lot of extensions,
> and Personas is one of them (So, Hiru, comment #36, this is not stealing your
> idea, just allowing an extension to work with my themes).
> 2. The problems described are NOT caused by Nautipolis, but by Firefox trying
> to switch back to the default theme, which failed somehow.

I believe you, Alfred. My conclusion is that Fireox 3.6 is not supposed to be switched out of its default theme when using the new Personas Extension. At least not on my computer. I apologize for any implication that Nautipolis is a source of this conflict -- it clearly is not uniquely involved.
Comment 43 Dave Townsend [:mossop] 2010-01-28 09:10:58 PST
All of the previous comments about bugs seen when trying to use personas on non-default themes and issues with AV tools are really off-topic, do not belong here and make it much harder for developers to read this bug making it less likely it will be fixed. If you are seeing bugs in the current version of Firefox then file them separately.
Comment 44 Bob Primak 2010-01-28 16:06:09 PST
I disagree strongly with this statement:(In reply to comment #43)
> All of the previous comments about bugs seen when trying to use personas on
> non-default themes and issues with AV tools are really off-topic, do not belong
> here and make it much harder for developers to read this bug making it less
> likely it will be fixed. If you are seeing bugs in the current version of
> Firefox then file them separately.

I was not aware that this thread is reserved for Firefox Developers only. And that these Developers are not interested in the experiences of actual non-technical Firefox everyday users. 

If a bug is indeed showing up when the Personas are applied on top of the Themes, this is extremely relevant to present and future development of both of these features. 

If the current schemes are in fact not always working well together, how can Firefox developers possibly entertain the notion of moving forward in developing the ability to seamlessly integrate Personas with Themes?

Perhaps some of us have gotten too specific about our own examples of how the Personas and Themes are not integrating well now, but as for myself, I want to see this issue included in the developers' knowledge about how real Firefox users in the real world are finding that the Personas and Themes are not well integrated right now. 

You cannot add anything of value to either feature if the present implementations of both features seem to be causing conflicts with each other.

But if the Moderators insist, I can copy all of my own comments here to a new bug report, if I have permission to open a new bug report. I think I know the correct button on the main Bugzilla page to try to do this. 

I was only trying to make clear that Personas and Themes need to be better integrated if Personas is now going to become a core feature of Firefox  itself, and not merely another Firefox Extension, as it was before Firefox 3.6 was released to the public. More development was obviously needed on Personas before it was added to the core feature set of Firefox 3.6. 

I was not the one who introduced Antivirus or other security products into this discussion. I was replying to another comment when I offered details relevant to my own configuration when the bug appeared. Perhaps that was off topic here, but the existence of this possible conflict is NOT off topic as I read the thread before I added to it. If this is a widespread bug (and it may or may not be widespread), it does affect future development of closer integration of Personas and Themes. 

If Moderators here are concerned about off topic comments, they should be able to: (a) delete irrelevant comments and notify "abusers" by e-mail; and (b) move off topic comments to new Bugzilla threads. Those two changes here would avoid confusing the developers if comments stray too far from narrowly-defined threads. 

I thought I was adding something of value -- obviously, the Bugzilla Moderators feel otherwise. Goodbye.
Comment 45 Dave Townsend [:mossop] 2010-01-28 16:29:23 PST
(In reply to comment #44)
> I disagree strongly with this statement:(In reply to comment #43)
> > All of the previous comments about bugs seen when trying to use personas on
> > non-default themes and issues with AV tools are really off-topic, do not belong
> > here and make it much harder for developers to read this bug making it less
> > likely it will be fixed. If you are seeing bugs in the current version of
> > Firefox then file them separately.
> 
> I was not aware that this thread is reserved for Firefox Developers only. And
> that these Developers are not interested in the experiences of actual
> non-technical Firefox everyday users.

This is a bug report, not a discussion forum. If you want to talk about related things please take it to the discussion forums, if you want to report a bug that you are seeing in Firefox (even if it is on a similar topic) please file a new bug report so it can be tracked and managed in its own right. If we aren't careful about these things and let bug reports run off topic they become difficult to read, developers become less inclined to work on them and important points get missed.

> If a bug is indeed showing up when the Personas are applied on top of the
> Themes, this is extremely relevant to present and future development of both of
> these features.

Firefox doesn't currently support applying personas on top of custom themes so either you are seeing this with the personas extension, in which case you should file a bug report let them know what issues you are seeing (since their implementation is what drives the in-product support), or you are seeing a bug where Firefox is letting you apply a persona on a custom theme, in which case you should file a bug report so that we can stop that happening for now since it isn't meant to work.

> Perhaps some of us have gotten too specific about our own examples of how the
> Personas and Themes are not integrating well now, but as for myself, I want to
> see this issue included in the developers' knowledge about how real Firefox
> users in the real world are finding that the Personas and Themes are not well
> integrated right now.

Assuming we move forward with allowing personas to apply on custom themes (and there is still no guarantee of that yet) it will mostly be up to the theme developers to ensure their themes work well with personas, there won't be a lot we can do from the product side.

> I was only trying to make clear that Personas and Themes need to be better
> integrated if Personas is now going to become a core feature of Firefox 
> itself, and not merely another Firefox Extension, as it was before Firefox 3.6
> was released to the public. More development was obviously needed on Personas
> before it was added to the core feature set of Firefox 3.6. 

I have seen no serious bug reports suggesting that there are any issues with the in-product support of personas.

> I was not the one who introduced Antivirus or other security products into this
> discussion.

My previous comment was not just directed at you.

> If Moderators here are concerned about off topic comments, they should be able
> to: (a) delete irrelevant comments and notify "abusers" by e-mail; and (b) move
> off topic comments to new Bugzilla threads.

We avoid deleting comments unless they are sensitive or abusive, we can notify people by email (and I have already done so for others in this report) however so many comments were here that I felt it important to make a comment in the bug to avoid other people piling in as tends to happen.
Comment 46 Bob Primak 2010-01-28 21:59:15 PST
I submitted Bug Report 542917, and was told it looks like my report does in fact belong in this thread. (Tyler Downer (Triage)):

"Third party themes can not be skinned with a persona. Persona will default
icons, etc. back to the default theme before skinning the browser. that is bug
520124 Is this what you mean?" 

Please resolve this runaround, or have Tyler Downer resolve it, could you? I definitely do not want to "pile on" as you put it.
Comment 47 hiryu 2010-01-29 08:48:02 PST
(In reply to comment #43)
> All of the previous comments about bugs seen when trying to use personas on
> non-default themes and issues with AV tools are really off-topic, do not belong
> here and make it much harder for developers to read this bug making it less
> likely it will be fixed. If you are seeing bugs in the current version of
> Firefox then file them separately.

Sadly NONE developer is interested in this problem as I received information from one of AMO team members with Mozilla statement that they think current solution is good and basically they don't give a s**t about users anymore.
Yes Mozilla = Microsoft so we can close this thread as they won't be fixing/changing current situation.

Regards for all community members :) Maybe someday "Old mozilla" will come back...
Comment 48 Bob Primak 2010-01-29 10:04:34 PST
@hiryu --

Please join me in moving all non-technical discussions to the Firefox User Support Forums. This kind of remark will not get this bug fixed any faster. I apologize for my own role, as I should have gone to Support first.
Comment 49 Dave Townsend [:mossop] 2010-01-29 10:51:25 PST
(In reply to comment #47)
> Sadly NONE developer is interested in this problem as I received information
> from one of AMO team members with Mozilla statement that they think current
> solution is good and basically they don't give a s**t about users anymore.
> Yes Mozilla = Microsoft so we can close this thread as they won't be
> fixing/changing current situation.

I don't know who it was that you heard from, but it clearly wasn't the people in charge of AMO (who I was in a meeting with yesterday and really want to see this happen) or the people in charge of making product decisions for Firefox and so really don't have the final word on what will/won't happen. If there had been a final decision not to implement this then I would have closed this bug as WONTFIX a long time ago.

That said before I read your comment this morning I had decided that I was going to close this bug report anyway, not because we're definitely not going to implement this but because the bug report has really turned into a big discussion thread, and bug reports aren't great places to have discussions about how a feature might be implemented, they are better for actually implementing something we have decided on.

I have opened two threads on the newsgroups to start some discussion on what I think are the two key questions that would need to be answered before this could be implemented. How would this be represented in the UI http://bit.ly/9tZdlA? Should themes be able to opt in/out of personas support http://bit.ly/c1hAML?

I've tried to include any points that were already brought up here but I hope you will all join in the discussions there and add in any of your points that I missed. Hopefully the threads can reach some kind of conclusion and then a clean bug report can be opened to track the implementation if the product drivers agree that we want this in Firefox.
Comment 50 Ryan Doherty (:rdoherty) 2010-02-05 16:36:44 PST
*** Bug 535526 has been marked as a duplicate of this bug. ***
Comment 51 Christie Bella 2010-03-23 16:43:15 PDT
this whole thing is ridiculous- everything was FINE until 3.6- "Personas vs/+ Themes" became confusing when MOZILLA Developers/TPTB made it confusing- Personas are skins or "Personalities" or "dress-up" for themes- it really is that simple- who's idea was it to then put Personas into the Theme manager, while at the same time, keeping it as an add-on/extension as well?? UNBELIEVABLE! THEMES are THEMES- they have the ability to alter the entire UI, whereas Personas can ONLY add a simple background to the UI- Personas are essentially toolbar (and statusbar) skins- Personas make sense as an EXTENSION- an extension that allows users to customize the BACKGROUND of their toolbars/status bar- again, it really is that simple- at least it was...
Comment 52 Roberto Ciang 2010-03-26 20:18:15 PDT
Personas is great. But it should not be mixed with themes. Please redesign the mechanism so that Personas is just an overlay (or background) on/under any themes.
Comment 53 Pardal Freudenthal (:ShareBird) 2010-05-24 17:52:49 PDT
Anything new on this bug? How are the discussions on Google Groups going on?
Comment 54 Pardal Freudenthal (:ShareBird) 2010-06-08 14:54:11 PDT
This bug has been temporarily closed for discussions on Google Groups for more than five months ago. In the meantime I believe we could collect some ideas of how this can implemented in the preceding discussions.
Already there are some mockups of how the UI for Wallpapers and Themes can look:
https://wiki.mozilla.org/Firefox/Projects/Extension_Manager_Redesign/design#Wallpapers_and_Themes_View

It seems that this is an issue that we want to solve, so, I'm requesting the reopening of this bug for inclusion in the work of the design for the new Addons Manager. I hope it's ok.
Comment 55 Dave Townsend [:mossop] 2010-07-03 06:46:36 PDT
*** Bug 576713 has been marked as a duplicate of this bug. ***
Comment 56 Alfred Kayser 2010-07-20 23:49:31 PDT
Allowing themes with Personas MUST be in FF4.0, so requesting blocking2.0
Comment 57 Dave Townsend [:mossop] 2010-07-21 08:55:28 PDT
Really wanted yes, and I am giving up bits and pieces of my spare time to try to make it happen, but this is not a blocker.
Comment 58 Dave Townsend [:mossop] 2010-08-07 11:39:31 PDT
Based on the number of blockers that we can fix by doing this I can now call this a blocker.
Comment 59 Dave Townsend [:mossop] 2010-08-10 08:11:36 PDT
Created attachment 464426 [details]
mockup

  - Themes are displayed first, backgrounds second - both groups are
collapsible
  - Each entry includes:
       * Add-on name
       * Add-on preview
       * Add-on author (links to AMO profile)
  - Clicking an add-on selects that entry; multi-select is enabled
  - When an add-on is selected, it displays:
       * An X in the top right corner for deletion
       * A "more" link (launches Detail View of add-on)
       * A link to wear that add-on
- If the user has only installed backgrounds, no themes display (and vice
versa)
Comment 60 Ahmed Aly 2010-08-19 22:52:42 PDT
okay guys i have a proposal for addon manager presentation of themes + personas
listing themes on left of Themes addon manager tab ; building border + buttons preview pane right to it ; providing a hollow box inside preview with personas boxes ; updating preview pane as personas are mouse hovered
Comment 61 Dave Townsend [:mossop] 2010-08-22 16:29:56 PDT
Created attachment 468186 [details] [diff] [review]
backend patch rev 1

This separates out lightweight themes into their own add-on type and removes all the code necessary for synchronizing the active theme between XPIProvider and LightweightThemeManager.

LightweighThemeManager has a slight API change, setting currentTheme will throw an exception if XPIProvider (or anyone else) vetoes the change. This is important since we want to display a doorhanger in this case and its better for the frontend code making the change to do that rather than the backend code (which doesn't know what browser is actually requesting it).

In XPIProvider we have to cache whether lightweight themes are allowed with the selected theme. This is done in the same way that we remember the selected skin name using a preference for the current value and a preference for the new value.
Comment 62 Dave Townsend [:mossop] 2010-08-22 16:47:46 PDT
Created attachment 468187 [details] [diff] [review]
backend patch rev 1

Correct version with some comment changes.
Comment 63 Dave Townsend [:mossop] 2010-08-22 16:48:55 PDT
Created attachment 468188 [details] [diff] [review]
browser patch rev 1

Patch for Firefox that makes it show the doorhanger notification when a lightweight theme install is blocked and offer to switch to the default theme in that case.
Comment 64 Dave Townsend [:mossop] 2010-08-22 17:07:52 PDT
Created attachment 468193 [details] [diff] [review]
UI patch rev 1

The main UI patch, dependent on a bunch of hte other UI patches. Couple of things probably still need to be resolved, like how to localize the default background name and creator (or make it so other apps can change it), and the references to the default theme's ID troubles me.
Comment 65 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-08-23 15:56:59 PDT
Comment on attachment 468188 [details] [diff] [review]
browser patch rev 1

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>+  _install: function (newLWTheme) {

>+    catch (e) {
>+      // If the switch was disabled because of using a theme that doesn't support
>+      // lightweight themes then offer to switch to the default theme
>+      if (e.result == Components.results.NS_ERROR_NOT_AVAILABLE) {

catch (e if e.result == Components.results.NS_ERROR_NOT_AVAILABLE) ?

Or at least early return in the catch to reduce indentation (if you really do want to catch all possible exceptions).

>+        var self = this;
>+        AddonManager.getAddonsByTypes(["theme"], function(aAddons) {
>+          let oldTheme = null;
>+          let newTheme = null;
>+          aAddons.forEach(function(aAddon) {
>+            if (aAddon.id == "{972ce4c6-7e08-4474-a285-3208198ce6fd}")
>+              newTheme = aAddon;

Seems like "newTheme" should be defaultTheme for clarity (if we support falling back to most-recently-used skinnable theme or something later, we can revisit).

(Is there really no easier way to get a reference to the active and default theme?)

>+          let buttonString = gNavigatorBundle.getFormattedString(strings + ".button",
>+                                                                [newTheme.name], 1)

(nit: semi-colon)

>+          let action = {

>+            callback: function () {

>+                LightweightThemeManager.currentTheme = newLWTheme;
>+                if (self._manager.currentTheme &&
>+                    self._manager.currentTheme.id == newLWTheme.id)

Doesn't the first LightweightThemeManager need to be self._manager too? I'm not sure how this works as-is.

>diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties

>+# LOCALIZATION NOTE (lwthemeNeedsThemeSwitch.message, lwthemeNeedsThemeSwitchRestart.message):
>+# %1$S will be replaced with the old theme name, %2$S will be replaced with the
>+# new theme name.
>+lwthemeNeedsThemeSwitch.message=Backgrounds cannot be worn with your current theme, %1$S.  Would you like to stop wearing %1$S and wear %2$S instead?
>+lwthemeNeedsThemeSwitchRestart.message=Backgrounds cannot be worn with your current theme, %1$S.  Would you like to stop wearing %1$S and wear %2$S instead (a restart is required)??

Two question marks is probably not necessary :) I assume these have been ui-reviewed? "Wear" is kind of odd sounding - I thought you only "wore" lwthemes?

Perhaps these should actually hardcode "the default theme" for now?

>+lwthemeNeedsThemeSwitch.button=Wear %S
>+lwthemeNeedsThemeSwitchRestart.button=Wear %S
>+lwthemeNeedsThemeSwitch.accesskey=W
>+lwthemeNeedsThemeSwitchRestart.accesskey=W

If these really are meant to be identical, refactor the code to avoid the duplicated strings.

r+ with all those addressed, but wouldn't mind looking at the updated patch as well.
Comment 66 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-24 12:37:46 PDT
Comment on attachment 468187 [details] [diff] [review]
backend patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -38,31 +38,36 @@
>...
> const PREF_DB_SCHEMA                  = "extensions.databaseSchema";
> const PREF_INSTALL_CACHE              = "extensions.installCache";
> const PREF_BOOTSTRAP_ADDONS           = "extensions.bootstrappedAddons";
> const PREF_PENDING_OPERATIONS         = "extensions.pendingOperations";
> const PREF_MATCH_OS_LOCALE            = "intl.locale.matchOS";
> const PREF_SELECTED_LOCALE            = "general.useragent.locale";
> const PREF_EM_DSS_ENABLED             = "extensions.dss.enabled";
> const PREF_DSS_SWITCHPENDING          = "extensions.dss.switchPending";
> const PREF_DSS_SKIN_TO_SELECT         = "extensions.lastSelectedSkin";
>+const PREF_DSS_ALLOW_LWTHEME          = "extensions.dss.allowLWTheme";
> const PREF_GENERAL_SKINS_SELECTEDSKIN = "general.skins.selectedSkin";
>+const PREF_LWTHEME_TO_SELECT          = "extensions.lwThemeToSelect";
>+const PREF_GENERAL_SKINS_ALLOW_LWTHEME = "general.skins.allowLWTheme";
nit: I'd just go with PREF_GENERAL_SKINS_ALLOWLWTHEME, PREF_DSS_ALLOWLWTHEME, and PREF_LWTHEMETOSELECT (this last one less concerned about) as you do elsewhere (e.g. PREF_DSS_SWITCHPENDING, PREF_GENERAL_SKINS_SELECTEDSKIN, etc.) so they align since the pref doesn't have a . between allow and LWTheme

>@@ -1226,19 +1227,46 @@ var XPIProvider = {
>     if (!Prefs.getBoolPref(PREF_DSS_SWITCHPENDING, false))
>       return;
> 
>     // Tell the Chrome Registry which Skin to select
>     try {
>       this.selectedSkin = Prefs.getCharPref(PREF_DSS_SKIN_TO_SELECT);
>       Services.prefs.setCharPref(PREF_GENERAL_SKINS_SELECTEDSKIN,
>                                  this.selectedSkin);
>+      let allowLWTheme = Prefs.getBoolPref(PREF_DSS_ALLOW_LWTHEME, false);
>+      Services.prefs.setBoolPref(PREF_GENERAL_SKINS_ALLOW_LWTHEME, allowLWTheme);
>       Services.prefs.clearUserPref(PREF_DSS_SKIN_TO_SELECT);
>+      try {
>+        Services.prefs.clearUserPref(PREF_DSS_ALLOW_LWTHEME);
>+      }
>+      catch (e) { }
This pref dance is getting confusing... please add comments about what is going on and why.

Also, the pref names don't help much with understanding and they should.

>       LOG("Changed skin to " + this.selectedSkin);
>       this.currentSkin = this.selectedSkin;
>+
>+      // If lightweight themes aren't supported by this theme then switch off
>+      // any lightweight theme
>+      if (!allowLWTheme && LightweightThemeManager.currentTheme)
>+        LightweightThemeManager.currentTheme = null;
>+
>+      if (Services.prefs.prefHasUserValue(PREF_LWTHEME_TO_SELECT)) {
>+        if (allowLWTheme) {
>+          // If lightweight thenes are supported then switch to any pending theme
s/pending theme/pending lightweight theme/

otherwise this can be taken as switch to any pending "non-lightweight" theme

>@@ -2526,19 +2515,64 @@ var XPIProvider = {
>             this.callBootstrapMethod(aAddon.id, aAddon.version, dir, "startup",
>                                      BOOTSTRAP_REASONS.ADDON_ENABLE);
>           }
>           AddonManagerPrivate.callAddonListeners("onEnabled", wrapper);
>         }
>       }
>     }
> 
>-    // Notify any other providers that a new theme has been enabled
>-    if (aAddon.type == "theme" && !isDisabled)
>-      AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type, needsRestart);
>+    if (aAddon.type == "theme") {
>+      if (!isDisabled) {
>+        // If the theme is becoming enabled then update the theme switch prefs
>+        let oldAddon = XPIDatabase.getVisibleAddonForInternalName(this.selectedSkin);
>+
>+        // Clear any pending lightweight theme change
>+        if (Services.prefs.prefHasUserValue(PREF_LWTHEME_TO_SELECT))
>+          Services.prefs.clearUserPref(PREF_LWTHEME_TO_SELECT);
>+
>+        if (needsRestart) {
>+          Services.prefs.setBoolPref(PREF_DSS_SWITCHPENDING, true);
>+          Services.prefs.setCharPref(PREF_DSS_SKIN_TO_SELECT, aAddon.internalName);
>+          Services.prefs.setBoolPref(PREF_DSS_ALLOW_LWTHEME, aAddon.skinnable);
>+        }
>+        else if (aAddon.internalName == this.currentSkin) {
>+          try {
>+            Services.prefs.clearUserPref(PREF_DSS_SWITCHPENDING);
>+          }
>+          catch (e) { }
>+          try {
>+            Services.prefs.clearUserPref(PREF_DSS_SKIN_TO_SELECT);
>+          }
>+          catch (e) { }
>+          try {
>+            Services.prefs.clearUserPref(PREF_DSS_ALLOW_LWTHEME);
>+          }
>+          catch (e) { }
>+        }
>+        else {
>+          Services.prefs.setCharPref(PREF_GENERAL_SKINS_SELECTEDSKIN, aAddon.internalName);
>+          Services.prefs.setBoolPref(PREF_GENERAL_SKINS_ALLOW_LWTHEME, aAddon.skinnable);
>+          if (!aAddon.skinnable)
>+            LightweightThemeManager.currentTheme = null;
>+          this.currentSkin = aAddon.internalName;
>+        }
>+        this.selectedSkin = aAddon.internalName;
>+
>+        // Mark the currently selected theme as disabled
>+        if (oldAddon && !oldAddon.pendingUninstall)
>+          this.updateAddonDisabledState(oldAddon, true);
Talked with Dave in person about a couple of concerns.
What if extensions.dss.enabled is true.
What about cancelling and uninstall?

>@@ -2622,19 +2656,26 @@ var XPIProvider = {
>       return;
> 
>     Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
> 
>     // TODO hide hidden add-ons (bug 557710)
>     let wrapper = createWrapper(aAddon);
>     AddonManagerPrivate.callAddonListeners("onOperationCancelled", wrapper);
> 
>-    // Notify any other providers that this theme is now enabled again.
>-    if (aAddon.type == "theme" && aAddon.active)
>-      AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type, false);
>+    // If this is the active theme and it isn't waiting to be disabled then
>+    // mark the default theme as disabled again
>+    if (aAddon.type == "theme" && aAddon.active &&
>+        !aAddon.userDisabled && !aAddon.appDisabled) {
>+      let addon = XPIDatabase.getVisibleAddonForInternalName(this.defaultSkin);
>+      if (addon)
>+        this.updateAddonDisabledState(addon, true);
>+      else
>+        WARN("Unable to activate the default theme");
nit: this warning seems incorrect. From the code it would seem that this should be an ERROR about not being able to get the default theme.
Comment 67 Robert Strong [:rstrong] (use needinfo to contact me) 2010-08-24 12:40:19 PDT
Comment on attachment 468187 [details] [diff] [review]
backend patch rev 1

Tests look good. Would like to take a look after the comments are addressed.
Comment 68 Wes Kocher (:KWierso) 2010-08-24 15:13:37 PDT
Backend patch rev 1 has a typo. s/thenes/themes
Comment 69 Blair McBride [:Unfocused] (UNAVAILABLE) 2010-08-24 21:14:28 PDT
Comment on attachment 468193 [details] [diff] [review]
UI patch rev 1

>+<!ENTITY appearance.themes.header             "Themes">
>+<!ENTITY appearance.lwthemes.header           "Backgrounds">
>+<!ENTITY theme.enable.label                   "Wear It">
>+<!ENTITY theme.details.label                  "More">
>+<!ENTITY theme.undo.label                     "Undo?">
>+<!ENTITY theme.restartNow.label               "Restart">

Tooltips?

>+#theme-list .addon[status="installed"],
>+#lwtheme-list .addon[status="installed"] {
>+  -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#theme");
>+}

Why is this only for items with status="installed" ?

>+Cu.import("resource://gre/modules/LightweightThemeManager.jsm");

Ugh. Really really don't like to see the UI interacting directly with individual providers.

>+  _defaultBackground: {
>+    id: "default@mozilla.org",
>+    type: "lwtheme",
>+    name: "Firefox Default",
>+    creator: "Mozilla",
>+    screenshots: [],
>+    isCompatible: true,
>+    pendingOperations: AddonManager.PENDING_NONE,
>+    blocklistState: Ci.nsIBlocklistService.STATE_NOT_BLOCKED,
>+
>+    get permissions() {
>+      if (this.userDisabled)
>+        return AddonManager.PERM_CAN_ENABLE;
>+      else
>+        return AddonManager.PERM_CAN_DISABLE;
>+    },
>+
>+    get isActive() {
>+      return !this.userDisabled;
>+    },
>+
>+    get userDisabled() {
>+      return !!LightweightThemeManager.currentTheme;
>+    },
>+
>+    set userDisabled(val) {
>+      LightweightThemeManager.currentTheme = null;
>+    }
>+  },

Ugh. Is there not a way to put all this stuff in the LightweightThemeManager itself? It should also provide some blank image like in the mockups - right now it just looks lke something is broken.

>+    AddonManager.getAddonsByTypes(["theme", "lwtheme"], function(aAddons) {
>+      aAddons.sort(function(a, b) {
>+        return a.name.localeCompare(b.name);
>+      });

Pitty we don't have a good way to expose sorting for this view - would be nice to be able to sort via install date too. Something we can look at in the future, I guess.

>+          if (aAddon.id == "{972ce4c6-7e08-4474-a285-3208198ce6fd}")

I'm guessing this is the ID of the default theme. Make it into a global const, and add a comment. Or add it to AddonManager, if you think it makes sense to do so. Or maybe introduce an additional property, like addon.defaultOfType

>+        }
>+        else {

Again with the newlines!

>+  onEnabled: function(aAddon) {
>+    if (aAddon.type == "lwtheme") {
>+      this._lwthemeList.firstChild._updateState();

I'd rather the default background item be kept as a poperty of gAppearanceView, instead of using firstChild here. Although, if the LightweightThemeManager were to manage the default background, this hack wouldn't be necessary (which I would much prefer).

>+  onDisabled: function(aAddon) {
>+    if (aAddon.type == "lwtheme") {
>+      this._lwthemeList.firstChild._updateState();

Ditto.

>     var screenshot = document.getElementById("detail-screenshot");
>     if (aAddon.screenshots && aAddon.screenshots.length > 0) {
>-      screenshot.src = aAddon.screenshots[0].url;
>+      screenshot.src = aAddon.screenshots[0];

Isn't this just fixing a bug in the patch in bug 562902?

>+  <binding id="theme"
>+           extends="chrome://mozapps/content/extensions/extensions.xml#addon-base">
>+    <content orient="vertical" align="stretch">
>+      <xul:hbox pack="end" class="theme-remove-container">
>+        <xul:button anonid="remove-btn" class="remove-button"
>+                    oncommand="document.getBindingParent(this).uninstall();"/>

The remove button gets vertically squished - needs some CSS love (min-height?).

>+      </xul:hbox>
>+      <xul:vbox class="theme-container" align="stretch">
>+        <xul:vbox class="screenshot-container" align="center" pack="center">
>+          <xul:stack class="theme-stack">
>+            <xhtml:div>
>+              <xhtml:img anonid="screenshot" class="theme-screenshot"/>
>+            </xhtml:div>

Does does this need to be XHTML nodes, rather than XUL? Add a comment.

The thumbnail images of some backgrounds are very small, because they're so wide compared to their height. They're much more recognisable in the mockup, where the image is zoomed to the far right corner, where most of the detail generally is (AMO and getpersonas.com does this too).

>+            <xul:vbox class="theme-overlay">
>+              <xul:hbox anonid="warning-container" align="start"
>+                        class="warning">
>+                <xul:image class="warning-icon"/>
>+                <xul:vbox flex="1" class="warning-text">
>+                  <xul:label anonid="warning" flex="1"/>
>+                  <xul:label anonid="warning-link" class="text-link"/>
>+                </xul:vbox>
>+              </xul:hbox>
>+              <xul:hbox anonid="error-container" align="start"
>+                        class="error">
>+                <xul:image class="error-icon"/>
>+                <xul:vbox flex="1" class="error-text">
>+                  <xul:label anonid="error" flex="1"/>
>+                  <xul:label anonid="error-link" class="text-link"/>
>+                </xul:vbox>
>+              </xul:hbox>
>+              <xul:hbox anonid="pending-container" align="start"
>+                        class="pending">
>+                <xul:image class="pending-icon"/>
>+                <xul:vbox flex="1" class="pending-text">
>+                  <xul:label anonid="pending" flex="1"/>
>+                  <xul:hbox pack="end">
>+                    <xul:button anonid="restart" class="button-link"
>+                                label="&theme.restartNow.label;"
>+                                oncommand="document.getBindingParent(this).restart();"/>
>+                    <xul:button anonid="undo" class="button-link"
>+                                label="&theme.undo.label;"
>+                                oncommand="document.getBindingParent(this).undo();"/>
>+                  </xul:hbox>
>+                </xul:vbox>
>+              </xul:hbox>

These notifications are really hard (sometimes possible) to read. Text in the main UI of Firefox uses a blurry text shadow to help improve legability - it should work here too (ie, dark text on white shadow; light text on black shadow). Though, there may be some security implications in overlaying chrome text over content like this  (even if its just an image).

>+              <xul:hbox class="theme-buttons" align="end" pack="end" flex="1">
>+                <xul:button anonid="enable-btn" class="theme-control"
>+                            label="&theme.enable.label;"
>+                            oncommand="document.getBindingParent(this).userDisabled = false;"/>
>+                <xul:button anonid="details-btn" class="theme-control"
>+                            label="&theme.details.label;"
>+                            oncommand="document.getBindingParent(this).showInDetailView();"/>

I know these are styled like the buttons on getpersonas.com, but they really don't look like they fit in with the rest of the UI here.

The "More" button is kinda confusing. I think it works in list view because its appended to the end of the description - but we don't have that here. getpersonas.com uses "Details", which works pretty well.

Also, the buttons should be horizontally aligned with the buttons of other items. At the moment, their position is dictated by the height of the screenshot (since that is centered vertically) - which makes the positioning seems a little random and unpredictable (or at least, unexpected).

>+            } else if (this.mAddon.blocklistState == Ci.nsIBlocklistService.STATE_SOFTBLOCKED) {

Add a note in bug 590347 for the appearance pane too.

>+        <!-- appearance view -->
>+        <vbox id="appearance-view" flex="1" class="view-pane">
>+          <scrollbox id="theme-lists" orient="vertical" flex="1">
>+            <expander id="theme-expander" class="theme-expander"
>+                      label="&appearance.themes.header;"
>+                      open="true" clearhidden="true" persist="open">
>+              <hbox id="theme-list" class="theme-list" flex="1" align="start"/>

I'm not sure naming these as lists makes sense - "theme-gallery" fits more, I think. Also, having a class the same as an ID is confusing. Maybe id="theme-gallery" class="gallery" ?

The whole expander should be clickable to expand/collapse a group, not just the tiny icon. No idea why it doesn't default to that behaviour - having to click that tiny icon is really annoying. Also, these are lacking a horizontal line like in the mockup (which really helps separate the two groups).

>+              <hbox id="lwtheme-unsupported" align="center">
>+                <image id="lwtheme-unsupported-image"/>
>+                <label id="lwtheme-unsupported-label" flex="1"/>
>+                <button id="lwtheme-unsupported-button" class="button-link"
>+                        oncommand="gAppearanceView.enableDefaultTheme();"/>

I tested this on a horribly broken theme, so it was hard to tell for sure, but:
Clicking the "Stop wearing XXX" button results in the "Restart to wear Default" notification for the Default theme. But that notification isn't obvious, because it doesn't show up near where you just clicked. In fact, it could be scrolled off-screen. I wonder if that button should actually say "Restart and stop wearing XXX" and automatically restart once you click it.

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_bug562890.js 

This seems, er, unrelated?

> .warning, .pending, .error, .info {
>-  -moz-margin-start: 48px;
>+  -moz-margin-start: 47px;
> }

Ditto.

>+.theme-list .addon {
>+  padding: 2px;
>+  margin: 6px 7px;
>+  border: 1px solid transparent;
>+  -moz-border-radius: 5px;
>+}

The mockups look like there should be a hover effect too - light grey gradient and border.

>+.addon[pending] .theme-overlay,
>+.addon[notification] .theme-overlay {
>+  background-color: rgba(0, 0, 0, 0.75);
>+}

Think this is too dark. Probably followup fodder though - it could do with some experimentation (along with the colors of the notification text).

>+.theme-control[anonid="enable-btn"] {
>+  background-color: #DD7400;
>+}
>+
>+.theme-control[anonid="details-btn"] {
>+  background-color: #71AFCD;
>+}

Should be using classes for this, not anonid.

>diff --git a/toolkit/themes/pinstripe/global/button.css b/toolkit/themes/pinstripe/global/button.css
...
>+/* ::::: disclosure buttons ::::: */
>+
>+button[type="disclosure"] {
>+  -moz-appearance: treetwisty;
>+  margin: 0 !important;
>+  padding: 0 !important;
>+  min-width: 0 !important
>+}
>+
>+button[type="disclosure"][open="true"] {
>+  -moz-appearance: treetwistyopen;
>+}
>+

I'm guessing this is the thing that was accidentally never added to pinstripe... fixing that in this bug worries me.
Comment 70 Dão Gottwald [:dao] 2010-08-24 23:51:46 PDT
(In reply to comment #69)
> Ugh. Is there not a way to put all this stuff in the LightweightThemeManager
> itself? It should also provide some blank image like in the mockups - right now
> it just looks lke something is broken.

Making it look like in the mockup is a matter of styling, it doesn't mean the LightweightThemeManager should provide a blank image.
Comment 71 patrickjdempsey 2010-08-25 17:11:30 PDT
PersonasPlus 1.6 with non-default theme "support" just went Public.  However, they have stripped all of the support styles out which actually allow the Personas to be visible through the toolbox, tabs and statusbar in non-default themes.  This requires that all themes *actively* support Personas in an unprecedented way as the last version of PersonasPlus to support themes (1.4) did apply some styles to themes to allow the Persona to show through.  Will this be the case for native Personas as well?  Will *all* theme authors be required to provide their own individual Personas support, or will Firefox provide at least some basic overriding styles?
Comment 72 Dave Townsend [:mossop] 2010-08-25 17:19:40 PDT
Themes will have to do whatever is necessary to support personas themselves.
Comment 73 Dave Townsend [:mossop] 2010-08-26 16:34:36 PDT
(In reply to comment #66)
> Comment on attachment 468187 [details] [diff] [review]
> backend patch rev 1
> >+          Services.prefs.setCharPref(PREF_GENERAL_SKINS_SELECTEDSKIN, aAddon.internalName);
> >+          Services.prefs.setBoolPref(PREF_GENERAL_SKINS_ALLOW_LWTHEME, aAddon.skinnable);
> >+          if (!aAddon.skinnable)
> >+            LightweightThemeManager.currentTheme = null;
> >+          this.currentSkin = aAddon.internalName;
> >+        }
> >+        this.selectedSkin = aAddon.internalName;
> >+
> >+        // Mark the currently selected theme as disabled
> >+        if (oldAddon && !oldAddon.pendingUninstall)
> >+          this.updateAddonDisabledState(oldAddon, true);
> Talked with Dave in person about a couple of concerns.
> What if extensions.dss.enabled is true.
> What about cancelling and uninstall?

I think that this is correct for the non-dss case, though I've added some further clarification. If the current theme is waiting to be uninstalled then there is no need to mark it as disabled. If the uninstall is cancelled then the selected theme will be marked as disabled again. For the dss case this was blocking onDisabled being sent for the pending uninstall add-on which is wrong, instead I've put a check that we never send onDisabling/onEnabling for add-ons pending uninstall.
Comment 74 Dave Townsend [:mossop] 2010-08-26 16:35:39 PDT
Created attachment 469686 [details] [diff] [review]
browser patch rev 2

You wanted to spin over this again Gavin?
Comment 75 Dave Townsend [:mossop] 2010-08-26 16:46:23 PDT
Created attachment 469693 [details] [diff] [review]
backend patch rev 2

Updated from comments. I've tried to give the pref constants some more sensible names and comments surrounding them have been improved.
Comment 76 Dave Townsend [:mossop] 2010-08-27 12:50:16 PDT
This is going to have to slip to b6 I think.
Comment 77 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-08-30 13:22:17 PDT
Comment on attachment 469686 [details] [diff] [review]
browser patch rev 2

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>+  _install: function (newLWTheme) {

>+        let buttonString = gNavigatorBundle.getString("lwthemeNeedsThemeSwitch.button");
>+
>+        let action = {
>+          label: buttonString,
>+          accessKey: gNavigatorBundle.getString(strings + ".accesskey"),

This will throw in the restart case - needs to be hardcoded to "lwthemeNeedsThemeSwitch.accesskey". I would also get rid of buttonString variable given that it's only referenced once.

>+          callback: function () {
>+            defaultTheme.userDisabled = false;
>+            if (needsRestart) {
>+              Services.prefs.setCharPref("extensions.lwThemeToSelect",
>+                                         defaultTheme.id);

Shouldn't this be newLWTheme.id?

>diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties

>+# LOCALIZATION NOTE (lwthemeNeedsThemeSwitch.button, lwthemeNeedsThemeSwitchRestart.button):
>+# %S will be replaced with the new theme name.
>+lwthemeNeedsThemeSwitch.button=Wear The Default Theme
>+lwthemeNeedsThemeSwitch.accesskey=W

This note no longer applies (and lwthemeNeedsThemeSwitchRestart.button no longer exists).
Comment 78 Dave Townsend [:mossop] 2010-08-30 15:44:26 PDT
Created attachment 470602 [details] [diff] [review]
browser patch rev 3

Simplified the strings stuff and verified it works correctly manually.
Comment 79 Dave Townsend [:mossop] 2010-08-30 16:13:41 PDT
(In reply to comment #69)
> Comment on attachment 468193 [details] [diff] [review]
> UI patch rev 1
> 
> >+<!ENTITY appearance.themes.header             "Themes">
> >+<!ENTITY appearance.lwthemes.header           "Backgrounds">
> >+<!ENTITY theme.enable.label                   "Wear It">
> >+<!ENTITY theme.details.label                  "More">
> >+<!ENTITY theme.undo.label                     "Undo?">
> >+<!ENTITY theme.restartNow.label               "Restart">
> 
> Tooltips?

Done

> >+#theme-list .addon[status="installed"],
> >+#lwtheme-list .addon[status="installed"] {
> >+  -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#theme");
> >+}
> 
> Why is this only for items with status="installed" ?

Removed that restriction, however there is currently nothing here supporting displaying the progress of install/updates in this view. I want to do that in a follow-up patch though

> >     var screenshot = document.getElementById("detail-screenshot");
> >     if (aAddon.screenshots && aAddon.screenshots.length > 0) {
> >-      screenshot.src = aAddon.screenshots[0].url;
> >+      screenshot.src = aAddon.screenshots[0];
> 
> Isn't this just fixing a bug in the patch in bug 562902?

Let's pretend you didn't see that.

> >+      </xul:hbox>
> >+      <xul:vbox class="theme-container" align="stretch">
> >+        <xul:vbox class="screenshot-container" align="center" pack="center">
> >+          <xul:stack class="theme-stack">
> >+            <xhtml:div>
> >+              <xhtml:img anonid="screenshot" class="theme-screenshot"/>
> >+            </xhtml:div>
> 
> Does does this need to be XHTML nodes, rather than XUL? Add a comment.

Surprisingly (and sadly) yes, commented.

> >diff --git a/toolkit/mozapps/extensions/test/browser/browser_bug562890.js 
> 
> This seems, er, unrelated?

Actually it isn't. This test assumes the add-ons manager opens up to show the extensions list, but since we remember the last pane that isn't guaranteed to be the case all the time.

> > .warning, .pending, .error, .info {
> >-  -moz-margin-start: 48px;
> >+  -moz-margin-start: 47px;
> > }
> 
> Ditto.

Grr, mixing patches up.

> >diff --git a/toolkit/themes/pinstripe/global/button.css b/toolkit/themes/pinstripe/global/button.css
> ...
> >+/* ::::: disclosure buttons ::::: */
> >+
> >+button[type="disclosure"] {
> >+  -moz-appearance: treetwisty;
> >+  margin: 0 !important;
> >+  padding: 0 !important;
> >+  min-width: 0 !important
> >+}
> >+
> >+button[type="disclosure"][open="true"] {
> >+  -moz-appearance: treetwistyopen;
> >+}
> >+
> 
> I'm guessing this is the thing that was accidentally never added to
> pinstripe... fixing that in this bug worries me.

Meant to do that separately, but maybe we should fix bug 385927 too.
Comment 80 Dave Townsend [:mossop] 2010-08-30 16:14:31 PDT
Created attachment 470618 [details] [diff] [review]
UI patch rev 2
Comment 81 Dave Townsend [:mossop] 2010-08-30 16:29:11 PDT
We are now 11 days from feature/string freeze. The main patch in this bug is not yet ready for its next review and is still missing fairly important behaviors (like handling install/update progress messaging). Even once those are done and it is landed there is no doubt that it would cause a number of follow-up issues to need to be fixed before release. We already have too many blockers for release and we're probably going to have to start cutting some of them. Landing a large feature like this that we know will create new blockers so late in the game does not make sense. Even though it will mean that we have to spend time on fixing other issues that this would have fixed by itself we're going to have to let this slip and come back to it after Firefox 4.

I'm sure many people won't like this decision, I don't really like it either but I have to be realistic. I don't believe that we're regressing Firefox 4 by not including this and it means that we will have the time to really polish this feature for the next release.
Comment 82 Alfred Kayser 2010-08-30 23:48:57 PDT
Dave, I understand the reasoning, and for the FF4 release itself it is a wise decision. I like to thank your efforts into getting this implemented, and still hope that this can be done anymore in a near timeframe, say FF4.0.1?
Comment 83 Peter Henkel [:Terepin] 2010-08-31 04:40:26 PDT
FX 4.0.1 will be bugfix release. And because many features will be cut of from 4.0, they will be moved to 4.x.
Comment 84 Dave Townsend [:mossop] 2010-08-31 08:05:03 PDT
(In reply to comment #82)
> Dave, I understand the reasoning, and for the FF4 release itself it is a wise
> decision. I like to thank your efforts into getting this implemented, and still
> hope that this can be done anymore in a near timeframe, say FF4.0.1?

I'd certainly like to see this make a point release and I'll be talking it over with the drivers once this is fixed on trunk and any issues with it ironed out however I'll be honest, my expectation is that there will be concerns over extension breakage and the amount of localisation that will be required. That is a discussion for the future though.
Comment 85 Dave Townsend [:mossop] 2010-08-31 11:05:41 PDT
The browser patch and probably parts of the backend patch will be migrated over to bug 592338 so we have proper feedback for that case.
Comment 86 Alfred Kayser 2010-08-31 11:18:51 PDT
Note, Personas 1.6 (not officially approved by AMO, but available right there) does support the combo...
Comment 87 mcdavis941 (sporadically reading bugmail) 2010-08-31 13:08:12 PDT
(In reply to comment #81)
> I am giving up bits and pieces of my spare time to try
> to make it happen

Thanks for having tried to fit it into this release. Given the outstanding blockers it sounds like you're making the right choice.
Comment 88 Jeremy Morton 2011-03-06 16:25:40 PST
I've just read this bug.  Can I just get this straight (sorry if I've missed the obvious here) - Firefox 4 final will not (initially) support personas applied on top of custom themes.  This means that if a user installs a custom theme and then installs a persona, Firefox will prompt them to restart, switch them back to the default Strata, and apply the persona?

OK, but wouldn't it be a good idea to give the user some notification that they'll be losing their custom theme if they apply the persona?  Otherwise they're going to be wondering where all their other custom styling went.
Comment 89 Dave Townsend [:mossop] 2011-03-07 10:30:48 PST
(In reply to comment #88)
> I've just read this bug.  Can I just get this straight (sorry if I've missed
> the obvious here) - Firefox 4 final will not (initially) support personas
> applied on top of custom themes.  This means that if a user installs a custom
> theme and then installs a persona, Firefox will prompt them to restart, switch
> them back to the default Strata, and apply the persona?

Correct

> OK, but wouldn't it be a good idea to give the user some notification that
> they'll be losing their custom theme if they apply the persona?  Otherwise
> they're going to be wondering where all their other custom styling went.

Perhaps. The model we are going for at the moment is that personas are just a kind of custom styling like other themes. You don't get warned you'll lose your custom styling from Charamel when switching to Nasa Night Launch for example.

Either way this bug is fairly high on our list of priorities now that Firefox 4 is done. Fair warning though, as Firefox 5 will have a short dev cycle and there are a couple of other things with even higher priorities it still may be Firefox 6 before this sees a release.
Comment 90 Jeremy Morton 2011-03-07 10:51:38 PST
(In reply to comment #89)
> Perhaps. The model we are going for at the moment is that personas are just a
> kind of custom styling like other themes. You don't get warned you'll lose your
> custom styling from Charamel when switching to Nasa Night Launch for example.

No, but then you don't get to mouseover Charamel and see it applied simultaneously with your current theme.  With personas, you do.  So, if my persona is a flowery background, I'm hovering the mouse over flowery background img, my toolbar background becomes a flowery background, but my custom theme's toolbar icons are still on top of it.  This strongly implies that the persona will be able to be combined with your current theme.  You then wear the persona and you lose your custom theme buttons (etc.)  This seems unintuative and misleading to me.  Is there actually a good reason we can't apply personas with custom themes at the moment?  If I can hover over a persona image and see it blend perfectly well with my custom theme in 'preview' mode, surely it's just a case of maintaining that 'preview' mode and the persona is applied.
Comment 91 Dave Townsend [:mossop] 2011-03-07 11:08:42 PST
(In reply to comment #90)
> (In reply to comment #89)
> > Perhaps. The model we are going for at the moment is that personas are just a
> > kind of custom styling like other themes. You don't get warned you'll lose your
> > custom styling from Charamel when switching to Nasa Night Launch for example.
> 
> No, but then you don't get to mouseover Charamel and see it applied
> simultaneously with your current theme.  With personas, you do.  So, if my
> persona is a flowery background, I'm hovering the mouse over flowery background
> img, my toolbar background becomes a flowery background, but my custom theme's
> toolbar icons are still on top of it.  This strongly implies that the persona
> will be able to be combined with your current theme.

Yes, this is a bug.

> Is there actually a good reason we can't apply personas with
> custom themes at the moment?  If I can hover over a persona image and see it
> blend perfectly well with my custom theme in 'preview' mode, surely it's just a
> case of maintaining that 'preview' mode and the persona is applied.

Yes, but that doesn't work with all themes.
Comment 92 Pardal Freudenthal (:ShareBird) 2011-03-07 12:30:51 PST
(In reply to comment #91)
> 
> > Is there actually a good reason we can't apply personas with
> > custom themes at the moment?  If I can hover over a persona image and see it
> > blend perfectly well with my custom theme in 'preview' mode, surely it's just a
> > case of maintaining that 'preview' mode and the persona is applied.
> 
> Yes, but that doesn't work with all themes.
Since Bug 574970 was fixed, we have already a way to determine if a theme can work or not with Personas: the skinnable property on install.rdf. So we are able to apply a persona only for themes that are in fact working with Personas, avoiding a possible mess-up.
Comment 93 Alfred Kayser 2011-03-08 03:35:21 PST
So, the excuse not to support themes with personas is not valid since a long time, but is still used to allow custom themes with personas?
Why not, when applying a persona, check whether the current theme is 'skinnable' and only switch to the default theme is the current theme is NOT skinnable.
Comment 94 Dave Townsend [:mossop] 2011-03-08 09:45:29 PST
(In reply to comment #93)
> So, the excuse not to support themes with personas is not valid since a long
> time, but is still used to allow custom themes with personas?
> Why not, when applying a persona, check whether the current theme is
> 'skinnable' and only switch to the default theme is the current theme is NOT
> skinnable.

Because without changes to the UI that situation would be very confusing.
Comment 95 Jeremy Morton 2011-03-08 10:01:11 PST
(In reply to comment #94)
> (In reply to comment #93)
> > So, the excuse not to support themes with personas is not valid since a long
> > time, but is still used to allow custom themes with personas?
> > Why not, when applying a persona, check whether the current theme is
> > 'skinnable' and only switch to the default theme is the current theme is NOT
> > skinnable.
> 
> Because without changes to the UI that situation would be very confusing.

Not really.  You just switch to the persona immediately if the current theme is skinnable, and if not, you have the popup requesting a restart warn you that because the current theme isn't skinnable, Firefox will switch back to the default theme when this persona is applied (that warning should be there in any case...)
Comment 96 Dave Townsend [:mossop] 2011-03-08 10:23:46 PST
(In reply to comment #95)
> (In reply to comment #94)
> > (In reply to comment #93)
> > > So, the excuse not to support themes with personas is not valid since a long
> > > time, but is still used to allow custom themes with personas?
> > > Why not, when applying a persona, check whether the current theme is
> > > 'skinnable' and only switch to the default theme is the current theme is NOT
> > > skinnable.
> > 
> > Because without changes to the UI that situation would be very confusing.
> 
> Not really.  You just switch to the persona immediately if the current theme is
> skinnable, and if not, you have the popup requesting a restart warn you that
> because the current theme isn't skinnable, Firefox will switch back to the
> default theme when this persona is applied (that warning should be there in any
> case...)

You two are talking about two different behaviours I think.
Comment 97 Robert Ciang 2011-03-09 22:32:23 PST
(In reply to comment #91)
> (In reply to comment #90)
> > (In reply to comment #89)
> > > Perhaps. The model we are going for at the moment is that personas are just a
> > > kind of custom styling like other themes. You don't get warned you'll lose your
> > > custom styling from Charamel when switching to Nasa Night Launch for example.
> > 
> > No, but then you don't get to mouseover Charamel and see it applied
> > simultaneously with your current theme.  With personas, you do.  So, if my
> > persona is a flowery background, I'm hovering the mouse over flowery background
> > img, my toolbar background becomes a flowery background, but my custom theme's
> > toolbar icons are still on top of it.  This strongly implies that the persona
> > will be able to be combined with your current theme.
> 
> Yes, this is a bug.
> 

Hi All, while discussing over a good way to "determine if the theme supports coexistence with Personas", will there be anyone to fix the current bug, that we still can't actually wear a Persona on Charamel theme (or any other supporting theme) like we could do in Fx 3.6 with Personas 1.6.1? If the correct mechanism has been incorporated into Fx 4 codebase already, will it be hard to make the coexisting mechanism back to normal, without the need of installing any addons?
Comment 98 Robert Ciang 2011-03-09 22:37:35 PST
Or at least although you decide not to fix this in Fx 4.0 Gold itself, will it be available soon as a patch in the new release of Personas Plus?
Comment 99 Robert Ciang 2011-03-09 22:51:10 PST
Although not every theme supports coexistence with Personas (people know, people know...), but please, before you make the final perfect warning-and-determining behavior and Appearance interface running, can it be allowed that people get a little workaround or tweaking method for their little happiness? (the lightweightThemes.forceSkinning setting was just such little but working method)

I just hope that although the construction of the huge hotel is not finished yet, I can still have a small door opened so that I can borrow the toilet or have a peep for the time being. It would be annoying if all doors are locked until the whole bunch thing finishes.
Comment 100 Robert Ciang 2011-03-14 00:58:11 PDT
Today I found that with updated Personas Plus 1.6.2, the lightweightThemes.isThemeSelected preference is affective again (although we have to restart Fx 3 times to enable Personas to wear on a custom theme). Thanks to anyone contributing and working for the bug fix! And sorry for my mumblers earlier...

For the exact method so far to wear a Persona on a custom theme, see https://forums.mozilla.org/addons/viewtopic.php?f=30&t=2636
Comment 101 Robert Ciang 2011-03-18 20:35:28 PDT
It would still be appreciated that the current bug (this bug!) that a Persona can't be worn on any custom theme directly and immediately can be fixed. Now users have to restart Fx three times to get the job done (see the method provided in #100). And such a limitation also prevents users of custom theme from using the convenient tools such as Personas Rotator.
Comment 102 Dão Gottwald [:dao] 2011-04-16 02:46:13 PDT
*** Bug 647507 has been marked as a duplicate of this bug. ***
Comment 103 Pardal Freudenthal (:ShareBird) 2011-05-16 07:19:53 PDT
Any news on this bug? There is already a considerable amount of work on it, it could be fine to fix it for Firefox 5.0.
Comment 104 Peter Henkel [:Terepin] 2011-05-16 07:25:57 PDT
No, FX5 is feature complete several weeks and considering recent activity and FX6's movement to Aurora, this will most likely land to FX7.
Comment 105 Robert Ciang 2011-05-17 17:01:44 PDT
Can we just implement the ability for Firefox to wear a Persona on top of any custom theme immediately first in Fx 4.0.x? I want to use Charamel, and I also hope to use Personas Rotator, and without any fix, I have to restart Fx 3 times to change a Persona so to make it coexist with my custom theme!

Can we just leave those complicated interface things to Fx 5 or 6 but fix this radical but simple (okay maybe not so simple) flaw ASAP? I just can't get why do we have to bundle so much work and so many considerations with the simple request: "I want to wear any Persona on top of the custom theme conveniently like in Fx 3.5 + Personas Plus 1.4!" Forget about the compatibility markup or whatever "reminder" since the author of Charamel has already make his theme compatible with Personas without any side effects and we know that so just let us use it already...
Comment 106 Dave Townsend [:mossop] 2011-05-17 18:41:52 PDT
(In reply to comment #104)
> No, FX5 is feature complete several weeks and considering recent activity
> and FX6's movement to Aurora, this will most likely land to FX7.

"likely" is not the term I'd use here. To make Firefox 7 it'd have to be completed and landed on trunk, ready to ship, in under 7 weeks, and right now there is no-one assigned to work on it. It is a high priority after the existing in-progress add-ons manager features are complete.

(In reply to comment #105)
> Can we just implement the ability for Firefox to wear a Persona on top of
> any custom theme immediately first in Fx 4.0.x?

Nothing is going to change on the 4.0.x branch, nor can anything make Firefox 5 or 6 at this stage. Nor would I make a change without fixing the UI which as it stands would be confusing to users who don't recognise the difference between full themes and personas.
Comment 107 Jeremy Morton 2011-05-18 01:07:49 PDT
Dave,

Will you acknowledge that it was a silly idea to implement personas as a built-in feature of FF4 before you had a proper GUI to expose the feature, especially given that it was guaranteed to cause user confusion between themes and personas and there was already a rather easy extension to use to wear a persona anyway?
Comment 108 Dão Gottwald [:dao] 2011-05-18 03:00:10 PDT
Please refrain from adding comments that will only waste people's time rather than moving bugs forward.
Comment 109 Robert Ciang 2011-06-30 20:09:06 PDT
It was not acknowledged that Mozilla was going to shorten the release cycle, so it seemed that "putting this bug to be fixed till Fx6 or Fx7" would take quite unbearably long time.

Now that Fx5 has been released and Fx6 and 7 are underway, let's hope to expect a total solution of the complex of themes and Personas in a relatively near future. And cheers... Mozilla personnel and voluteers...
Comment 110 Tony Mechelynck [:tonymec] 2011-09-04 03:10:32 PDT
I'm summarizing here the workaround (with three restarts! so it is a workaround, not a fix) described in the add-ons forums at https://forums.mozilla.org/addons/viewtopic.php?f=30&t=2636
The order of the steps is critical:
1. Enable the desired Persona and restart.
(The Persona is displayed on top of the default theme.)
2. Enable the desired theme and restart.
(The theme is displayed without a Persona.)
3. In about:config, set lightweightThemes.isThemeSelected to true and restart.
(The theme and persona appear together.)
Comment 111 Tony Mechelynck [:tonymec] 2011-10-10 01:41:20 PDT
If this bug is ever to get fixed, the heart of the solution will be found in $MOZILLA_CENTRAL/toolkit/mozapps/extensions/LightweightThemesManager.jsm; another relevant source file is $MOZILLA_CENTRAL/toolkit/mozapps/extensions/XPIProvider.jsm which will hopefully require no change. Let me add immediately that I am still far from having studied these files well enough to propose anything resembling a patch; if you have, feel free to try.

I see the following points to be addressed, or at least kept in mind:
- Any theme other than the default makes Personas non-bootstrappable. Experiment shows that setting extensions.dss.enabled to true changes nothing. The relevant function is at LightweightThemesManager.jsm lines 463 to 477. It is not clear to me which changes, if any, may be required in the various /.*RequiresRestart(/ functions in XPIProvider.jsm lines 3293 and later.
- Selecting a different theme removes the current Persona immediately without waiting for the next restart.
- Setting lightweightThemes.isThemeSelected to true in user.js doesn't make the third restart in comment #110 unnecessary. The pref is set to true after the second restart, but apparently too late for the Persona to be worn without an additional restart.
Comment 112 Dão Gottwald [:dao] 2011-11-16 08:58:57 PST
*** Bug 702927 has been marked as a duplicate of this bug. ***
Comment 113 James 2011-12-20 12:59:31 PST
this is a very annoying bug I get very tired of having to restart Firefox 3 or 4 times just to switch a persona that im using with the NASA night launch theme.
Comment 114 rob64rock 2012-03-02 15:10:04 PST
(In reply to James from comment #113)
> this is a very annoying bug I get very tired of having to restart Firefox 3
> or 4 times just to switch a persona that im using with the NASA night launch
> theme.

For those interested in not having to restart the browser multiple times or not having to modify about:config preference settings.

Use this extension:
Allow instant previews of Personas on custom themes without browser restart and also allows instant switching between your installed Personas without browser restart when using a custom theme.

https://addons.mozilla.org/firefox/addon/lightweight-theme-switcher

The Add-on developer only states it compatible up to Fx 11*, but I have tested it on latest Fx Nightly 13.0a1 and it still works without any problems.
Comment 115 rob64rock 2012-03-02 15:50:36 PST
(In reply to rob64rock from comment #114)
> (In reply to James from comment #113)
> > this is a very annoying bug I get very tired of having to restart Firefox 3
> > or 4 times just to switch a persona that im using with the NASA night launch
> > theme.
> 
> For those interested in not having to restart the browser multiple times or
> not having to modify about:config preference settings.
> 
> Use this extension:
> Allow instant previews of Personas on custom themes without browser restart
> and also allows instant switching between your installed Personas without
> browser restart when using a custom theme.
> 
> https://addons.mozilla.org/firefox/addon/lightweight-theme-switcher
> 
> The Add-on developer only states it compatible up to Fx 11*, but I have
> tested it on latest Fx Nightly 13.0a1 and it still works without any
> problems.

How to Post for: https://addons.mozilla.org/firefox/addon/lightweight-theme-switcher

 * Note: If you have Complete(current) theme enabled without a persona applied, then go to the Tools Menu instead of the Add-on Manager and select the menu item Persona Switcher and select a Persona to Apply/Enable without restart. If you try to Apply/Enable an Persona theme using the Add-on Manager it will require a browser restart, which causes the Complete Theme to disabled.

 * Persona Switcher 1.6 version reinstated the applying/enabling of an Persona without restart.
Comment 116 patrickjdempsey 2012-03-03 01:46:15 PST
Seriously, this has absolutely nothing to do with this bug or moving it forward, please do not spam this bug again.
Comment 117 Robert Ciang 2012-03-03 01:52:16 PST
Thanks rob64rock anyway for the information, because at least that extension works this bug around.
Comment 118 Danial Horton 2012-03-03 01:56:27 PST
workarounds are valid and constructive discussion patrick.
Comment 119 patrickjdempsey 2012-03-03 02:24:43 PST
Please keep it in a forum somewhere, most of the people subscribed to this bug are waiting for *real* news about *real* movement on this bug which they have been waiting for for over 2 1/2 years.  If you want to post hacks or extensions there are plenty of outlets online to promote those without ticking off the people who have been waiting years for an official response to this issue. See comment 108.
Comment 120 Tony Mechelynck [:tonymec] 2012-03-10 05:16:28 PST
I believe describing workarounds is on-topic for bug comments, even though it doesn't help fixing the actual bug.
* The workaround in comment #110 applies to all Persona-capable Toolkit applications but it requires no fewer than three restarts in the general case.
* The "Lightweight Theme (Persona) Switcher" extension linked in comment #114 is easier to use, but it applies only to Firefox (I didn't try editing its install.rdf); yet Thunderbird and SeaMonkey (at least) can also use Persona (lightweight) theming.
Comment 121 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-09 19:31:39 PDT
(Assigning to myself so no-one accidentally takes this - I've been slowly working on this when I have free time.)
Comment 122 Tony Mechelynck [:tonymec] 2014-01-23 16:58:33 PST
Let's adjust the Version field, and make the Subject more searchable
Comment 123 Tony Mechelynck [:tonymec] 2016-01-27 21:10:33 PST
(In reply to Tony Mechelynck [:tonymec] from comment #110)
> I'm summarizing here the workaround (with three restarts! so it is a
> workaround, not a fix) described in the add-ons forums at
> https://forums.mozilla.org/addons/viewtopic.php?f=30&t=2636
> The order of the steps is critical:
> 1. Enable the desired Persona and restart.
> (The Persona is displayed on top of the default theme.)
> 2. Enable the desired theme and restart.
> (The theme is displayed without a Persona.)
> 3. In about:config, set lightweightThemes.isThemeSelected to true and
> restart.
> (The theme and persona appear together.)

I don't know exactly when it changed, but lightweightThemes.isThemeSelected is now irrelevant, at least for Trunk (and for I don't know how many Branches).

For these versions, Step 3 above should be replaced by:

3a. In about:config, double-click the preference lightweightThemes.selectedThemeID and set it tho the numeric value of the very first "id" in the preference lightweightThemes.usedThemes (which is the ID of your chosen lightweight theme).
3b. Restart.
(Your chosen complete theme and lightweight theme appear together.)

Note You need to log in before you can comment on or make changes to this bug.