Full installer does not set default browser on Windows 7

VERIFIED FIXED in Firefox 55

Status

()

VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: agashlin, Assigned: mhowell)

Tracking

54 Branch
Firefox 55
Unspecified
Windows 7
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox55 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
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
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8855170 - Attachment is obsolete: true
Attachment #8855170 - Flags: review?(agashlin)
(Assignee)

Comment 3

2 years ago
The above patch wouldn't have fixed SetAsDefaultAppUser for previously existing installs, only newish ones. I'm redoing it.
Comment hidden (mozreview-request)
(Reporter)

Comment 5

2 years ago
mozreview-review
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.
(Assignee)

Comment 6

2 years ago
mozreview-review
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.
(Reporter)

Comment 7

2 years ago
mozreview-review
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.
(Assignee)

Comment 8

2 years ago
mozreview-review-reply
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 hidden (mozreview-request)
(Reporter)

Comment 10

2 years ago
mozreview-review
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+

Comment 11

2 years ago
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49abe31287b4
Fix making ourselves the default browser on Windows 7. r=agashlin

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/49abe31287b4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
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
status-firefox55: fixed → verified
You need to log in before you can comment on or make changes to this bug.