Defect - Not switching to desktop if Adobe Reader set as default application

VERIFIED FIXED in Firefox 28

Status

defect
P2
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: kjozwiak, Assigned: kjozwiak)

Tracking

unspecified
Firefox 28
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [block28] feature=defect c=content_features u=metro_firefox_user p=2)

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

6 years ago
If the Adobe Reader is set as the default application, Firefox Metro doesn't switch to the desktop when the PDF is opened. So we have a situation where a user downloads a PDF, it opens on the desktop but Firefox Metro just display's a blank screen and does nothing.

If the Windows Reader App is set to the default application, Firefox Metro correctly switches over without issues. Think this should be the same behavior if the Adobe Reader is the default PDF application.

Steps to reproduce the issue:

Prerequisites:
- Download Adobe Reader for the desktop
- Remove the default "Reader" from Windows 8 App Store

1) Open Firefox Metro
2) Search for PDF under Google and select any trusted link that's a [PDF]
3) The PDF will start downloading and once completed, will open on the desktop PDF reader but you'll just have a "blank" page under Firefox Metro

Current Behavior:

- PDF opens on the desktop but the user is never taken there. User just see's a blank page with the "download app bar" at the bottom indicating the PDF was downloaded successfully.

Expected Behavior:

- I think Firefox Metro should switch to the desktop so the PDF that was downloaded is visible & displayed. User can always switch back to Firefox Metro once they are done with the PDF.

Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-10-31-03-02-03-mozilla-central/
Whiteboard: feature=defect c=content_features u=metro_firefox_user p=0 → [triage] feature=defect c=content_features u=metro_firefox_user p=0
Sounds like we have a ShellExecute call somewhere that's not passing:
seinfo.fMask = SEE_MASK_FLAG_LOG_USAGE;
That flag allows windows to make an environment switch when opening files.
Whiteboard: [triage] feature=defect c=content_features u=metro_firefox_user p=0 → [block28] feature=defect c=content_features u=metro_firefox_user p=0
Assignee

Comment 3

6 years ago
Used Brians advice from comment #1 and tried each of the shell executes and this is the one that fixed the problem. Found the ShellExecute calls by using dxr.mozilla.org

Also went through a few test cases to make sure everything was still working:
- Switched the default PDF reader to Adobe and ensured that once downloaded it would switch to the desktop after the PDF has been opened
- Switched the default PDF reader to MS App Reader and ensured that the PDF is opened in the app once it has been downloaded
- Downloaded about 10 PDF's just to make sure things worked
Attachment #827183 - Flags: review?(jmathies)
Assignee

Updated

6 years ago
Assignee: nobody → kamiljoz
From the metro enabled desktop browser doc ms produced, which explains this change - 

'Should a new experience enabled desktop browser try to launch a desktop app and have that app come to the foreground (for example, downloading and launching a file such as a PDF reader), it may do so by using ShellExecuteEx() and specifying the SEE_MASK_FLAG_LOG_USAGE flag in the fMask field. If this flag is not specified, the desktop app launches, but won’t be able to come to the foreground.   There is no automatic switching between the new experience and the desktop.'
Comment on attachment 827183 [details] [diff] [review]
Switches to the Desktop if the default application is not a MS App Reader

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

::: xpcom/io/nsLocalFileWin.cpp
@@ +206,5 @@
>          memset(&seinfo, 0, sizeof(seinfo));
>          seinfo.cbSize = sizeof(SHELLEXECUTEINFOW);
> +        if (XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro) {
> +          seinfo.fMask  = SEE_MASK_FLAG_LOG_USAGE;
> +        } else {

seinfo was zeroed out above using memset, so there's no need to set it to zero. You can remove the |else {...} | code block.
Attachment #827183 - Flags: review?(jmathies) → review+
Blocks: metrov1it18
No longer blocks: metrov1backlog
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [block28] feature=defect c=content_features u=metro_firefox_user p=0 → [block28] feature=defect c=content_features u=metro_firefox_user p=2
Assignee

Comment 6

6 years ago
Attached the new patch with the changes from comment #5. Also went through the test cases from comment #3 and everything worked without any issues.

Jim, would you be able to push this to the try server?
Attachment #827183 - Attachment is obsolete: true
Assignee

Updated

6 years ago
Attachment #827766 - Flags: review+
(In reply to Kamil Jozwiak [:kjozwiak] from comment #6)
> Created attachment 827766 [details] [diff] [review]
> Switches to the Desktop if the default application is not a MS App Reader
> (with changes)
> 
> Attached the new patch with the changes from comment #5. Also went through
> the test cases from comment #3 and everything worked without any issues.
> 
> Jim, would you be able to push this to the try server?

Nothing on try will massage this, and the msdn page says this constant was introduced in xp, so I think you should be ok to push.
Pushed to try just in case since nsILocalFile is used at various places still.
It passed as expected.

Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4036edfda952

Thanks for the patch, one more block28 down! \o/
Target Milestone: --- → Firefox 28
https://hg.mozilla.org/mozilla-central/rev/4036edfda952
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Build ID: 20131117030203

Verified as fixed on latest Nightly using the steps from comment 0 and comment 3. 
Tested with several PDF files of various size and got the expected results each time.
Status: RESOLVED → VERIFIED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.