447934 - Default browser check doesn't return false if the registry key doesn't exist

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect)

Description

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

Found while fixing bug 404609 for Thunderbird's installer and OS Integration.

If a registry key doesn't exist the check continues on to the next key instead of setting *aIsDefaultBrowser to PR_FALSE

http://mxr.mozilla.org/mozilla/source/browser/components/shell/src/nsWindowsShellService.cpp#318

Comment 1

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

Attachment: patch rev1

There is also some cleanup in this patch

Comment 2

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

Comment on attachment 331276 [details] [diff] [review]
patch rev1

>Index: browser/components/shell/src/nsWindowsShellService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v
>retrieving revision 1.56
>diff -u -8 -r1.56 nsWindowsShellService.cpp
>--- browser/components/shell/src/nsWindowsShellService.cpp	25 Apr 2008 16:44:16 -0000	1.56
>+++ browser/components/shell/src/nsWindowsShellService.cpp	25 Jul 2008 06:46:04 -0000
>...
>+    if (NS_FAILED(rv)) {
>+      *aIsDefaultBrowser = PR_FALSE;
>+      break;
>+    }
Instead of a "break" this should be "return NS_OK;" to be consistent with the rest of this file. I've fixed this in my tree.

Comment 3

[Jim Mathies [:jimm]]

> rv = OpenKeyForReading(HKEY_CLASSES_ROOT, key, &theKey);

On Vista, do we set the protocol handlers in root manually or are we using  IApplicationAssociationRegistration's SetAppAsDefault? I'm curious because we check for the root keys first before falling back to IsDefaultBrowserVista. The logic here seems a little strange. If we're using SetAppAsDefault, there would be no reason to look for these keys.

Comment 4

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

We use SetAppAsDefault in the helper.exe and verify we are default in this code. Those keys can change in the case of a second install so we need to verify they point to |this| installation. We also need the workaround for Bug 369703 but that is secondary.

Comment 5

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

Checked in to mozilla-central

http://hg.mozilla.org/mozilla-central/index.cgi/rev/e77a3c2920bca752fabb0af8f7cca19fa9c9410a

changeset:   16387:e77a3c2920bc
tag:         tip
user:        Robert Strong <[e-mail]>
date:        Mon Aug 04 23:13:44 2008 -0700

Comment 6

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

Lost comment #2 from my tree :(

http://hg.mozilla.org/mozilla-central/index.cgi/rev/3bb3190f6148bf3a820c877e7a97c88ec96da16f

changeset:   16388:3bb3190f6148
tag:         tip
user:        Robert Strong <[e-mail]>
date:        Mon Aug 04 23:48:06 2008 -0700

Comment 7

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

Attachment: patch including change from comment #2

Comment 8

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

Comment on attachment 332313 [details] [diff] [review]
patch including change from comment #2

Drivers, this fixes a case where the OS Integration registry key doesn't exist and we don't tell the user that Firefox is NOT the default browser.

Comment 9

[Samuel Sidler (old account; do not CC)]

Comment on attachment 332313 [details] [diff] [review]
patch including change from comment #2

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 10

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

Checked in for Firefox 3.0.2

Checking in mozilla/browser/components/shell/src/nsWindowsShellService.cpp;
/cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v  <--
nsWindowsShellService.cpp
new revision: 1.57; previous revision: 1.56
done
Checking in mozilla/browser/components/shell/src/nsWindowsShellService.h;
/cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.h,v  <--  ns
WindowsShellService.h
new revision: 1.16; previous revision: 1.15
done