547225 - Split the uninstaller and the default browser/registry integration code into separate executables

Status: RESOLVED WONTFIX

(Firefox :: Installer, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

When we start producing MSI installation packages we don't want to ship the NSIS uninstaller with them.  Currently the NSIS installer also includes all the code to handle registry entries for default browser stuff/shell integration, etc.

Splitting that code apart is the first step to not shipping a deadweight NSIS uninstaller in the MSI package.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Robert, Axel,

Would something like the INI file that the crashreporter uses[0] be acceptable for l10n purposes?  If not, what's the best way to go about localizing a native Win32 executable?

[0] http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/crashreporter/crashreporter.ini

Comment 2

[Robert Strong (they/them - no direct email)]

Yes for the registry / file system strings that need to be localized though there shouldn't be UI displayed... true?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Correct.  There will be no UI at all, only registry and file system strings will need to be localized.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 5

[Robert Strong (they/them - no direct email)]

Awesome turnaround on this. I'm not going to have time to review this for a few days but I do have a couple of changes. The exe name should be something other than helper.exe... we compromised on that name since it was serving as both the uninstaller and the exe to set defaults. Not sure what to go with yet but I'll think of something if you don't come up with something first. Instead of putting the files under browser/installer please put them in browser/components/shell/. If possible it would be great if the exe could be built somewhere in toolkit if it could be made generic enough to support all Windows toolkit apps.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #5)
> Awesome turnaround on this. I'm not going to have time to review this for a few
> days but I do have a couple of changes. The exe name should be something other
> than helper.exe... we compromised on that name since it was serving as both the
> uninstaller and the exe to set defaults. Not sure what to go with yet but I'll
> think of something if you don't come up with something first. Instead of
> putting the files under browser/installer please put them in
> browser/components/shell/. If possible it would be great if the exe could be
> built somewhere in toolkit if it could be made generic enough to support all
> Windows toolkit apps.

That's fine, I'm not going to have time to push forward on anything else for at least a week because of midterms.

I put some of the basic primitive routines in a common file in toolkit/ (which I guess we'll want to move to toolkit/components/shell?) but a lot of the code is browser specific so I'm not sure how much better we can do.  I more or less translated the NSIS code to C++ so there's probably some refactoring we could do to make a few things more generic (FixShellIconHandler and maybe another function or two).

As for the name, how about "setdefault.exe" or "makedefault.exe"?

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://build.mozilla.org/tryserver-builds/[e-mail]-try-ffce78976e0b/

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Actually, that tryserver build was from a slightly earlier version.  New builds will show up at https://build.mozilla.org/tryserver-builds/[e-mail]-try-f6d9313a4f27/ in a bit.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch V1.1

Same idea as before, changed the location of the files and the name of the executable to setdefault.exe

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch V1.3

Comment 11

[Robert Strong (they/them - no direct email)]

Kyle, I have a couple of projects to finish in the next couple of weeks. Would you mind waiting on my reviewing this until next month? Jim Mathies might be able to take a look if you'd like it reviewed sooner or I might be able to get to it sooner if so.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #11)
> Kyle, I have a couple of projects to finish in the next couple of weeks. Would
> you mind waiting on my reviewing this until next month? Jim Mathies might be
> able to take a look if you'd like it reviewed sooner or I might be able to get
> to it sooner if so.

That's not a problem at all.  This doesn't really block anything on my end, and I'm fine with the review waiting for a while.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So I've been thinking about this and I'm not sure this is the best path forward.  I think a better path would be to write a small shim executable that passes the call along to either the NSIS helper/uninstaller or calls into the appropriate MSI APIs (detecting at runtime which is appropriate).  All of the shortcuts and registry entries that we want to create can be done with native MSI facilities and the few other things that we do (such as using AppAssocReg) can be recreated through simple custom actions.  Using the native facilities would make supporting rollback, proper uninstall, etc. trivial.

The downside is that we would have to maintain two separate lists of these shortcuts and registry entries (in the NSIS script and in the WiX XML), but that is IMO preferable to maintaining one copy in C++.

Comment 14

[Molly Howell (she/her) (mostly inactive)]

Closing out old bugs; we can refile something like this when/if we figure out how to handle MSI.