234744 - auto and silent modes should check destination directory

Status: RESOLVED FIXED

(SeaMonkey :: Installer, defect)

Description

[Andrew Schultz]

this timed out trying to submit... let's try this:

auto and silent installer modes skip SetupTypeDlg and go straight to
InstallerDlg.  This means they skip SetupTypeDlg's VerifyDestination,
DeleteOldInst and VerifyDiskSpace.

Things aren't so bad because XPInstall creates the Mozilla directory when it
tries to create the first file.  But it also happily installs in old Mozilla
directories without deleting them, which is bad.

The installer should delete the directory if it's a Mozilla directory and bail
if it's a non-Mozilla directory (bug 234741)

Comment 1

[Andrew Schultz]

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

Comment 2

[Alexander Opitz]

But when is a mozilla directory a mozilla directory ;-)

Comment 3

[Andrew Schultz]

based on the same criteria that it has now.  If the directory has "mozilla-bin"
(or anything else specified in the "Legacy Check"), then it's a Mozilla
directory.  If the definition of a "Mozilla directory" changes, then the
behavior of auto/silent mode will come along for the ride.

Comment 4

[Andrew Schultz]

Attachment: patch

this patch fixes the bug.  in addition to what might be expected, this patch
1. makes E_MKDIR_FAIL a fatal error for auto/silent modes.
2. make VerifyDestination and DeleteOldInst always return to
nsSetupTypeDlg::Next() without firing off nsInstallDlg.  Without this change,
VerifyDiskSpace was called ONLY when installing to a pre-existing empty
directory.
3. make the installer bail on fatal errors in silent mode.

Comment 5

[Andrew Schultz]

Attachment: patch

This moves work out of nsXInstaller and creates a directory after deleting it.

The directory creation won't be needed after bug 240677 since the directory
itself won't be deleted.  I ripped out the umask dance from CreateDestYes to be
consistent with what I added.  The umask code has no effect:

The resulting perms are (0777 & ~oldUmask) & ~022   (~022 = 0755)

& is commutative, so that's just 0755 & ~oldUmask, but you can get that by just
using 0755 in the first place.

Comment 6

[Andrew Schultz]

Attachment: patch with diff -b

the last part of the patch is easier to read here

Comment 7

[Andrew Schultz]

Attachment: patch merged to trunk

this is the previous patch merged to trunk + a bMoving hack to make gtk1 happy.

Comment 8

[Andrew Schultz]

Comment on attachment 147860 [details] [diff] [review]
patch merged to trunk

oops.
bryner, I really wanted your sr=.  I already had r=bsmedberg.

Comment 9

[Andrew Schultz]

fixed