Closed Bug 1343938 Opened 7 years ago Closed 7 years ago

Firefox doesn't prompt to become default browser (for first profile created after installing)

Categories

(Firefox :: Installer, defect)

defect
Not set
blocker

Tracking

()

VERIFIED FIXED
Firefox 54
Tracking Status
firefox52 + verified
firefox-esr52 --- verified
firefox53 --- verified
firefox54 --- verified
firefox55 --- verified

People

(Reporter: aryx, Assigned: molly)

References

Details

Attachments

(1 file)

Latest 54.0a1 to 51.0b1, likely also before. Windows 8.1 64-bit

The first profile created after installing Firefox doesn't prompt for setting it as the default browser.

Steps to reproduce (Nightly and Developer Edition will prompt on second launch, browser on first, release likely similar, but I haven't tested):
1. Installed Nightly into the old custom install directory (but any custom install directory should work).
2. Launched it from the console with firefox.exe -no-remote -P
3. Create a new profile.
4. Launch the profile.
5. Close the profile.
6. Launch the profile.
7. Close the profile.
Actual result: Not prompted to set it as default browser.
8. Launched it from the console with firefox.exe -no-remote -P
9. Create a new profile.
10. Launch the profile.
6. Launch the profile.
7. Close the profile.
Actual & Expected result: Prompted to set it as default browser.

The first profile which won't show the prompt has browser.shell.checkDefaultBrowser automatically set to the custom value false.

This gets sets at https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/browser/components/shell/ShellService.jsm#52-74 because DefaultBrowserOptOut in the registry is True and gets deleted, so it won't get set for profiles created later.

The code for setting the registry key is at https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/browser/installer/windows/nsis/installer.nsi#1025
Thanks for the thorough investigation. It looks like the issue starts when, on Windows versions newer than 7, we skip displaying the default browser checkbox in the installer [0]. We do that because the installer isn't able to automatically change the default browser setting on those Windows versions. Then, later on in the installation [1] we try to read the value of that checkbox, and interpret the fact that it wasn't checked to mean that the user explicitly told us they didn't want us to be the default browser (because the box is checked by default). But of course, since we didn't show the checkbox at all, we just haven't asked them yet.

The fix will be to skip writing the offending registry key when the checkbox was never shown, so that we don't record an intent that the user did not express.

However... there is still a pref called "browser.shell.skipDefaultBrowserCheckOnFirstRun", and it defaults to true, so you still won't actually see the prompt immediately.

By the way, it doesn't seem like you have to use a custom install location to trigger this bug; using the default path gets the same result for me, and the relevant code paths don't appear to care one way or the other.

[0] https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/browser/installer/windows/nsis/installer.nsi#1044
[1] https://dxr.mozilla.org/mozilla-central/rev/e91de6fb2b3dce9c932428265b0fdb630ea470d7/browser/installer/windows/nsis/installer.nsi#595
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Component: Shell Integration → Installer
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

https://reviewboard.mozilla.org/r/116696/#review118514

Looks good, one issue mostly for consistency:

::: browser/installer/windows/nsis/installer.nsi:559
(Diff revision 1)
>    SetDetailsPrint both
>    DetailPrint "$(STATUS_CLEANUP)"
>    SetDetailsPrint none
>  
>    ${Unless} ${Silent}
>      ${MUI_INSTALLOPTIONS_READ} $0 "summary.ini" "Field 4" "State"

There should be a ClearErrors before this to tell if MUI_INSTALLOPTIONS_READ set the error.
Attachment #8842971 - Flags: review?(agashlin) → review-
As usual I mispredicted what would be quoted, I meant to suggest ClearErrors directly before MUI_INSTALLOPTIONS_READ.
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

https://reviewboard.mozilla.org/r/116696/#review118514

> There should be a ClearErrors before this to tell if MUI_INSTALLOPTIONS_READ set the error.

Good catch; revision incoming.
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

https://reviewboard.mozilla.org/r/116696/#review118522
Attachment #8842971 - Flags: review?(agashlin) → review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/09600e445256
Fix new installs erroneously not prompting to change the default browser setting. r=agashlin
https://hg.mozilla.org/mozilla-central/rev/09600e445256
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Verified fixed with 54.0a1 20170304.
Status: RESOLVED → VERIFIED
[Tracking Requested - why for this release]:
Users on Windows 8, 8.1 or 10 who download and install Firefox won't be prompted to set it as default browser which should result in a decreased retention rate.

This is a regression from bug 1041514 and started on 2016-02-24 with 47.0a1.

This should get a test case for manual testing.

Post-mortem questions:
1) Why wasn't this detected by QA? Does it only use Windows 7, updated installations for testing or throw away the first profile which gets created?
2) Was there a change in retention rate between 46 and 47? If yes, had this been investigated? If not, why is the prompt ineffective?
3) How can the behavior be kept consistent for similar (e.g. new) profiles? This is a regression by code which changes a registry value.
Blocks: 1041514
Severity: major → blocker
Summary: First profile created after installing Firefox in custom location doesn't prompt for setting it as default browser → Firefox doesn't prompt to become default browser (for first profile created after installing)
If I understand correctly, this isn't a new regression as 51.0b1 was impacted.
I agree this is really critical but it doesn't seem critical enough to delay the release but we should maybe take it in potential dot release.

Matt, wdyt?
Could you anyway fill the uplift requests to aurora & beta?
Thanks
Flags: needinfo?(mhowell)
Aryx did find the right patch for this regression; it's from 47, it's not terribly new.

I agree with not delaying releases over this. I would support taking it in a point release.

Uplift request comment coming up.
Flags: needinfo?(mhowell)
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1041514

[User impact if declined]:
Users on Windows 8, 8.1 or 10 who download and install Firefox won't be prompted to set it as default browser which should result in a decreased retention rate.
(from comment 11)

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
Probably. Already been manually verified on Nightly, but nowhere else. See STR in comment 0.

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
It's a very very narrow change.

[String changes made/needed]:
None
Attachment #8842971 - Flags: approval-mozilla-beta?
Attachment #8842971 - Flags: approval-mozilla-aurora?
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

Taking it to aurora before the merge of tomorrow (Monday)

Setting the uplift request to release too for a potential dot release.
Attachment #8842971 - Flags: approval-mozilla-release?
Attachment #8842971 - Flags: approval-mozilla-aurora?
Attachment #8842971 - Flags: approval-mozilla-aurora+
Attachment #8842971 - Flags: approval-mozilla-esr52?
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

Too late for beta52, will consider for a dot release instead.
Attachment #8842971 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 8842971 [details]
Bug 1343938 - Fix new installs erroneously not prompting to change the default browser setting.

fix windows installer to prompt about default browser, let's take this on release and esr52.
Attachment #8842971 - Flags: approval-mozilla-release?
Attachment #8842971 - Flags: approval-mozilla-release+
Attachment #8842971 - Flags: approval-mozilla-esr52?
Attachment #8842971 - Flags: approval-mozilla-esr52+
https://hg.mozilla.org/releases/mozilla-esr52/rev/7a9fff28b52d (FIREFOX_ESR_52_0_X_RELBRANCH - 52.0.2)
Flags: qe-verify+
We managed to verify this issue on Firefox 52.0.2esr, Firefox 52.0.2, Firefox 53.0b6, Firefox 54.0a2 (2017-03-27), Firefox 55.0a1 (2017-03-26) builds, under Ubuntu 16.04x64, Windows 7x64, Windows 10x64 and under Mac OS X 10.12.
On ESR, Beta and Release builds the Pop-up for setting Firefox as default browser is displayed on the first run and for Nightly and Aurora builds, the pop-up is displayed only from the second run.
Matt, can you please confirm that this is the expected behavior?
Flags: needinfo?(mhowell)
Yes, that is the expected behavior; there is a pref which specifically prevents showing the prompt on the first startup when it is set, and it's set by default on nightly and aurora, but cleared for beta and release/ESR. See https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/browser/app/profile/firefox.js#212
Flags: needinfo?(mhowell)
Thanks Matt. 
Based on Comment 22 and Comment 23, I'm changing the flags to 'Verified' for all the builds on which the tested was performed.
You need to log in before you can comment on or make changes to this bug.