What about an 'about:profiles' page where we can show the existing profiles, having an easy way to rename them, create new ones, delete them and so on... I wrote a patch for that. If the reviewer agrees, maybe we can land it. Bonus goal: have a full integration of about:profiles in the ff UI.

Does the patch ensure that currently used profiles can't be removed? But this is great.

(In reply to Olli Pettay [:smaug] from comment #1) > Does the patch ensure that currently used profiles can't be removed? Yes. You cannot remove the active profile. But you can rename it, and that's still ok. I guess the next step, if this patch is ok, is to allow multiple profiles running at the same time. This would be great!

yes, I was asking about multiple running profiles. That is what I do all the time. 2-3 profiles running and perhaps 10 others for random testing purposes, but those are used only rarely, and tend to forget what all I even have.

We already have one profile manager that we've on and off been trying to get rid of, does it make sense to add a second?

Indeed, although we might want to do something with better profile management in the future as part of great-or-dead, this is not code that we want to maintain.

I've never understood why we've tried to get rid of useful features, like profile manager, without any replacement. about:profiles could be the replacement, at least for most of the features.

I guess we should talk before marking a bug wontfix. Especially when there is a patch and especially after a work-week where we discuss about implementing this feature. Plus, getting rid of that profile manager doesn't meant that we cannot have something better.

bsmedberg, could you explain why you're against about:profiles?

Actually, having about:profiles would be a nice way to get rid of that profile manager. What about implementing a simple: <xul:browser src="about:profiles" /> instead of that horrible UI we show with '-P'.

This is just a prototype about replacing the existing ProfileManager with about:profiles. In this way we have the same UI and the same code for these 2 features.

Comment on attachment 8628129 [details] [diff] [review] patch 1 - about:profiles Review of attachment 8628129 [details] [diff] [review]: ----------------------------------------------------------------- I'm clearing the flag for now while we're waiting for Benjamin to respond, so that this doesn't keep showing up in my queue...

Especially about:profiles page is rather nice. I see it as a similar level feature as about:plugins or perhaps about:memory. Something for a tiny bit more educated users. The version profile manager uses could perhaps hide Root/Local directory, and perhaps Default Profile could be a checkbox or radiobutton.

Also, it would be nice to have the default profile as the first one.

I agree with comment 12. I can improve the UI once we have a feedback from Benjamin.

Comment on attachment 8628749 [details] [diff] [review] patch 2 - Profile Manager So about:profile should perhaps have a mode for profile manager which hides some of the information by default.

Do you think we can proceed somehow with these patches?

I'm not sure. I'll email Benjamin.

I have several inter-related concerns. * Profile management in any of its current forms is not good enough to expose to ordinary users. There are too many edge cases around startup, remoting, etc. I'm worried that exposing about:profiles will cause advanced users to start using it, which an anti-goal. * Without also getting rid of the existing profile manager, this is duplicated UI and I'd reject it simply on that account. * I primarily want to get rid of the existing profile manager so that I don't have to keep supporting XPCOM/gecko without a profile, and the startup/restart environment machinery. * We need to be selective about all new features and the total amount of supported code. We're dying under a glut of unsupported features. * I'm clearly the owner for the profile code, and I don't think that supporting this is a good tradeoff. * I don't know whether it's possible to implement this as an addon, but I'd suspect so and I would strongly recommend that instead of building this in-tree.

FWIW, the patches here seem to actually help with at least the 3rd points in comment 18 (even though they may not solve them completely), and it is also removing the existing profile manager code for the functionality it's implementing, as far as I can tell.

Hi Benjamin, thanks for these points. > * Profile management in any of its current forms is not good enough to > expose to ordinary users. There are too many edge cases around startup, > remoting, etc. I'm worried that exposing about:profiles will cause advanced > users to start using it, which an anti-goal. About this, my second patch removes the current form and replace it with about:profiles. In this way we have just 1 single UI. Plus: we are not going to expose this about:profiles to ordinary uses. (I have to admin I would like to :) but this is a second step. > * Without also getting rid of the existing profile manager, this is > duplicated UI and I'd reject it simply on that account. Right. Second patch is for that reason. Plus: we cannot get rid of the profile manager because for developers this is extremely useful. And aurora (developer edition) uses separate profiles. There are also several addons that use this profile manager. We should get rid of the UI and replace it with something better. I would like to point out that chrome has a similar feature (multiple-profiles) and this is very very useful. We have everything in place, except exposing it. > * I'm clearly the owner for the profile code, and I don't think that > supporting this is a good tradeoff. I'm happy to help to manage the profile code.

As far as I see, the patches simplify the code and the setup we have here. - in other words takes useful functionality out from profile manager and reuse it also for about:profile. It is really hard to see the reluctantcy to improve the setup we have. "get rid of the existing profile manager so that I don't have to keep supporting XPCOM/gecko without a profile" is probably the most annoying problem related to profile manager, but it is a separate issue from what is being done in this bug, and I think we should just have some separate profile for that case. But anyhow, different bug.

(In reply to Olli Pettay [:smaug] from comment #21) > "get rid of the existing profile manager so that I don't have to keep > supporting XPCOM/gecko without a profile" is probably the most annoying > problem related to profile manager, but it is a separate issue from what is > being done in this bug, and I think we should just have some separate > profile for that case. But anyhow, different bug. I don't understand this part. It seems to me that when Andrea finishes building about:profiles, that problem would be completely solved. Wouldn't it?

Oh, sure, if we just always start browser with some profile, and then selecting some other profile would go via about:profiles.

if we run ff -ProfileManager, with my second patch we open about:profiles and from there we can open any existing profile.

I really would like to proceed with this bug. Do we prefer to continue the discussion by email? Or do we prefer to organize a meeting?

Comment on attachment 8628129 [details] [diff] [review] patch 1 - about:profiles Benjamin, can you review these 2 patches?

Comment on attachment 8628129 [details] [diff] [review] patch 1 - about:profiles Sorry, I'll get back to these very soon, promise!

Comment on attachment 8628129 [details] [diff] [review] patch 1 - about:profiles Review of attachment 8628129 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/locales/en-US/chrome/global/aboutProfiles.dtd @@ +2,5 @@ > + - License, v. 2.0. If a copy of the MPL was not distributed with this > + - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > + > +<!ENTITY aboutProfiles.title "About Profiles"> > +<!ENTITY aboutProfiles.subtitle "This page helps you to manage your profiles. Each profile is a separate world who contains its own history, bookmarks, settings and Add-ons. Having multiple profiles can improve the security and the privacy of your web experience."> Here is a suggestion for a better text: This page helps you to manage your profiles. Each profile is a separate world which contains separate history, bookmarks, settings and add-ons. Also, I would personally remove the following sentence. "Having multiple profiles can improve the security and the privacy of your web experience." I know what you're trying to explain here but I think this one sentence doesn't capture it. If you think it's useful, we can have a "Learn More" link that links to an article on SUMO which explains things in more detail. @@ +3,5 @@ > + - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > + > +<!ENTITY aboutProfiles.title "About Profiles"> > +<!ENTITY aboutProfiles.subtitle "This page helps you to manage your profiles. Each profile is a separate world who contains its own history, bookmarks, settings and Add-ons. Having multiple profiles can improve the security and the privacy of your web experience."> > +<!ENTITY aboutProfiles.create "Create a new Profile"> Nit: "New" @@ +6,5 @@ > +<!ENTITY aboutProfiles.subtitle "This page helps you to manage your profiles. Each profile is a separate world who contains its own history, bookmarks, settings and Add-ons. Having multiple profiles can improve the security and the privacy of your web experience."> > +<!ENTITY aboutProfiles.create "Create a new Profile"> > +<!ENTITY aboutProfiles.restart.title "Restart"> > +<!ENTITY aboutProfiles.restart.inSafeMode "Restart with Add-ons Disabled..."> > +<!ENTITY aboutProfiles.restart.normal "Restart normally..."> Please use an ellipsis for the last two strings. ::: toolkit/locales/en-US/chrome/global/aboutProfiles.properties @@ +2,5 @@ > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +name = Profile: %S > +isDefault: Default Profile Hmm, why the colons? I was under the impression that the format of the properties file is lines containing |name = value| pairs. Am I missing something? Please use =. @@ +3,5 @@ > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +name = Profile: %S > +isDefault: Default Profile > +isSelected: Profile in-use Looks unused? Please re-check all strings to make sure everything is being used. @@ +5,5 @@ > +name = Profile: %S > +isDefault: Default Profile > +isSelected: Profile in-use > +rootDir: Root Directory > +# LOCALIZATION NOTE: information about local directory can be read in nsIProfileLock Please change this l10n note according to the nsIProfileLock.localDirectory comments. An example would be nice. Also please mention that if this is the same as rootDir, it won't be displayed.

Comment on attachment 8628749 [details] [diff] [review] patch 2 - Profile Manager Review of attachment 8628749 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/content/aboutProfiles.js @@ +15,5 @@ > '@mozilla.org/toolkit/profile-service;1', > 'nsIToolkitProfileService' > ); > > +const gProfileManager = window.location.href.indexOf('?profileManager') != -1; How about "?manage"? about:profiles?manage looks like a more sane URL. Also, please name the variable gManage. ::: toolkit/locales/en-US/chrome/global/aboutProfiles.dtd @@ +6,5 @@ > <!ENTITY aboutProfiles.subtitle "This page helps you to manage your profiles. Each profile is a separate world who contains its own history, bookmarks, settings and Add-ons. Having multiple profiles can improve the security and the privacy of your web experience."> > <!ENTITY aboutProfiles.create "Create a new Profile"> > <!ENTITY aboutProfiles.restart.title "Restart"> > +<!ENTITY aboutProfiles.restart.inSafeMode "Restart with Add-ons Disabled…"> > +<!ENTITY aboutProfiles.restart.normal "Restart normally…"> These will need to go in the previous patch. ::: toolkit/locales/en-US/chrome/global/aboutProfiles.properties @@ +12,5 @@ > > rename: Rename > remove: Remove > setAsDefault: Set as default profile > +open: Open Same comment about the colons. ::: toolkit/profile/content/profileSelection.js @@ +17,5 @@ > var gProfileService; > > function startup() > { > + gDialogParams = window.arguments[0].QueryInterface(I.nsIDialogParamBlock); Do you mind converting the C and I variables to the canonical names in a follow-up, please?

Comment on attachment 8696741 [details] [diff] [review] patch 3 - 'Profiles' entry in the 'tool' menu Let's not do this in this bug. We can consider it in the future...

ehsan, thanks a lot for these reviews. I want to land these patches but in separately: First I want the about:profile, then in a separate bug, I want to land the replacement of ProfileManager.

(In reply to Andrea Marchesini (:baku) from comment #2) > (In reply to Olli Pettay [:smaug] from comment #1) > > Does the patch ensure that currently used profiles can't be removed? > > Yes. You cannot remove the active profile. You cannot remove the *default profile* but you *can* remove the active profile if you don't use the default profile. Reported by a user in the German Firefox forum and confirmed by me.

Is it an idea to optimise the about:profiles a bit? At the moment it is taking up a huge amount of space just to show 2 profiles?

Please file new bugs about the tweaks the new profile manager or about:profiles need and make those bugs block this one. I like the new UI, but agree it needs some changes.

Yes, please, file a separate bug and CC/NI/Assign it to me. Thanks!

I think improving the Profile Manager is definitely worth doing; without a good one users are often given no choice but to use another browser. Chrome has true multi-user support, which I assume for us is a long away away, but would love to see. I'm hoping Madhava can assign someone from UX to improve the design of this feature and am NIing him. IMO it should primarily be about launching profiles, and secondarily about CRUD.