Closed Bug 1353955 Opened 6 years ago Closed 6 years ago

Full installer does not set default browser on Windows 7

Categories

(Firefox :: Installer, defect)

54 Branch
Unspecified
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- verified

People

(Reporter: agashlin, Assigned: mhowell)

Details

Attachments

(1 file, 1 obsolete file)

With a currently nightly full installer (20170404030204)
https://archive.mozilla.org/pub/firefox/nightly/2017/04/2017-04-04-03-02-04-mozilla-central/firefox-55.0a1.en-US.win32.installer.exe

0. Have some other browser (say IE) as default
1. Run the installer with all default options
2. Check the default browser, Nightly has not become the default (I have been checking in IE's Internet Options Programs tab, as IE had previously been the default browser, it still claims IE is the default)

The regression range seems to be between 2017-02-01 (20170201030207) and 2017-02-02 (20170202030211), so I suspect bug 1324617 may be involved. Aurora is also affected.

---

It seems that at the same time another issue appears, when running the uninstaller from an non-admin command line as
C:\Program Files\Nightly\uninstall\helper.exe /SetAsDefaultAppUser
it will pop up a UAC prompt. This didn't happen before 2017-02-02.

I think this is causing a UAC prompt for helper.exe towards the end of a stub install in current nightlies and aurora (on Win 7). I can file another bug for that if it seems unrelated.

---

I'll also note here that manually running helper /SetAsDefaultAppUser to set nightly as default stopped working between 2017-03-02 and 2017-03-03, though it still attempts to elevate with the UAC prompt, possibly related to the fix for bug 1340568 given the timing. The stub installer is still able to set default, though, and as far as I know it uses this mechanism, so I don't know exactly what is going on here.
We're getting into a really fun state where we get all the file and protocol associations we want, but we don't think we're the default browser and IE does. That points to the SetAppAsDefaultAll call being messed up. Which, on further inspection, it definitely is messed up. Patch incoming.

(In reply to Adam Gashlin [:agashlin] from comment #0)
> It seems that at the same time another issue appears, when running the
> uninstaller from an non-admin command line as
> C:\Program Files\Nightly\uninstall\helper.exe /SetAsDefaultAppUser
> it will pop up a UAC prompt. This didn't happen before 2017-02-02.
> 
> I think this is causing a UAC prompt for helper.exe towards the end of a
> stub install in current nightlies and aurora (on Win 7). I can file another
> bug for that if it seems unrelated.

I don't see how this could be related. I'll investigate tomorrow.
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Attachment #8855170 - Attachment is obsolete: true
Attachment #8855170 - Flags: review?(agashlin)
The above patch wouldn't have fixed SetAsDefaultAppUser for previously existing installs, only newish ones. I'm redoing it.
Comment on attachment 8855561 [details]
Bug 1353955 - Fix making ourselves the default browser on Windows 7.

https://reviewboard.mozilla.org/r/127410/#review131758

::: browser/installer/windows/nsis/shared.nsh:1251
(Diff revision 1)
>  ; under HKEY_CURRENT_USER via registry calls and using the AppAssocReg NSIS
>  ; plugin. This is a function instead of a macro so it is
>  ; easily called from an elevated instance of the binary. Since this can be
>  ; called by an elevated instance logging is not performed in this function.
>  Function SetAsDefaultAppUserHKCU
> +  ; See if we're using path hash suffixed registry keys for this install

I've been having trouble testing this with a non-hashed path.

::: browser/installer/windows/nsis/shared.nsh:1259
(Diff revision 1)
> +  ClearErrors
> +  ReadRegStr $0 HKCU "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""
> +  ${If} ${Errors}
> +  ${OrIf} ${AtMostWin2008R2}
> +    ClearErrors
> +    ReadRegStr $0 HKLM "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""

If we get here $0 may already be "" due to the ReadRegStr above failing.
Comment on attachment 8855561 [details]
Bug 1353955 - Fix making ourselves the default browser on Windows 7.

https://reviewboard.mozilla.org/r/127410/#review132060

::: browser/installer/windows/nsis/shared.nsh:1251
(Diff revision 1)
>  ; under HKEY_CURRENT_USER via registry calls and using the AppAssocReg NSIS
>  ; plugin. This is a function instead of a macro so it is
>  ; easily called from an elevated instance of the binary. Since this can be
>  ; called by an elevated instance logging is not performed in this function.
>  Function SetAsDefaultAppUserHKCU
> +  ; See if we're using path hash suffixed registry keys for this install

I've been doing that by running a later channel installer, then dropping a patched helper.exe into that install.

You can also just manually rename the keys in regedit.

::: browser/installer/windows/nsis/shared.nsh:1259
(Diff revision 1)
> +  ClearErrors
> +  ReadRegStr $0 HKCU "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""
> +  ${If} ${Errors}
> +  ${OrIf} ${AtMostWin2008R2}
> +    ClearErrors
> +    ReadRegStr $0 HKLM "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""

That's intentional; if $0 is empty than $0 == $8 is guaranteed to fail.
Comment on attachment 8855561 [details]
Bug 1353955 - Fix making ourselves the default browser on Windows 7.

https://reviewboard.mozilla.org/r/127410/#review132110

::: browser/installer/windows/nsis/shared.nsh:1259
(Diff revision 1)
> +  ClearErrors
> +  ReadRegStr $0 HKCU "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""
> +  ${If} ${Errors}
> +  ${OrIf} ${AtMostWin2008R2}
> +    ClearErrors
> +    ReadRegStr $0 HKLM "Software\Clients\StartMenuInternet\$0\DefaultIcon" ""

I'm referring to the $0 with uppercase FileMainEXE used in creating the registry key. After line 1255 $0 holds either the icon path or "", but 1259 is using $0 again to build the HKLM registry key.
Comment on attachment 8855561 [details]
Bug 1353955 - Fix making ourselves the default browser on Windows 7.

https://reviewboard.mozilla.org/r/127410/#review132110

> I'm referring to the $0 with uppercase FileMainEXE used in creating the registry key. After line 1255 $0 holds either the icon path or "", but 1259 is using $0 again to build the HKLM registry key.

Oooooh, right. Yep, that's broken.
Comment on attachment 8855561 [details]
Bug 1353955 - Fix making ourselves the default browser on Windows 7.

https://reviewboard.mozilla.org/r/127410/#review132204
Attachment #8855561 - Flags: review?(agashlin) → review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49abe31287b4
Fix making ourselves the default browser on Windows 7. r=agashlin
https://hg.mozilla.org/mozilla-central/rev/49abe31287b4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Flags: qe-verify+
Verified fixed on Windows 7 x64 using Firefox 55 Beta 3 (buildID: 20170619071723), win64 and win32.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.