Closed Bug 190484 Opened 22 years ago Closed 22 years ago

After GRE install, user is asked to restart computer, ending rest of install

Categories

(Core Graveyard :: Installer: GRE, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: agracebush, Assigned: ssu0262)

References

Details

(Whiteboard: fixed1.3)

Attachments

(1 file, 1 obsolete file)

Steps to reproduce: 1. Download trunk build 2003012404 2. Run the installer- any setup type actual results: After GRE install, a dialog is presented to user notifying that files are in use and asking to restart computer now or restart computer later. If user chooses to restart now, the rest of installation is lost. If user chooses to not restart, the installation completes as expected. expected results: User is not asked to restart the computer
*** Bug 192829 has been marked as a duplicate of this bug. ***
This seems to be the most negative-reaction-provoking thing about 1.3b so far (admittedly, the data set thus far is small).
Flags: blocking1.3?
Agreed. Blocking 1.3 final.
Flags: blocking1.3? → blocking1.3+
Just adding my voice to the data set. Actually, with my distrust of Windows and knowing that Mozilla had never previously required a system restart, I seriously suspected that I had gotten a virus-infected version of the installer.
*** Bug 193440 has been marked as a duplicate of this bug. ***
Yep, Asa got it right, this is definately a blocker. I also thought I had a bad download. Even went back to redownload, with the same effect.
found the problem. working on a fix now.
Status: NEW → ASSIGNED
*** Bug 193503 has been marked as a duplicate of this bug. ***
should note I didn't get any notice to restart when I uninstalled Mozilla before installing. When installing and a version is already installed, it prompts me, then appears to cut off.
Attached patch patch v1.0 (obsolete) (deleted) — Splinter Review
this patch will now prevent the GRE installer from throwing up the Reboot dialog. Instead, it will log it and have the Mozilla installer throw up the Reboot dialog. seeking reviews now.
Attachment #114856 - Flags: superreview?(dveditz)
Attachment #114856 - Flags: review?(sgehani)
Comment on attachment 114856 [details] [diff] [review] patch v1.0 This patch doesn't address the question of *why* the GRE install thinks it needs a reboot. I assume it's because the GRE install is run before the Mozilla installer gets around to checking whether Mozilla is already running and shutting it down. *That* needs to get fixed; the install shouldn't require a reboot unless we're actually installing a system library, which we've taken pains not to do so far and should be extremely rare -- not every install. (To those who get this prompt, just ignore it. After the install finishes the xpicleanup util will replace all the DLL's correctly without having to reboot) >-$verPartial = "5.0.0."; >+$verPartial = "1.3.0."; > $ver = $verPartial . GetVersion($DEPTH); >-$verGre = "1.3b.0.0"; >+$verGre = $verParial . "0"; $verPartial ? .......^ >+ CleanupXpcomFile(); > if(NeedReboot()) > { >- CleanupXpcomFile(); >+ LogExitStatus("Reboot"); >+ if(sgProduct.mode == NORMAL) Tab issues? > /* Function to copy strings safely. > * returns the amount of memory required (including NULL byte) if there's not enough > * else, it returns 0 for success. > */ > int MozCopyStr(LPSTR szSrc, LPSTR szDest, DWORD dwDestBufSize) > { >- DWORD length = lstrlen(szSrc) + 1; >+ DWORD length; >+ >+ if(!szSrc || !szDest) >+ return(WIZ_ERROR_UNDEFINED); Is this such a good idea? some calling code will now think they need to recall this function with a 1024-byte buffer. On the other hand I can't find any places where the MozCopyStr return value is interpreted as a length and only 5 places that use it at all. I assume this was added because you found a case that passed a null. With this patch you won't know when that happens, but it's got to be an error. Would an assertion help? >+ /* clean up previous exis status log file */ "exit" >+BOOL GreInstallerNeedsReboot() >+{ >+ BOOL greReboot = FALSE; >+ >+ /* if this setup is not installing GRE *and* the GRE Setup has been run, then >+ * check for GRE setup's exit value, if one exists */ the comment confused me at first, because the Mozilla installer *does* install the GRE (unless it's up to date and not forced). Maybe if this is not the GRE install itself *and* the GRE Setup has been run, then ... sr=dveditz on this patch if you fix the above minor issues, but I'd rather fix the real problem which is the bogus need for a reboot.
Attachment #114856 - Flags: superreview?(dveditz) → superreview+
Attached patch patch v1.1 (deleted) — Splinter Review
> This patch doesn't address the question of *why* the GRE install thinks it > needs a reboot. I assume it's because the GRE install is run before the > Mozilla installer gets around to checking whether Mozilla is already > running and shutting it down. *That* needs to get fixed; the install > shouldn't require a reboot unless we're actually installing a system > library, which we've taken pains not to do so far and should be extremely > rare -- not every install. This patch definitely does not address that problem. Requiring a reboot right now is probably due to a combination of these bugs: bug 193173 - Installer needs to detect running apps using GRE bug 190816 - Running process causes Installer to hang until process is shut down via task manager bug 151532 - Delay system-tray icon until it is ready to accept mouse-clicks > $verPartial ? > .......^ typo fixed. > Tab issues? not really. I forgot to mention that the last patch was diffed with a '-w'. this one is not. > I assume this was added because you found a case that passed a null. With > this patch you won't know when that happens, but it's got to be an error. > Would an assertion help? yes, I did run into a case where it crashed. It was due to the order of how things are initialized as setup starts up. Code changed to use assertions instead. > "exit" typo fixed. > the comment confused me at first, because the Mozilla installer *does* > install the GRE (unless it's up to date and not forced). Maybe > > if this is not the GRE install itself *and* the GRE Setup has been run, then > ... changed.
Attachment #114856 - Attachment is obsolete: true
Comment on attachment 114856 [details] [diff] [review] patch v1.0 r=sgehani Caveat for review: Please file a bug to make WinSpawn() return the child process's exit code so we can simply detect the child installer reboot requirement. Thanks.
Attachment #114856 - Flags: review?(sgehani) → review+
Attachment #114872 - Flags: approval1.3?
Comment on attachment 114872 [details] [diff] [review] patch v1.1 a=asa (on behalf of drivers) for checkin to 1.3.
Attachment #114872 - Flags: approval1.3? → approval1.3+
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
20030225 and 20030227 mozilla nightlies. Verified - No reboot messages being generated directly from the GRE installer itself.
Status: RESOLVED → VERIFIED
Whiteboard: fixed1.3
*** Bug 195699 has been marked as a duplicate of this bug. ***
um? is the release fixed or do i have to download that .dif file... what do i do with the .dif after i download it?
I Really do need help on how to fix this bug. IE isnt saving my passwords correctly and it sucks and its slow :(
My browser is totally mest up and i cant downgrade and i never heard of a damn .dif file before...
This was checked in on the branch, so it should be fixed in the release. Follow up in bug 197503
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: