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
Status: RESOLVED FIXED
completed-elm
:
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]
:
:
Mentors:
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: ---


Attachments
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
http://hg.mozilla.org/integration/mozilla-inbound/rev/578ab55e644b
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-07-15 10:20:30 PDT
https://hg.mozilla.org/mozilla-central/rev/578ab55e644b

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