872012 - Work - Non-Metro-enabled Firefox cannot take the default browser from Metro-enabled Nightly

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:
1. Install a release version of Firefox.
2. Install Nightly.
3. Try to set the release Firefox to default from [Options]-[Advanced]-[System Defaults]-[Make Firefox the default browser].
4. Double click a .url shortcut or type a URL into Start Screen.

Actual result:
Nightly will run.

Expected result:
The release firefox should start.

Perhaps we need to remove the "DelegateExecute" named value even if the Metro support is disabled.

Comment 1

[Masatoshi Kimura [:emk]]

I would like to backport this because this is obviously useless unless it is fixed before enabling Metro.

Comment 2

[Masatoshi Kimura [:emk]]

Removing "DelegateExecute" didn't work somehow.

Comment 3

[Brian R. Bondy [:bbondy]]

At step 3.5 are you going to control panel and finishing the default setup by selecting Firefox and making it the default?

Comment 4

[Masatoshi Kimura [:emk]]

I didn't have to control panel myself. [Make Firefox the default browser] brought the [Default Programs] panel for me. Then I clicked the panel to try to make the release Firefox default, of course.

Comment 5

[Brian R. Bondy [:bbondy]]

Ya I meant [Default Programs] by [Control Panel] thx.  Just making sure since it wasn't specified :D

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Remove Metro registrations when Metro support is disabled

This patch worked for me.
I had to remove AppUserModelID either.

Comment 7

[Brian R. Bondy [:bbondy]]

Comment on attachment 750409 [details] [diff] [review]
Remove Metro registrations when Metro support is disabled

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

I think we should be just calling CleanupMetroBrowserHandlerValues before setting the default when MOZ_METRO is not defined.  The appusermmodelid is used for a lot of things and shouldn't be removed.

Comment 8

[Masatoshi Kimura [:emk]]

(In reply to Brian R. Bondy [:bbondy] from comment #7)
> I think we should be just calling CleanupMetroBrowserHandlerValues before
> setting the default when MOZ_METRO is not defined.  The appusermmodelid is
> used for a lot of things and shouldn't be removed.

That sounds like a good idea except that it didn't work at all. CleanupMetroBrowserHandlerValues did nothing for other installations.

Comment 9

[Brian R. Bondy [:bbondy]]

I think we have to call it after setting the default since that function aborts if you aren't the default, did you try it before or after the default was set?

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: Non working patch

If this code does nothing, even the current code will do nothing. Because I just moved CleanupMetroBrowserHandlerValues out from MOZ_METRO ifdefs.

Comment 11

[Masatoshi Kimura [:emk]]

(In reply to Brian R. Bondy [:bbondy] from comment #7)
> The appusermmodelid is
> used for a lot of things and shouldn't be removed.

The AppUserModelID will be overwritten with a different value anyway if another Metro-enabled installation try to take the default browser. I don't understand why we shouldn't remove it for a non-Metro-enabled installation.

Comment 12

[Brian R. Bondy [:bbondy]]

Even if we don't have a Metro browser we still want to have the AppUserModelID, it is used for other OS integration points like the taskbar and the jumplists.

Comment 13

[Jim Mathies [:jimm]]

Seems like we could update the CEH such that when installed it's smarter about choosing the right browser. More generally I think we want to migrate toward using the CEH for launching since it gives us more control.

http://mxr.mozilla.org/mozilla-central/source/browser/metro/shell/commandexecutehandler/

Comment 14

[Masatoshi Kimura [:emk]]

Attachment: Get default browser's path from the association when launching on the desktop

Like this?

Comment 15

[Masatoshi Kimura [:emk]]

The new patch will not need uplifting. The problem will be fixed solely inside the Metro-enabled browser.

Comment 16

[Masatoshi Kimura [:emk]]

Attachment: Get default browser's path from the association when launching on the desktop, v2

If non-Metro-enabled browser is default, tapping the Nightly tile will fail to launch the Metro browser. We need to fallback to launching the desktop browser in this case.

Comment 17

[Masatoshi Kimura [:emk]]

Comment on attachment 751053 [details] [diff] [review]
Get default browser's path from the association when launching on the desktop

Forgot to obsolete the previous patch.

Comment 18

[Jim Mathies [:jimm]]

Comment on attachment 751324 [details] [diff] [review]
Get default browser's path from the association when launching on the desktop, v2

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

::: browser/metro/shell/commandexecutehandler/CommandExecuteHandler.cpp
@@ +293,2 @@
>  
> +    BOOL res;

Just noticed this, we should set a default here in case QueryAppIsDefaultAll fails - 

BOOL res = FALSE;

@@ +311,5 @@
> +
> +    Log(L"registeredApp=%s", registeredApp);
> +    bool result = !wcsicmp(registeredApp, kDefaultMetroBrowserIDPathKey);
> +    if (!result) {
> +      result = !wcsicmp(registeredApp, kDemoMetroBrowserIDPathKey);

kDemoMetroBrowserIDPathKey is no longer in use, you need to merge to mc.

Comment 19

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d177dbf23f4

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0d177dbf23f4