Last Comment Bug 760680 - Update shell integration default browser check for Windows 8
: Update shell integration default browser check for Windows 8
Product: Firefox
Classification: Client Software
Component: Shell Integration (show other bugs)
: Trunk
: x86_64 Windows 8
: -- normal (vote)
: Firefox 16
Assigned To: Brian R. Bondy [:bbondy]
: Robert Strong [:rstrong] (use needinfo to contact me)
: 740424 760679 (view as bug list)
Depends on: 764515
Blocks: 831612 760023
  Show dependency treegraph
Reported: 2012-06-01 14:39 PDT by Jim Mathies [:jimm]
Modified: 2013-02-22 16:38 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1. (3.35 KB, patch)
2012-06-05 13:46 PDT, Brian R. Bondy [:bbondy]
jmathies: review-
Details | Diff | Splinter Review
Patch v2 (4.28 KB, patch)
2012-06-06 09:44 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Splinter Review

Description Jim Mathies [:jimm] 2012-06-01 14:39:07 PDT
Our current methods do not detect this properly. The shell code will need to be updated.
Comment 1 Brian R. Bondy [:bbondy] 2012-06-05 07:27:03 PDT
I think the way to fix this is to check the ProgId for the default protocol in nsWindowsShellService::IsDefaultBrowserVista via IApplicationAssociationRegistration::QueryCurrentDefault
Comment 2 Brian R. Bondy [:bbondy] 2012-06-05 13:46:03 PDT
Created attachment 630310 [details] [diff] [review]
Patch v1.

This review request is to land directly on m-c.
Registering http implies https by the way. 

It pops up the same select browser window as when you install.  The pref UI doesn't change at all. If you don't select Firefox in the popup it will not update the pref UI saying you set the default (I.e. it works correctly).
Comment 3 Jim Mathies [:jimm] 2012-06-06 04:11:25 PDT
Comment on attachment 630310 [details] [diff] [review]
Patch v1.

Review of attachment 630310 [details] [diff] [review]:

::: browser/components/shell/src/nsWindowsShellService.cpp
@@ +582,5 @@
> +    info.pcszClass = NULL;
> +    info.oaifInFlags = OAIF_FORCE_REGISTRATION | 
> +                       OAIF_URL_PROTOCOL |
> +                       OAIF_REGISTER_EXT;
> +    if FAILED(SHOpenWithDialog(NULL, &info)) {

This api is Vista and up, we'll need to use GetProcAddress to avoid xp problems. :/

With a NULL parent window handle, are there any focus issues with the main fx desktop window?
Comment 4 Brian R. Bondy [:bbondy] 2012-06-06 09:44:28 PDT
Created attachment 630601 [details] [diff] [review]
Patch v2

Fixed the dynamic load.

We don't need to specify an HWND because the defaults dialog is dismissed automatically when focus is lost anyway.  The call is blocking though in that the next line of code will not execute until the window loses focus or a selection is made.

I haven't pushed to try yet by the way but will before landing.
Comment 5 Jim Mathies [:jimm] 2012-06-06 10:32:14 PDT
Comment on attachment 630601 [details] [diff] [review]
Patch v2

Hrm, modal dialog calls are best avoided, but if the call throws up the right looking win8 interface I suppose it's worth it the risk.
Comment 6 Brian R. Bondy [:bbondy] 2012-06-08 06:21:12 PDT
Comment 7 Ed Morley [:emorley] 2012-06-08 13:56:00 PDT
Comment 8 Brian R. Bondy [:bbondy] 2012-06-11 06:05:04 PDT
*** Bug 760679 has been marked as a duplicate of this bug. ***
Comment 9 Brian R. Bondy [:bbondy] 2012-06-13 13:27:21 PDT
*** Bug 740424 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.