MAR files do not contain updates to the uninstaller The uninstaller ZIP file that is included in the Windows installer is currently not included in the MAR files. This means that an update may result in a Firefox that the user cannot uninstall. I don't know if this blocks beta, but it certainly blocks Firefox 1.5 final.

This is entirely unpleasant :-( What do you guys think about always shipping the uninstaller (even in windows zip builds)? I'm looking at the build scripts now to figure out whether that is feasible.

Yes, I think that would be a decent solution. Then we would leave it to nsPostUpdateWin.js to copy the uninstaller into C:\Windows.

I've looked at this, and it sucks more than a little (the uninstaller is a self-extracting .exe which is then packed up in a ZIPfile for the installer). What are the chances we could just punt on this altogether? Other than the official/unofficial branding stuff (which we don't really need to worry about), there isn't anything in this file that I forsee needing any changes before the next major release cycle, when this will be entirely revamped anyway. There are no version numbers or other release-specific information in this file.

Benjamin: Hmm.... uninstaller.ini is also in that self-extracting executable, and it contains the product name. That won't be changing now that we've rebranded. It was an issue for the "Deer Park Alpha 1" -> "Deer Park Alpha 2" transition. So, yeah... I think we could punt on this for the 1.5 release... until or unless we end up needing to rev the 1.5b1 version of the uninstaller ;-)

Did this ever get fixed for 1.5?

No, we decided not to fix this for 1.5 (and it probably won't need to be fixed for ffox 2 or the trunk since we're switching installer mechanisms).

I think this will need to be fixed for FF 2 at least since there surely won't be time to write a new installer/uninstaller for FF 2.

On the contrary, we are planning a new installer for ff2 which does stub downloading and other things. See http://wiki.mozilla.org/User:Beltzner/Specification_of_Stub_Installer_User_Interface

> On the contrary, we are planning a new installer for ff2 which does stub > downloading and other things. See Nice. Who is implementing that? I don't see this work on the FF 2 doc: http://wiki.mozilla.org/Firefox:2.0_Product_Planning:Draft_Plan At any rate, even if we change the install/uninstall mechanism for FF 2, we will still need to solve this bug. Afterall, FF 1.5 will need to know how to update users to FF 2, and I think that means hooking up the new uninstaller properly and cleaning up after the old one. I did some further investigation, and it appears that UninstallFirefox.exe keeps running even after spawning UNINSTALL.exe to do the actual work. I presume UninstallFirefox.exe goes away so that it can delete UNINSTALL.exe after it completes its work or something like that. I think it would work better to have UNINSTALL.exe and UNINSTALL.ini live in the application folder, and when run, UNINSTALL.exe should make a copy of itself in the users temp folder, and then spawn that copy and simply exit. That way, the uninstaller can delete everything in the application folder including itself.

This does the trick. Some issues: - I just learned today that we set the Show/HideIconsCommand registry entries to point to the uninstaller. I'm not sure why the uninstaller is the beast that handles those operations, and I'm not sure that it actually does so correctly, but nonetheless those registry keys are not updated by the update system (i.e., nsPostUpdateWin.js doesn't modify them). - If sunbird builds an installer/uninstaller, then it will need similar changes. - We probably want to use the same OpenProcess+WaitForSingleObject loop in updater.cpp since I believe that that one sometimes (very rarely) has problems that prevent the update from going through.

Whoops.. please ignore that last patch. It had a glitch in the uninstaller's code that spawned the second instance.

I've tested this patch using my own release build of FF off the 1.8 branch. I haven't built with official branding, but hopefully that won't be an issue. I've included the necessary changes for Thunderbird, but I haven't verified that they work properly. I think they will ;-) I would like to get this in on the trunk, and then land it on the 1.8 branch for FF 2 and FF 1.5.0.1 (so that we can implement bug 320504). Per suggestions from bsmedberg, the localization files for the uninstaller live in browser.xpi/mail.xpi instead of ab-CD.xpi because 1) the uninstaller is not localized today, and 2) we do not want to try to change the set of strings to be localized between FF 1.5 and 1.5.0.1. We can clean up those details for FF 2 if desired.

thanks for making the tbird changes Darin. I can spin up a test build and try it out with your patch if you like, otherwise we can test it in the trunk when you check in :)

Let's get this in and tested on the trunk ASAP if you're trying to hit 1.5.0.1

Comment on attachment 207462 [details] [diff] [review] v1.1 patch >Index: browser/locales/Makefile.in >+ifdef MOZ_INSTALLER >+ifeq ($(OS_ARCH),WINNT) >+ >+instgen/uninstaller.inc: \ >+ $(addprefix $(LOCALE_SRCDIR)/,installer/uninstaller.inc) 1) Don't continue makefile dependencies on new lines, just let them grow beyond 80 chars >- "DisplayIcon", pathToExe + ",0", // XXX don't hardcode me! >+ "DisplayIcon", pathToExe + ",0", // XXX dont hardcode me! The original apostrophe was correct.

Gah, all the important parts of my patch comments were lost, hang on :-(

Comment on attachment 207462 [details] [diff] [review] v1.1 patch >Index: browser/locales/Makefile.in >+instgen/uninstaller.inc: \ >+ $(addprefix $(LOCALE_SRCDIR)/,installer/uninstaller.inc) >+ $(NSINSTALL) -D instgen >+ iconv -f UTF-8 -t $(WIN_INSTALLER_CHARSET) $< > instgen/uninstaller.inc 2) This rule is going to end up being called during locale repackaging, which is not what we want. I think this entire set of rules should be put in browser/Makefile.in for the time being, and please hardcode the en-US path intead of using LOCALE_SRCDIR >Index: browser/locales/en-US/installer/uninstaller.inc >+#define BRAND_EXE_NAME firefox.exe Yuck: we shouldn't be putting a locale-constant filename in a locale file. Please just hardcode this and get rid of BRAND_EXE_NAME altogether. Same comments for mail/

> 1) Don't continue makefile dependencies on new lines, just let them grow > beyond 80 chars Why? 80 chars is more readable, I think. > >- "DisplayIcon", pathToExe + ",0", // XXX don't hardcode me! > >+ "DisplayIcon", pathToExe + ",0", // XXX dont hardcode me! > > The original apostrophe was correct. But it confused my editor's JS syntax highlighting! ;-) Maybe I can fix my editor instead...

> 2) This rule is going to end up being called during locale repackaging, which > is not what we want. I think this entire set of rules should be put in > browser/Makefile.in for the time being, and please hardcode the en-US path > intead of using LOCALE_SRCDIR OK, no problem. > >Index: browser/locales/en-US/installer/uninstaller.inc > > >+#define BRAND_EXE_NAME firefox.exe > > Yuck: we shouldn't be putting a locale-constant filename in a locale file. > Please just hardcode this and get rid of BRAND_EXE_NAME altogether. > > Same comments for mail/ Good point. I think the original value came from installer.cfg, which is not a localized file, so it makes sense to hardcode firefox.exe in uninstall.it. Will do.

Here's the revised patch.

Comment on attachment 208043 [details] [diff] [review] v1.2 patch OK, this is in on the trunk now. I am told that this is too late to make it for FF 1.5.0.1, which is unfortunate. Therefore, I am requesting approval for FF 1.5.0.2.

fixed-on-trunk

It appears that with this patch installed the uninstaller files are not correctly modified for Deer Park. If you try to unistall Deer Park, it will either unistall Mozilla Firefox 1.5 (if you have that installed) or get an error saying it can't find the unistall log.

Bill, yes it is true. I discovered the cause of the problem while developing the patch for bug 320504. When calling CreateProcess, I was not prefixing the command line string with the executable name, and so when the uninstaller spawned itself from the temporary directory, it trimmed off the "/ua" command line switch, which caused the uninstaller to be confused about what it was uninstalling. The bug slipped through my testing because I was only testing on the 1.8 branch, where the brand name is Firefox and not Deerpark.

I think this patch might have caused a regression. If I am in <objdir>/browser, and type "make", I get the following result: make: `../dist/bin/uninstall/uninstall.ini' is up to date. The rest of the browser directory is not built, no matter what changes I've made. Reverting the changes to browser/Makefile.in restores expected functionality.

> The rest of the browser directory is not built, no matter what changes I've > made. Yikes.. I'm seeing that too. Investigating...

(In reply to comment #26) > I think this patch might have caused a regression. If I am in <objdir>/browser, > and type "make", I get the following result: > > make: `../dist/bin/uninstall/uninstall.ini' is up to date. > > The rest of the browser directory is not built, no matter what changes I've > made. Reverting the changes to browser/Makefile.in restores expected > functionality. > Not surprisingly, I see this too in Thunderbird on the trunk.

> Not surprisingly, I see this too in Thunderbird on the trunk. Yeah, makes sense. I checked in a fix for thunderbird too. Please update mail/Makefile.in, and let me know if it didn't work. Thanks!

This is a follow-up patch for the trunk. I realized that we need to quote the file path of the uninstaller since it could contain spaces. On my system it does not, apparently because GetTempPath returns a "short" file path. But, support for short file paths may be disabled, so this patch is needed.

Here's the patch for the branch(es).

Comment on attachment 208167 [details] [diff] [review] v1.3 patch - for 1.8/1.8.0 branches missed 1.8.0.1

Marking this bug FIXED again since everything is in on the trunk.

The issue I brought up in Comment #24 still persists in the latest Windows Official Nightly, which contians all of these fixes. It is Still not possible to uninstall a Trunk build because the unistall.ini file has the incorrect product name (Mozilla Firefox instead of Deer Park ALpha 2).

The January 27, 2006 latest-trunk nightly of Deer Park Alpha 2 for Windows (.exe version) still cannot be uninstalled. "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060127 Firefox/1.6a1"

"Uninstall log folder not found" problem still exists with 01-29-06 latest-trunk version (Deer Park) for Windows. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060129 Firefox/1.6a1(In reply to comment #36) > The January 27, 2006 latest-trunk nightly of Deer Park Alpha 2 for Windows > (.exe version) still cannot be uninstalled. > > "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060127 > Firefox/1.6a1" >

Comment on attachment 208167 [details] [diff] [review] v1.3 patch - for 1.8/1.8.0 branches Darin, I think we'd like to understand (and hopefully fix) the DeerPark/Firefox issue before proceeding to the branches. Is there build-config work that needs to happen for official/unofficial branding?

(In reply to comment #38) > (From update of attachment 208167 [details] [diff] [review] [edit]) > Darin, I think we'd like to understand (and hopefully fix) the DeerPark/Firefox > issue before proceeding to the branches. Is there build-config work that needs > to happen for official/unofficial branding? > Actually this is more important for Thunderbird. One could argue that for the Branch with official branding, the product name will be correct, but this only holds true for Firefox. For Thunderibrd the uninstall.ini file ends up having the following: Company Name=@BRAND_COMPANY_NAME@ Product Name=@BRAND_PRODUCT_NAME@ So if this were to land on the branch as is, it would nmost likely not be possible to uninstall Thunderbird.

I plan on investigating this bug shortly. I agree that we should hold off on landing this on the release branch until after this bug has been resolved.

"Uninstall log folder not found" problem appears to have been fixed with the 02-02-06 latest-trunk version (Deer Park) for Windows. I just uninstalled it without problem, then installed the 02-03-06 version. Thank you.

Comment on attachment 208043 [details] [diff] [review] v1.2 patch Bsmedberg, I would like to synchronize the 1.8 branch uninstaller and update code with that of the trunk. I know that Rob is working on a new uninstaller, but in the meantime, let's get the trunk and 1.8 branch synchronized.

Comment on attachment 208043 [details] [diff] [review] v1.2 patch a=me to sync trunk/1.8branch

While it is true that the "Uninstall log folder not found" problem appears to have been fixed with the 02-02-06 latest-trunk version (Deer Park) for Windows, there is a cruel irony (OK, no big deal): After uninstalling Deer Park, a new message window appears asking what you think of Deer Park. If you choose to respond, you are sent to: https://survey.mozilla.com/1/Deer%20Park%20Alpha%202/1.6a1%20(en-US)/exit.html Of course, you just uninstalled Deer Park, so Internet Explorer opens to take you there. And that link doesn't work anyway. But at least Deer Park can be uninstalled properly (so the newest latest-trunk version can be installed). Thanks.

fixed1.8.1

Tenatively marked blocking, but we need to work out the QA resources before approving the actual patch.

This bug is fixed1.8.0.2 by the patch for bug 320504.

Please comment if I've got this wrong: To verify the fix, we'll need to use software update to update to a recent 1.5.0.2 candidate, then uninstall. If the exit survey comes up, this bug can be marked as verified.

correct