Closed Bug 1167294 Opened 9 years ago Closed 9 years ago

Windows 10 blocks the HTTP handler/default browser application dialog

Categories

(Firefox :: Shell Integration, defect, P1)

Unspecified
Windows 10
defect
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 41
Iteration:
41.2 - Jun 8
Tracking Status
firefox39 --- fixed
firefox40 + verified
firefox41 --- verified

People

(Reporter: phlsa, Assigned: jaws)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Attached image 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.
Tracking as we want to see that fixed in 40.
Attached image 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.
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...
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
Assignee: nobody → jaws
Status: NEW → ASSIGNED
(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.
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.
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.
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...
Attached patch Patch (untested) (obsolete) — Splinter Review
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).
Attachment #8613881 - Flags: review?(jmathies)
Iteration: --- → 41.2 - Jun 8
Points: --- → 5
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.
Attachment #8613881 - Flags: review?(jmathies)
Attachment #8613881 - Flags: review-
Attachment #8613881 - Flags: feedback+
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.
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.
Summary: Use the native Windows 10 mechanism for setting the default browser → Windows 10 blocks the HTTP handler/default browser application dialog
Attached patch PatchSplinter Review
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.
Attachment #8613881 - Attachment is obsolete: true
Attachment #8614308 - Flags: review?(jmathies)
Attachment #8614308 - Flags: review?(gijskruitbosch+bugs)
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.
Attachment #8614308 - Flags: review?(jmathies) → review+
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?
Attachment #8614308 - Flags: review?(gijskruitbosch+bugs) → review+
(I also did not test this on win10 - hope you have! :-) )
(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.
https://hg.mozilla.org/mozilla-central/rev/afdd2579f938
https://hg.mozilla.org/mozilla-central/rev/2923c5cc5dcc
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(In reply to Pulsebot from comment #20)
> https://hg.mozilla.org/integration/fx-team/rev/2923c5cc5dcc

Hmm? Why was this necessary?
Flags: needinfo?(jaws)
(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.
Flags: needinfo?(jaws)
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
Attachment #8614308 - Flags: approval-mozilla-aurora?
Comment on attachment 8614308 [details] [diff] [review]
Patch

Taking it for 40 as we would have time to fix potential regressions.
Attachment #8614308 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1173357
Flags: qe-verify+
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).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
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)
Attachment #8614308 - Flags: approval-mozilla-beta?
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.
Attachment #8614308 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.