772711 - Invoking default application does not work if default application is a metro app

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Brian R. Bondy [:bbondy]]

When trying to obtain the command to execute for a default application we simply check:
HKEY_CLASSES_ROOT\[appid]\Shell\Open\command's default value.

If that value is not filled out then mDefaultAppDescription will be empty and this call will return no default handler.

> nsMIMEInfoWin::GetHasDefaultHandler(bool * _retval)

But sometimes this 'command' key can be left blank and instead a DelegateExecute can be used.  This is the case for all (or most?) metro apps.

To fix this we should lookup the path from the class ID instead.
You can test this right now since pdfjs isn't working in metro by clicking on a pdf file.  Before this patch it would give an error.  After this patch it will succeed and invoke the default pdf viewer in metro.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

This is a review request for m-c.

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 640883 [details] [diff] [review]
Patch v1.

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

::: uriloader/exthandler/win/nsOSHelperAppService.cpp
@@ +431,5 @@
> +    if (NS_FAILED(rv)) {
> +
> +      // Check if there is a DelegateExecute string
> +      nsAutoString delegateExecute;
> +      rv = regKey->ReadStringValue(NS_LITERAL_STRING("DelegateExecute"), delegateExecute);

nit - missing rv check - you use this string below in querying the path.

Comment 3

[Jim Mathies [:jimm]]

Comment on attachment 640883 [details] [diff] [review]
Patch v1.

tested on win8, works with pdfs. Although the default app name is 'TWINUI', is that expected?

Comment 4

[Brian R. Bondy [:bbondy]]

I noticed that too, I'm not sure if it's right or not but we can file a follow up if it has problems.

Comment 5

[Brian R. Bondy [:bbondy]]

Pushed to try, if that passes I'll push to m-i.

Comment 6

[Brian R. Bondy [:bbondy]]

> Although the default app name is 'TWINUI', is that expected?

I'll file a new bug to investigate this. I seen now that you can see this name inside the options of application / mime type association.

Comment 7

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/projects/elm/rev/2b945f193575
http://hg.mozilla.org/projects/elm/rev/99b82aa93e27

Comment 8

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/578ab55e644b

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/578ab55e644b