1167294 - Windows 10 blocks the HTTP handler/default browser application dialog

Status: VERIFIED FIXED

(Firefox :: Shell Integration, defect, P1)

Description

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: default browser.png

Trying to set Nightly as the default browser in the newest Windows 10 build (10122) results in the screen in the attachment.

Evidently MS has changed the way default apps are handled. There's more information in this article: https://blogs.windows.com/bloggingwindows/2015/05/20/announcing-windows-10-insider-preview-build-10122-for-pcs/

We need to at least get to a state where the user can set the default browser in place, without having to manually go to system settings.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Tracking as we want to see that fixed in 40.

Comment 2

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: default choser.png

To clarify here: we need to find a way to directly open the dialog shown in this attachment after clicking the »Make Firefox the default« button.

Comment 3

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Related question: Is there a way for Firefox to know whether it was the default browser prior to the update to Windows 10? If so, we could be able to be more specific with our messaging...

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Here are some videos that show the current upgrade experience from Windows 8 to Windows 10 (with a focus on the state of the default browser):
http://people.mozilla.org/~mverdi/projects/windows10/fx-user-win10-update.m4v
http://people.mozilla.org/~mverdi/projects/windows10/ie-user-win10-update.m4v

Comment 6

[Masatoshi Kimura [:emk]]

(In reply to Philipp Sackl [:phlsa] please use needinfo from comment #2)
> To clarify here: we need to find a way to directly open the dialog shown in
> this attachment after clicking the »Make Firefox the default« button.

According to the blog post, it is no longer possible for apps to invoke the dialog without actually opening a file or a URL.

Comment 7

[Jim Mathies [:jimm]]

After the initial prompt once you open Fx and you click set firefox as the default in our initial dialog, did we register any default handlers? In 8/8.1 we were able to take defaults for protocol handlers, then the user had to go into the control panel through options to take file handlers. From what I can tell, not much has changed here unless we're not getting protocol handlers after confirming the first prompt.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

I'm noticing some differences between video 1 in comment #5 and my local VM of Windows 10.

Comment #5 is using build 10122.
I have build 10074 installed.

In build 10074, clicking "Use Firefox as my Default Browser" shows http://screencast.com/t/Th0FQ8njGN3V. I was able to reproduce the different behavior between the Default Browser dialog upon startup and the "Make Default Browser" button in about:preferences.

I'm currently updating to build 10130 to see what happens there.

Comment 9

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Yeah, the behavior was introduced with 10122 and persists in 10130. Given that MS blogged about it, I assume that they won't change it from here...

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch (untested)

In Chrome Dev, they are now launching in to the Windows 'settings' modern app with the 'default apps' view focused. You can see their change at https://chromium.googlesource.com/chromium/src/+/8707fef05b454ec373cb05d8a9f815780281ad2b%5E!/#F0

This appears to be the best that we can do on Windows 10.

Unfortunately, I am having trouble building this locally since I don't think the correct version of shlobjidl.h is getting included (undeclared identifier for IID_IApplicationActivationManager). This was introduced in the Windows 8.1 SDK (https://msdn.microsoft.com/en-us/library/windows/desktop/hh706903%28v=vs.85%29.aspx).

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 8613881 [details] [diff] [review]
Patch (untested)

Review of attachment 8613881 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/shell/nsWindowsShellService.cpp
@@ +39,5 @@
>  #endif
>  #define _WIN32_WINNT 0x0600
>  #define INITGUID
>  #include <shlobj.h>
> +#include <shobjidl.h>

_WIN32_WINNT is bumped right above here, this should provide access to these apis. Check the headers.

@@ +679,5 @@
> +           L"windows.immersivecontrolpanel_cw5n1h2txyewy"
> +           L"!microsoft.windows.immersivecontrolpanel",
> +           L"page=SettingsPageAppsDefaults", AO_NONE, &pid);
> +    pActivator->Release();
> +    return SUCCEEDED(hr);

this method returns nsresult.

Comment 12

[Masatoshi Kimura [:emk]]

Comment on attachment 8613881 [details] [diff] [review]
Patch (untested)

The Settings App window immediately disappears for me. I couldn't change the default app before the window vanishes.

Comment 13

[Masatoshi Kimura [:emk]]

I manually launched Settings App and selected the default browser, but it didn't change the default browser! Microsoft blocked themselves to change the default browser. I think we should just launch the classic Default Apps control panel (it still works). In other words, let's always call LaunchControlPanelDefaultPrograms() on Windows 10.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Tested this patch out and it works. I also fixed the inconsistency between the startup "default browser" check and the one that is accessed through the preferences.

shlobj.h includes shobjidl.h already, so I just placed the #undef/#define ahead of the include for shlobj.h.

Comment 15

[Jim Mathies [:jimm]]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Review of attachment 8614308 [details] [diff] [review]:
-----------------------------------------------------------------

I did not test this on win10.

::: browser/components/shell/nsWindowsShellService.cpp
@@ +675,5 @@
> +                                IID_IApplicationActivationManager,
> +                                (void**)&pActivator);
> +
> +  if (SUCCEEDED(hr)) {
> +    DWORD pid = 0;

nit - no need for the assignment.

Comment 16

[:Gijs (he/him)]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Review of attachment 8614308 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/nsBrowserGlue.js
@@ +2623,5 @@
>    },
>  
>    setAsDefault: function() {
>      let claimAllTypes = true;
>  #ifdef XP_WIN

Any reason to not use AppConstants here, too, while we're here?

::: browser/components/shell/nsWindowsShellService.cpp
@@ +717,2 @@
>      if (aClaimAllTypes) {
>        rv = LaunchControlPanelDefaultPrograms();

Sigh. It's kind of sad how we have version checks both in the JS that calls this, and then in the code here as well, and it leads to confusing permutations of about 5 different things that can happen here, based on a bool ("claim all types") that doesn't do what it says on the tin, really.

I would ask you to file a bug to clean this up but I can't see us actually getting to it because of the risk and how there are 0 tests for this code. Ideas welcome, I guess.

@@ +724,5 @@
>      } else {
> +      // Windows 10 blocks attempts to load the HTTP Handler
> +      // association dialog, so the modern Settings dialog
> +      // is opened with the Default Apps view loaded.
> +      if (IsWin10OrLater()) {

Can we not do this on 8.1 as well, and/or does that not have the same problem?

Comment 17

[:Gijs (he/him)]

(I also did not test this on win10 - hope you have! :-) )

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gijs Kruitbosch from comment #16)
> > +      // Windows 10 blocks attempts to load the HTTP Handler
> > +      // association dialog, so the modern Settings dialog
> > +      // is opened with the Default Apps view loaded.
> > +      if (IsWin10OrLater()) {
> 
> Can we not do this on 8.1 as well, and/or does that not have the same
> problem?

It doesn't have the same problem. Windows 10 is the first to block the HTTP handler association dialog.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/afdd2579f938

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/2923c5cc5dcc

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/afdd2579f938
https://hg.mozilla.org/mozilla-central/rev/2923c5cc5dcc

Comment 22

[:Gijs (he/him)]

(In reply to Pulsebot from comment #20)
> https://hg.mozilla.org/integration/fx-team/rev/2923c5cc5dcc

Hmm? Why was this necessary?

Comment 23

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gijs Kruitbosch from comment #22)
> (In reply to Pulsebot from comment #20)
> > https://hg.mozilla.org/integration/fx-team/rev/2923c5cc5dcc
> 
> Hmm? Why was this necessary?

AppConstants isn't defined in nsBrowserGlue.js and I didn't want to hold up getting it fixed while the build was broken (I must not have properly rebuilt before my local testing of the AppConstants usage).

Plus, there are many other preprocessor usages within nsBrowserGlue.js (I see 34 matches of #ifdef in the file), so this didn't seem to make the issue any worse.

Comment 24

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: windows 10 default browser support
[User impact if declined]: much harder for users to set the default browser on windows 10
[Describe test coverage new/current, TreeHerder]: manual testing
[Risks and why]: affects how the default browser is set on Windows 10. this is the same approach/code that Chromium, so that reduces the risk.
[String/UUID change made/needed]: none

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Taking it for 40 as we would have time to fix potential regressions.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/69dd71505f74

Comment 27

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced this on Aurora 40.0a2, 2015-06-08.

Verified fixed on Nightly 41.0a1 (build ID: 20150614030204) and Aurora 40.0a2 (build ID: 20150614004003).

Comment 28

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: windows 10 default browser support
[User impact if declined]: much harder for users to set the default browser on windows 10
[Describe test coverage new/current, TreeHerder]: manual testing
[Risks and why]: affects how the default browser is set on Windows 10. this is the same approach/code that Chromium, so that reduces the risk.
[String/UUID change made/needed]: none

(this is part of a approval-mozilla-beta request in conjunction with bug 1170803 and bug 1173357)

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8614308 [details] [diff] [review]
Patch

Approved for uplift to beta. Good not to break default browser process for early adopters of windows 10.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5e03a12dd57e