Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 772711 - Invoking default application does not work if default application is a metro app
: Invoking default application does not work if default application is a metro app
Product: Core Graveyard
Classification: Graveyard
Component: File Handling (show other bugs)
: unspecified
: x86_64 Windows 8.1
: -- normal (vote)
: mozilla16
Assigned To: Brian R. Bondy [:bbondy]
Depends on: 772863 1260483
Blocks: 772815 elm-merge
  Show dependency treegraph
Reported: 2012-07-10 18:07 PDT by Brian R. Bondy [:bbondy]
Modified: 2016-06-22 12:16 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Patch v1. (2.46 KB, patch)
2012-07-10 18:17 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Splinter Review

Description Brian R. Bondy [:bbondy] 2012-07-10 18:07:18 PDT
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] 2012-07-10 18:17:34 PDT
Created attachment 640883 [details] [diff] [review]
Patch v1.

This is a review request for m-c.
Comment 2 Jim Mathies [:jimm] 2012-07-11 05:51:39 PDT
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] 2012-07-11 05:54:18 PDT
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] 2012-07-11 06:29:52 PDT
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] 2012-07-11 07:08:14 PDT
Pushed to try, if that passes I'll push to m-i.
Comment 6 Brian R. Bondy [:bbondy] 2012-07-11 08:00:50 PDT
> 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 8 Brian R. Bondy [:bbondy] 2012-07-14 17:46:10 PDT
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-07-15 10:20:30 PDT

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