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

RESOLVED FIXED in mozilla16

Status

Core Graveyard
File Handling
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: bbondy, Assigned: bbondy)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: completed-elm)

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 640883 [details] [diff] [review]
Patch v1.

This is a review request for m-c.
Attachment #640883 - Flags: review?(jmathies)

Updated

5 years ago
Blocks: 772815
Component: Widget: Win32 → File Handling

Comment 2

5 years ago
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

5 years ago
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?
Attachment #640883 - Flags: review?(jmathies) → review+
(Assignee)

Comment 4

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 747347
(Assignee)

Comment 5

5 years ago
Pushed to try, if that passes I'll push to m-i.
(Assignee)

Updated

5 years ago
Whiteboard: completed-elm
(Assignee)

Comment 6

5 years ago
> 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.
(Assignee)

Updated

5 years ago
Depends on: 772863
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/projects/elm/rev/2b945f193575
http://hg.mozilla.org/projects/elm/rev/99b82aa93e27
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/578ab55e644b
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/578ab55e644b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1

Updated

a year ago
Depends on: 1260483
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.