1336243 - Launcher after install fails to start Firefox Nightly

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

After using the .exe file for an install of the browser when you get to the last panel that shows 'Finish' there is a check-box pre-checked to "Launch Firefox"

Nightly builds are failing to launch. It will start from the Icon on the Desktop.

Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
win10 x64 

cset 1d025ac534a6333a8170a59a95a8a3673d4028ee good
cset ae91b2b5bb69ffd18b4679188730d0c6af3b4a95 bad

regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1d025ac534a6333a8170a59a95a8a3673d4028ee&tochange=ae91b2b5bb69ffd18b4679188730d0c6af3b4a95

I guess I don't know how to test the Launch failure with Mozregressioin

Comment 1

[Molly Howell (she/her) (mostly inactive)]

Thanks for the report, and for getting the regression range. My bug 1324617 patch is the one that caused this; I broke the method used for finding the right executable to launch. It's a simple fix, so I'll have a patch up shortly.

Comment 2

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1336243 - Fix a regression from bug 1324617 in launching the browser post-install.

Bug 1324617 changed the name of the registry path that was being used to find
the new installation to launch, but forgot to update that function. This patch
switches to a more direct method of getting the right path to launch.

Review commit: https://reviewboard.mozilla.org/r/109336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109336/

Comment 3

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

Matt, this would probably be a good patch to have agashlin review. Sound good?

Comment 4

[Molly Howell (she/her) (mostly inactive)]

Totally agree, been having similar thoughts myself. Switching request.

Comment 5

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8833115 [details]
Bug 1336243 - Fix a regression from bug 1324617 in launching the browser post-install.

https://reviewboard.mozilla.org/r/109336/#review110482

::: browser/installer/windows/nsis/installer.nsi:842
(Diff revision 1)
>      UAC::ExecCodeSegment $0
>    ${EndIf}
>  FunctionEnd
>  
>  Function LaunchAppFromElevatedProcess
> -  ; Find the installation directory when launching using GetFunctionAddress
> +  Pop $0

It looks like when the original code was written (no later than September 2007 [1]), variables were not copied between the elevated and unelevated installers on UAC::ExecCodeSegment.

History.txt [2] suggests this was added in November 2007:

> v0.0.6d - 20071108 (AndersK)
> +Now syncs basic registers/variables before calling UAC::*Exec* and UAC::ExecCodeSegment (r0-r9,R0-R9,$CMDLINE,$INSTDIR,$OUTDIR,$EXEDIR,$LANGUAGE)

Since we do have that support now [3], wouldn't it make sense to use $INSTDIR directly instead of the UAC::StackPush and Pop?
I did a quick test and it seems to work as expected.

[1] https://hg.mozilla.org/mozilla-central/rev/3305baa9f049
[2] http://nsis.sourceforge.net/mediawiki/images/8/8f/UAC.zip
[3] The current UAC.dll is from bug 571387, in 2010

Comment 6

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

This is my first review request, I think may have gone too deep into it. If you don't think this is worth changing I'll drop that issue.

Comment 7

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8833115 [details]
Bug 1336243 - Fix a regression from bug 1324617 in launching the browser post-install.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109336/diff/1-2/

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 8833115 [details]
Bug 1336243 - Fix a regression from bug 1324617 in launching the browser post-install.

https://reviewboard.mozilla.org/r/109336/#review110732

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/autoland/rev/e6d624b8326b6a9152f30a203bb41c3c66ac0d52

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e6d624b8326b

Comment 11

[Jim Jeffery not reading bug-mail 1/2/11]

Verified FIXED today's Nightly 
cset: https://hg.mozilla.org/mozilla-central/rev/3e555770a90a41e04bbb4ac41b65fa2f1db6977d 

Win10 x64 - win32 builds.