281235 - installer fails to remove all files during uninstall

Status: RESOLVED WONTFIX

(Firefox :: Installer, defect)

Description

[Mike Connor [:mconnor]]

spun off from bug 233625 in order to actually get something useful done.

When installing, since we don't append a directory to the selected install path,
you can in theory install into the root of C:\Program Files\ and elsewhere. 
Then, when we uninstall, we detect files left over, so we ask the user if they
want to delete the contents of the folder.  Given that users don't read dialogs,
we end up with users who have nuked their entire program files directory.

Problems linked into this:

Should we only install to an empty directory unless we know its a previous
Firefox install?  And should we test for the possibility that they already did
something bad?

Can we replace the "do you want to remove the entire contents of X" message with
a warning that not all files were deleted, with an option to open the install
folder to do manual cleanup?  This is used by other apps in varying forms.  We
could record whether the folder was empty initiallly, and allow automatic
purging in that case, if we really want to retain that functionality.

Comment 1

[Ben Basson]

I think that sums it up nicely Mike, but I'd like to add that if installing to
an existing Firefox directory, the installer should probably replace any
existing files. 

Strictly speaking this should be in another bug, but it's related and would help
avoid possible unfinished uninstall / reinstall messes that could occur.

Comment 2

[Ben Basson]

> the installer should probably replace any
> existing files. 

Ok, I'm going back to bed. I meant to say that it should empty the directory.

Comment 3

[Asa Dotzler [:asa]]

I think what we want to do is just force install to a /firefox directory. That
solves most of our problems. Ben, can you look into that?

Comment 4

[Daniel Veditz [:dveditz]]

I think what we need to do for 1.0.1 is comment out the call to
VerifyAndDeleteInstallationFolder() in the UN-installer.  Who knows how many
people have already installed to a dangerous place, fixing a future installer
will not help them since we'll default to an existing installation directory.

If we're leaving stuff behind we can fix it in a less dangerous way in 1.1, but
leaving a few files around (if we do) is a trivial bug compared to nuking
"Program Files".

Comment 5

[Daniel Veditz [:dveditz]]

Stop-gap fix checked in to uninstall in bug 233265. I don't think the remaining
issues need to block 1.0.1, kicking back to nomination status for reconsideration.

Comment 6

[Greg Campbell]

I don't think forcing an install into a /firefox directory is necessary. The
installer should log exactly what files it installed. During the uninstall, it
should delete only those files. If the install directory is empty upon deleting
all those files, the directory can also be deleted. If not empty, the installer
should say something like "the following files could not be removed," list them
in a scrolling box, and that's it.

The problem is when plugins and search plugins add things to the firefox
directory. In Windows, these things should all go in USER\%APPDATA% or (for
global plugins) All Users\%APPDATA%. Maybe this bug should depend on Bug 123315,
Bug 232638 et al.

Comment 7

[chris hofmann]

this bug is now for larger work to follow in 1.1.  minus for 1.0.1

Comment 8

[u88484]

forcing an install into a /firefox directory is not at all the right step. I,
along with other should be able to install Firefox where ever I(we) want. It
should be the uninstallers job to know what files need to be deleted upon the
uninstall. Most notable programs have a log that the uninstaller uses to know
which files to delete, so why can't firefox? 

Whatever the outcome may be for this bug, forcing to install into any folder in
not at all the correct decision!

Comment 9

[Worcester12345]

It wouldn't allow me to add "dataloss" keyword.

Comment 10

[Asa Dotzler [:asa]]

sort of worked around. not gonna block.

Comment 11

[Jaime Mitchell (use [e-mail] for email)]

*** Bug 302114 has been marked as a duplicate of this bug. ***

Comment 12

[Worcester12345]

(In reply to comment #11)
> *** Bug 302114 has been marked as a duplicate of this bug. ***

Comment 13

[Mike Connor [:mconnor]]

Building the new installer is trunk work, as it is we just ignore leftover files, which is acceptable for now.

Comment 14

[Mike Connor [:mconnor]]

To clarify, this isn't a dataloss bug, its really just about finding a better uninstall mechanism so that we cleanly uninstall.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

I think the better uninstall mechanism is a policy that states:

 - any file added to the %installdir% must be added to the uninstall list
 - plugins, addons, and other things we don't neccessarily control should go in subdirs off the %installdir%

That way we can always be sure to clean up *our* mess, and only *our* mess.

Comment 16

[Mike Connor [:mconnor]]

Rob, are we more or less where Beltzner is suggesting we need to be?

Comment 17

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

If the file is listed in the proper format in the uninstall/uninstall.log then we will remove it and if after removing the directory is empty we will walk up to the install dir and remove empty directories along the way. We don't try to clean up any file *mess* that is not listed in the uninstall/uninstall.log or prompt the user to provide the option to delete empty directories.

Comment 18

[Mike Connor [:mconnor]]

I think we do this as safely as we can, anything else carries the risk of dataloss in edgecases, and isn't really worth it.  Marking WONTFIX.