PRD Item INST-003a Mike, I could use your help with the strings on this dialog. I'd also like to display an "Are you really really realllllly sure" messagebox prior to uninstalling when this checkbox is checked and could use your string wizardry there if you are ok with displaying a messagebox when the checkbox is checked.

FWIW, I still think this is a very bad idea and should be WONTFIX: see http://blogs.msdn.com/oldnewthing/archive/2007/09/17/4948130.aspx for a cogent argument.

I agree for the most part but we are only doing this for the current as stated in that blog entry: "Now, if you want, you can clean up the per-user data for the current user (after asking for permission), but you definitely should not be messing with the profiles of other users." That's exactly what this bug is about. The default will be to not remove the profiles and if I get my way the user will have to confirm that they "really, really, realllllly" want to do this before proceeding.

Requesting blocking‑firefox3 since this was added to the PRD as a P1

btw: I have a patch ready to go as long as the ui isn't made too complex (e.g. installers typically don't have all of the ui options as apps have) as soon as I get the strings.

Comment on attachment 283399 [details] screenshot "Profiles" is obviously not the thing, since it requires an understanding of the multiple profile thing which we don't officially support :) How about: [ ] Remove my &brandShortName personal data clicking that could cause the following to appear below on a yellow background or as a confirmation dialog: Removing Personal Data This will permanently remove your bookmarks, saved passwords, cookies and customizations. You may wish to keep this information if you plan on installing another version of &brandShortName in the future. (( Remove my personal data )) ( Keep my personal data )

This implements your comments inside the wizard.

To circumvent problems with locales I've decided to go with a scrollbar on this control.

Comment on attachment 285292 [details] [diff] [review] patch rev1 (requires patch from bug 399381 and Bug 399665 to apply cleanly) Seth brought up an excellent point non relative regarding profiles possibly being directories we don't want to delete. For example My Documents or for that matter any directory that wasn't created by the app since it may contain user data not related to the profile. Firefox doesn't try to prevent using these directories and deleting them would be to say the least bad. For non-relative directories I will probably just delete extremely well-known directories and files.

< + RmDir /r "$R7\bookmarkbackups" < + RmDir /r "$R7\Cache" < + RmDir /r "$R7\chrome" < + RmDir /r "$R7\extensions" < + RmDir /r "$R7\minidumps" < + Delete "$R7\bookmarks.html" < + Delete "$R7\bookmarks.preplaces.html" < + Delete "$R7\cert8.db" < + Delete "$R7\compatibility.ini" < + Delete "$R7\compreg.dat" < + Delete "$R7\content-prefs.sqlite" < + Delete "$R7\cookies.sqlite" < + Delete "$R7\downloads.sqlite" < + Delete "$R7\extensions.cache" < + Delete "$R7\extensions.ini" < + Delete "$R7\extensions.rdf" < + Delete "$R7\formhistory.sqlite" < + Delete "$R7\key3.db" < + Delete "$R7\localstore.rdf" < + Delete "$R7\places.sqlite" < + Delete "$R7\pluginreg.dat" < + Delete "$R7\prefs.js" < + Delete "$R7\search.sqlite" < + Delete "$R7\secmod.db" < + Delete "$R7\signons2.txt" < + Delete "$R7\urlclassifier3.sqlite" < + Delete "$R7\XPC.mfl" < + Delete "$R7\xpti.dat" < + Delete "$R7\XUL.mfl" For grins, I looked in my profile dir and I had these files that are not on your list: blocklist.xml formhistory.sqlite localstore.rdf mimeTypes.rdf sessionstore.bak sessionstore.js webappsstore.sqlite Perhaps instead we could just check for existence of some well know / guaranteed to be there files in $R7 (to make sure we're in the right place), and if we find them, then RmDir /r "$R7". What do you think, robert?

We can't do that since the non relative profile could very well be "My Documents", the user's desktop, or a user created directory that has user documents that they selected in the profile manager so at best we can RmDir /r well known dirs in the profile and list out the well known files... hence my "yeah right" comment about being able to remove the profile dir when done. I can add those files easily though I know we will almost always be missing some.

Comment on attachment 285390 [details] [diff] [review] patch rev2 (requires patch from bug 399381 and Bug 399665 to apply cleanly) r=sspitzer, thanks for clarifying robert. a few questions / comments: 1) can you elaborate the comment to explain why we are doing RmDir "$R7" (and not RmDir /r "$R7), for the My Documents scenario? 2) Was this prd item (PRD Item INST-003a) a request from a partner, or something we wanted to do for firefox? if for partners only, is it possible to not show this checkbox at all (for the standard firefox version of the uninstaller)? reading that blog post that bsmedberg linked to points out some good reasons why removing user data on uninstall can be a bad idea.

RmDir "$R7" is so we don't remove a directory unless it is empty. So, if the user selected "My Documents" for their profile directory it will only be deleted if it is empty after the specific dir / file removal. It is for Firefox in general and was requested by mconnor. Agreed regarding the blog post. I think it is also true that it is a bad idea in general to have an app provide the ability to have multiple profiles as we do. As the blog post also points out it is acceptable to remove the current user's personal data for an app and it would be much simpler for us to provide this option if we didn't provide the ability for the user to specify a profile path outside of the one under appdata\mozilla\firefox\profiles. It is also important to note that with offline apps our cache will only increase in size. I'm going to discuss the non relative profile case with mconnor to figure out what we should do here. To be blunt, I'm not terribly thrilled with doing this from the uninstaller in the first place since it can never have the exact same logic as used by the app and the potential for evil happenings is rather high. On the other hand I do understand why this has been asked for in the past and was added to the PRD.

Non relative we will not delete... patch coming up.

Seth, could you give this a once over? The only change is that we no longer try to remove non relative profiles per mconnor.

Comment on attachment 285411 [details] [diff] [review] patch rev3 (requires patch from bug 399381 and Bug 399665 to apply cleanly) r=sspitzer, thanks robert.

Comment on attachment 285411 [details] [diff] [review] patch rev3 (requires patch from bug 399381 and Bug 399665 to apply cleanly) Requesting a1.9 for this P1 PRD item

Comment on attachment 285278 [details] screenshot as discussed in over-the-shoulder review, the scrollbar makes me sad. instead, let's move the checkbox so that it's one linespace beneath the textbox, and then leave the remaining space for the warning (which should allow for plenty of l10n leeway) Also, I think maybe "Remove my history and bookmarks" works better than "Remove my Minefield personal data"; I know it's not 100% accurate, but it's more likely to cue users to the actual choice they're making.

After discussing with Mike we've decided to go with "Remove my Minefield personal data and customizations"

Comment on attachment 285550 [details] screenshot Received a verbal ui-r+ from beltzner with the string change

Comment on attachment 285557 [details] [diff] [review] patch - updated to comments and w/o dependencies on bug 399665 >Index: browser/locales/en-US/installer/custom.properties >=================================================================== >RCS file: /cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v >retrieving revision 1.10 >diff -u -8 -r1.10 custom.properties >--- browser/locales/en-US/installer/custom.properties 21 Sep 2007 22:09:04 -0000 1.10 >+++ browser/locales/en-US/installer/custom.properties 20 Oct 2007 01:06:17 -0000 >@@ -76,16 +76,23 @@ >+UN_CONFIRM_PAGE_TITLE=Uninstall ${BrandFullName} >+UN_CONFIRM_PAGE_SUBTITLE=Remove ${BrandFullName} from your computer. >+UN_CONFIRM_UNINSTALLED_FROM=${BrandShortName} will be uninstalled from the following location: >+UN_CONFIRM_CLICK=Click Uninstall to continue. >+UN_REMOVE_PROFILES=&Remove my ${BrandShortName} personal data >+UN_REMOVE_PROFILES_DESC=This will permanently remove your bookmarks, saved passwords, cookies and customizations. You may wish to keep this information if you plan on installing another version of $BrandShortName in the future. s/$BrandShortName/${BrandShortName} I will fix this before checkin

s/$BrandShortName/${BrandShortName} I will fix this before checkin robert, you know that perl script that is part of this bug? Should we run it on en-US as well, to help catch mistakes like that? (or would that not work?)

It is actually a part of bug 399665 and the fix I mentioned has to do with this bug already being set to blocking Firefox 3.0 since it is a P1 PRD item and bug 399665 not having a1.9 yet since it is a P2 PRD item. I considered it and I think the note in the locale file should be sufficient.

bah... I wasn't clear... it wouldn't have caught this mistake since I was backing out the dependency on bug 399665 as mentioned in the patch description

Checked in to trunk Checking in mozilla/browser/installer/windows/nsis/uninstaller.nsi; /cvsroot/mozilla/browser/installer/windows/nsis/uninstaller.nsi,v <-- uninstaller.nsi new revision: 1.15; previous revision: 1.14 done Checking in mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh; /cvsroot/mozilla/toolkit/mozapps/installer/windows/nsis/common.nsh,v <-- common.nsh new revision: 1.31; previous revision: 1.30 done Checking in mozilla/browser/locales/en-US/installer/custom.properties; /cvsroot/mozilla/browser/locales/en-US/installer/custom.properties,v <-- custom.properties new revision: 1.11; previous revision: 1.10 done

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007102407 Minefield/3.0a9pre on a clean Vista Business VM. The only issue I see, and I am not sure whether it is expected, is an empty Firefox directory persists in the AppsData\Roaming\Mozilla directory.

I didn't want to remove the Firefox dir since people sometimes copy profile files to that location