Closed Bug 1337530 Opened 9 years ago Closed 9 years ago

Default browser detection doesn't work for new installs since bug 1324617

Categories

(Firefox :: Installer, defect)

Product:
Firefox
Component:
Installer
Platform:
Unspecified
Windows
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: molly, Assigned: molly)

References

Details

Attachments

(1 file)

Bug 1337530 - Fix Windows default browser detection.
59 bytes, text/x-review-board-request
robert.strong.bugs
: review+
Details
In bug 1324617 I broke the check for whether the browser is currently set as the default on Windows. This is visible in the general preferences, which always claims that we aren't the default browser and offers to change the setting, even if we actually are the default browser. This only applies to new installs since bug 1324617 landed; installations updated to that version are not affected. And it won't be necessary to reinstall to pick up the fix for this regression.
Comment on attachment 8834972 [details] Bug 1337530 - Fix Windows default browser detection. https://reviewboard.mozilla.org/r/110708/#review112126 Talked over removing the additional checks that were kept for Vista and above since not all apps used IApplicationAssociationRegistration when it was first introduced. Clearing review.
Attachment #8834972 - Flags: review?(robert.strong.bugs)
Somewhat unrelated to this bug. I tested things a little bit and found multiple entries for the same install location Examples: HKEY_CURRENT_USER\SOFTWARE\Clients\StartMenuInternet\Firefox-4FD8BA6A4A33F689\shell\open\command Default = "c:\Program Files\Nightly\firefox.exe" HKEY_CURRENT_USER\SOFTWARE\Clients\StartMenuInternet\Firefox-A3710B8EBB50CD3\shell\open\command Default = "C:\Program Files\Nightly\firefox.exe" Prior to these appearing I deleted all entries keys under HKEY_CURRENT_USER\SOFTWARE\Clients\StartMenuInternet I also have both entries under HKEY_CURRENT_USER\SOFTWARE\RegisteredApplications Firefox-4FD8BA6A4A33F689 = Software\Clients\StartMenuInternet\Firefox-4FD8BA6A4A33F689\Capabilities Firefox-A3710B8EBB50CD3 = Software\Clients\StartMenuInternet\Firefox-A3710B8EBB50CD3\Capabilities Of these two I only have HKEY_LOCAL_MACHINE\SOFTWARE\RegisteredApplications Firefox-4FD8BA6A4A33F689 = Software\Clients\StartMenuInternet\Firefox-4FD8BA6A4A33F689\Capabilities and HKEY_LOCAL_MACHINE\SOFTWARE\Clients\StartMenuInternet\Firefox-4FD8BA6A4A33F689 I first noticed this when trying to set as default and having two entries for Nightly and Firefox said it was default no matter which one I selected.
I haven't seen that. I assume the second entry got created by whatever decided to use a lowercase drive letter, but I don't know what that would have been.
I did what I usually do when I test these sort of changes which is install using the installer and then run the following from a command prompt as admin. cd into the installation's uninstall directory. helper.exe /PostUpdate helper.exe /SetAsDefaultAppGlobal helper.exe /SetAsDefaultAppUser I'll try to reproduce again in a few minutes.
That STR doesn't reproduce it for me, I still just get one set of entries in all those places.
It reproduced twice last night but I might not have your last installer patch in the build I tested with so I updated my repo and am building again.
(In reply to Matt Howell [:mhowell] from comment #0) > This only applies to new installs since bug 1324617 landed; installations > updated to that version are not affected. And it won't be necessary to > reinstall to pick up the fix for this regression. I see this "Nightly is not your default browser" problem in Nightly (on Windows 10 Insider Preview channel), even though my Nightly installation is not a new install.
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #8) > It reproduced twice last night but I might not have your last installer > patch in the build I tested with so I updated my repo and am building again. and now it no longer reproduces after rebuilding.
Comment on attachment 8834972 [details] Bug 1337530 - Fix Windows default browser detection. https://reviewboard.mozilla.org/r/110708/#review112642 ::: browser/components/shell/nsWindowsShellService.cpp:178 (Diff revision 2) > // FileAssociations .xhtml REG_SZ FirefoxHTML-<PathHash> > // StartMenu StartMenuInternet REG_SZ <appname>-<PathHash> > // URLAssociations ftp REG_SZ FirefoxURL-<PathHash> > // URLAssociations http REG_SZ FirefoxURL-<PathHash> > // URLAssociations https REG_SZ FirefoxURL-<PathHash> > Remove the entire 'Default Browser Registry Settings' section above since the registry checks are removed and only IApplicationAssociationRegistration is used. ::: browser/components/shell/nsWindowsShellService.cpp:327 (Diff revision 2) > do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv); > NS_ENSURE_SUCCESS(rv, rv); > > nsCOMPtr<nsIFile> exeFile; > rv = dirSvc->Get(XRE_EXECUTABLE_FILE, > - NS_GET_IID(nsIFile), > + NS_GET_IID(nsIFile), nit: why not keep the existing indentation so it is consitent in the file? ::: browser/components/shell/nsWindowsShellService.cpp:341 (Diff revision 2) > - uint64_t hash = CityHash64(static_cast<const char *>(path.get()), > - path.Length() * sizeof(nsAutoString::char_type)); > - > aAppRegName = APP_REG_NAME_BASE; > - aAppRegName.AppendInt((int)hash, 16); > + uint64_t hash = CityHash64(static_cast<const char *>(appDirStr.get()), > + appDirStr.Length() * sizeof(nsAutoString::char_type)); nit: why not keep the existing indentation so it is consitent in the file? ::: browser/components/shell/nsWindowsShellService.cpp:367 (Diff revision 2) > - if (NS_FAILED(rv)) { > - *aIsDefaultBrowser = false; > - return NS_OK; > + return NS_OK; > - } > + } > > - ::ZeroMemory(currValue, sizeof(currValue)); > + *aIsDefaultBrowser = IsAARDefault(pAAR, L"http"); Previously, if aCheckAllTypes is true then pAAR->QueryAppIsDefaultAll was called on all versions of Windows. Maybe IsDefaultBrowserVista made you think it was only for Vista. It looks like the patch in bug 796038 made it so anything later than Win8 would override that result though which is weird. I'm ok with this and will followup later in another bug if needed.
Attachment #8834972 - Flags: review?(robert.strong.bugs)
r+ with the comments addressed of course.
(In reply to Chris Peterson [:cpeterson] from comment #9) > I see this "Nightly is not your default browser" problem in Nightly (on > Windows 10 Insider Preview channel), even though my Nightly installation is > not a new install. I had forgotten that some of the registry entries that get created on new installs are also updated in a few other places. This patch should fix the default browser detection in that case as well. But I need to be more thorough about those code paths handling existing installs. I'll file a new bug to address those.
Comment on attachment 8834972 [details] Bug 1337530 - Fix Windows default browser detection. https://reviewboard.mozilla.org/r/110708/#review112642 > nit: why not keep the existing indentation so it is consitent in the file? Visual Studio decides to do this sometimes and I don't always notice. It wasn't intentional. I'll put it back.
Pushed by mhowell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e4c6ea682f56 Fix Windows default browser detection. r=rstrong
https://hg.mozilla.org/mozilla-central/rev/e4c6ea682f56
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Depends on: 1338843
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: