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)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: agracebush, Assigned: ssu0262)
References
Details
(Whiteboard: fixed1.3)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
asa
:
approval1.3+
|
Details | Diff | Splinter Review |
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. ***
Comment 2•22 years ago
|
||
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?
Comment 4•22 years ago
|
||
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.
Comment 5•22 years ago
|
||
*** Bug 193440 has been marked as a duplicate of this bug. ***
Comment 6•22 years ago
|
||
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
Comment 8•22 years ago
|
||
*** Bug 193503 has been marked as a duplicate of this bug. ***
Comment 9•22 years ago
|
||
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.
Assignee | ||
Comment 10•22 years ago
|
||
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 11•22 years ago
|
||
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+
Assignee | ||
Comment 12•22 years ago
|
||
> 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 13•22 years ago
|
||
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 14•22 years ago
|
||
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+
Assignee | ||
Comment 15•22 years ago
|
||
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 16•22 years ago
|
||
20030225 and 20030227 mozilla nightlies.
Verified - No reboot messages being generated directly from the GRE installer
itself.
Status: RESOLVED → VERIFIED
Updated•22 years ago
|
Whiteboard: fixed1.3
Comment 17•22 years ago
|
||
*** Bug 195699 has been marked as a duplicate of this bug. ***
Comment 18•22 years ago
|
||
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?
Comment 19•22 years ago
|
||
I Really do need help on how to fix this bug. IE isnt saving my passwords
correctly and it sucks and its slow :(
Comment 20•22 years ago
|
||
My browser is totally mest up and i cant downgrade and i never heard of a
damn .dif file before...
Comment 21•22 years ago
|
||
This was checked in on the branch, so it should be fixed in the release. Follow
up in bug 197503
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•