Closed Bug 237727 Opened 19 years ago Closed 18 years ago

Installer should delete certain files if we install over an existing firefox installation

Categories

(Firefox :: Installer, defect, P2)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
Firefox1.0beta

People

(Reporter: bugs, Assigned: sspitzer)

References

Details

(Keywords: fixed-aviary1.0)

Attachments

(3 files, 1 obsolete file)

We have a number of problems that are caused by installing over the top of an
existing Firefox installation. 

There are a number of ways around this:
1 Firefox used to just wipe C:\Program Files\Mozilla Firefox\ but this made
people very angry. 
2 Firefox could invoke the uninstaller for that directory if it can find one.
3 Firefox could show a message and invoke the Add/Remove programs window

I favor a combination of 2 & 3. If an uninstaller is found, try and run it,
otherwise do 3 and don't let the user proceed until the old install is gone.
This is such a problem we should really have some kind of interim solution by .9
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → Firefox0.9
The uninstaller for 0.8 apparently deletes more than it should, just like the
installer did before 0.8.  See bug 233625.  Because of this, I think calling an
old uninstaller is a bad idea.
Flags: blocking0.9?
greprefs makes this an absolute requirement, we had at least a dozen bugs
related to that and non-clean installs
Flags: blocking0.9? → blocking0.9+
scuttle this one out a little. 
Flags: blocking1.0+
Flags: blocking0.9-
Flags: blocking0.9+
Target Milestone: Firefox0.9 → Firefox1.0beta
I accidentally (?) installed 0.9.1 over 0.9, which I had installed over 0.8,
which made it so my user agent and credits ui looked like I was using 0.8, when
I was not.

let me attach a screen shot.
A good fix here might be for config.it to maintain a list of files that should
be removed by the newer installer. This introduces a maintenance cost in the
future as new files are added that may cause problems (e.g. new preferences
files) but these changes should be limited. Talk to me for more details. 
Assignee: bugs → dveditz
Status: ASSIGNED → NEW
Flags: blocking-aviary1.0RC1+
I think the current mozilla installer has support for removing files.  

I'll look into it.
updating summary, per a conversation I had with ben.
Assignee: dveditz → sspitzer
Summary: Installer should halt if an existing Firefox install is found → Installer should delete certain files if we install over an existing firefox installation
good news, as we thought, the installer has things in place to handle this sort
of problem.

see deleteThisFile() and deleteThisFolder() in
http://lxr.mozilla.org/mozilla/source/xpinstall/packager/common/share.t

for example usages, see
http://lxr.mozilla.org/mozilla/source/xpinstall/packager/windows/browser.jst#377

and stuff like:

function upgradeCleanup()
{
  // Obsolete files from Netscape 6.0 and Netscape 6.01 that
  // need to be cleaned up.
  deleteThisFile("Program",    "msgMapi.dll");
  deleteThisFile("Components", "signed.dll");
  deleteThisFile("Components", "smimestb.dll");
  deleteThisFile("Components", "nsMapiRegistry.dll");
  deleteThisFile("Components", "absyncsv.dll");
}
sorry for the delay, working on this now.

I'm going to see which files that 0.8, 0.9, 0.9.1 and 0.9.2 are installing, and
see which ones should be deleted.

then I'll fix upgradeCleanup() in
http://lxr.mozilla.org/aviarybranch/source/browser/installer/windows/browser.jst#314
to delete those files.

Then I'll do the same thing for linux.  I think mac is different and doesn't
need to worry about this, since mac doesn't use the installer.
Status: NEW → ASSIGNED
when comparing 0.8, 0.9, 0.9.1, 0.9.2 and a fresh build off of the aviary 1.0
branch, I come up with this list of files that need to be removed:

./.autoreg
./chrome/en-win.jar
./chrome/US.jar
./components/compreg.dat
./components/xpti.dat
./defaults/pref/all.js
./defaults/pref/security-prefs.js
./defaults/pref/winpref.js
./defaults/pref/xpinstall.js
./defaults/profile/US/panels.rdf

I'll work up a patch and double check this list with ben.
Attached patch patch (obsolete) — Splinter Review
Attached patch updated patchSplinter Review
Attachment #154257 - Attachment is obsolete: true
(In reply to comment #12)
If the installer just updates installed-chrome.txt, won't installed-chrome.txt
still have references to US.jar?
> If the installer just updates installed-chrome.txt, won't installed-chrome.txt
> still have references to US.jar?

hussam, good point.

I'll check with ben and see if it is dangerous to remove those chrome files.
seth: erm, autoreg's sole job in life is to tell the next xpcom app to register
components. when the app succeeds it tries to delete the file. you should never
need to delete compreg.dat, simply packaging or forcing the creation of .autoreg
should be sufficient.
Except that there's no good way to unregister components you're going to get rid
of. autoreg will pick up new components, but leave registered components you may
have just deleted.

In many cases this doesn't really matter, but it does bloat memory, and any that
are registered in any of several categories will cause us to waste time trying
to load them.

This is especially true if we have NOT deleted the dll that contains the
component, say we keep revving the class ID, we'll end up with multiple entries
for the same thing.

xpti.dat has a similar problem, old interfaces into sticks around and bloats
memory. At least with compreg.dat there's a theoretical way to unregister
components even though it's pretty unworkable in practice since we can't
unregister loaded libraries.
Flags: blocking-aviary1.0PR+ → blocking-aviary1.0PR-
Flags: blocking-aviary1.0PR- → blocking-aviary1.0PR+
br & trunk fixed. 
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Adding fixed-aviary1.0 keyword for searching purposes based on comment #21
Keywords: fixed-aviary1.0
Is there a separate bug for removing old versions from the "Add/Remove programs
window"?
this isn't the right place to ask that question, but yes - bug 247884
Depends on: 233625
Status: RESOLVED → VERIFIED
QA Contact: bugzilla → installer
You need to log in before you can comment on or make changes to this bug.