787820 - Can't open downloaded files directly

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[JK]

1. Click: http://vexillium.org/dl.php?/Windows_Kernel-mode_GS_Cookies_subverted.pdf

2. Choose Open with (e.g. Adobe Reader)

Result:
The file is saved to the temp folder but it is not opened.

Expected result:
The file should be opened in the application you chose.

Comment 1

[Alice0775 White]

I can reproduce the issue with new profile + Adobe Acrobat10.1.4.38 installed but disabled it in about:addons

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/360ab7771e27
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820140803
Bad:
http://hg.mozilla.org/mozilla-central/rev/f9a8fdb08193
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120821062401
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=360ab7771e27&tochange=f9a8fdb08193


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/657b3f3a616b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820171600
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f9094ef74bf3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120820172201
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=657b3f3a616b&tochange=f9094ef74bf3

Suspected: 
17ddc4d169e1	Yury — Bug 777388 - Make PDF Viewer settings persistent. r=mak

Comment 2

[Yury Delendik (:yury)]

Attachment: Fixes 777388 incomplete implementation

Hmmm, looks like we need to continue executing of the updateMIMEInfo function logic (change the preferred action), but discard any persistent update to the MIME info database entry.

https://tbpl.mozilla.org/?tree=Try&rev=78874b545dc8

Comment 3

[Bruce]

Just my limited observation - It seems most .pdf files work ok, but those from a .php file fail and dont meet the requirements for prepending the http:// when cut and paste is used from the URL bar.

http://www.irs.gov/pub/irs-pdf/f1040ez.pdf

http://vexillium.org/dl.php?/Windows_Kernel-mode_GS_Cookies_subverted.pdf


This one is an exception.  This also breaks the rules of prepending the http:// when using the cut and paste from the URL bar.  It does not contain .com or .htm/html.

www.aneca.es/content/download/10522/118049/file/academia_guiadeayuda_110310.pdf

Perhaps there is a correlation between .pdf the listing to prepend the http:// using cut and paste from the URL.

Comment 4

[Yury Delendik (:yury)]

Attachment: Fixes 777388 incomplete implementation

Comment 5

[Alex Keybl [:akeybl]]

Is FF17 on aurora affected? I wasn't able to reproduce. pdf.js is disabled in aurora, so perhaps bug 777388 does not affect that version.

Comment 6

[Yury Delendik (:yury)]

It affects FF17. However there was decision made to not enable pdf.js for FF 15/16/17, so we are good here.

Comment 7

[JK]

(In reply to Yury (:yury) from comment #6)
> It affects FF17. However there was decision made to not enable pdf.js for FF
> 15/16/17, so we are good here.

How does disabling pdf.js affect this bug?

Comment 8

[Yury Delendik (:yury)]

The pdfjs handler is not registered with MIME types http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/content/PdfJs.jsm#73, so the functionality added in bug 777388 is not invoked.

Comment 9

[JK]

(In reply to Yury (:yury) from comment #8)
> The pdfjs handler is not registered with MIME types
> http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/
> content/PdfJs.jsm#73, so the functionality added in bug 777388 is not
> invoked.

This bug occurs regardless of the pdfjs.disabled setting.

Comment 10

[Yury Delendik (:yury)]

That what I was afraid of, once a user touched pdfjs.disabled, the MIME entry stays stored.

Comment 11

[Yury Delendik (:yury)]

JK, can you replicate it with fresh profile without changing the pdfjs.disabled property to false?

Comment 12

[JK]

(In reply to Yury (:yury) from comment #11)
> JK, can you replicate it with fresh profile without changing the
> pdfjs.disabled property to false?

It works with a fresh profile.

However, the patch that causes this doesn't seem to be specific to pdf.js, but to any file type that is handled by Firefox but sent by the server as content-dispositon:attachment. This should be tested with e.g. a JPG with the attachment header.

Comment 13

[Yury Delendik (:yury)]

JK(In reply to JK from comment #12)
> (In reply to Yury (:yury) from comment #11)

> However, the patch that causes this doesn't seem to be specific to pdf.js,
> but to any file type that is handled by Firefox but sent by the server as
> content-dispositon:attachment. This should be tested with e.g. a JPG with
> the attachment header.

Could you provide steps to replicate this with JPG?

Comment 14

[JK]

(In reply to Yury (:yury) from comment #13)
> JK(In reply to JK from comment #12)
> > (In reply to Yury (:yury) from comment #11)
> 
> > However, the patch that causes this doesn't seem to be specific to pdf.js,
> > but to any file type that is handled by Firefox but sent by the server as
> > content-dispositon:attachment. This should be tested with e.g. a JPG with
> > the attachment header.
> 
> Could you provide steps to replicate this with JPG?

I don't know if this happens with a JPG. It's hard to find a content-disposition:attachment file.

Other content types that should be tested are Podcast, Web Feed and Video Podcast since they appear in the content type settings with the "Preview in Firefox" option (like PDF, but unlike JPG).

Comment 15

[JK]

Is it possible to set the content-disposition:attachment header for a Bugzilla attachment?

Comment 16

[Yury Delendik (:yury)]

(In reply to JK from comment #14)

> Other content types that should be tested are Podcast, Web Feed and Video
> Podcast since they appear in the content type settings with the "Preview in
> Firefox" option (like PDF, but unlike JPG).

The podcast files with "content-disposition: attachment" had the bug 777388 problem as well. The issue was not reported/found with podcast due to absence real life examples of rss feeds with "content-disposition: attachment".

JK, so what are your expectations: land bug 787820 into FF17 or back out bug 777388?

Comment 17

[JK]

Land this to Aurora since bug 777388 makes web content able to modify the content settings, which is sort of a security hole.

Comment 18

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 658083 [details] [diff] [review]
Fixes 777388 incomplete implementation

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

this function is becoming confusing, though I don't have a good suggestion to fix it off-hand :(

Did you have a chance to evaluate how hard would be to add an automated test for this piece of code? I think may be worth to start filing a bug for it, especially for these content-disposition cases

Comment 19

[Yury Delendik (:yury)]

(In reply to Marco Bonardo [:mak] from comment #18)

> Did you have a chance to evaluate how hard would be to add an automated test
> for this piece of code? I think may be worth to start filing a bug for it,
> especially for these content-disposition cases

I did. It's all clear up to the point of the actual opening of the external application or saving it locally -- opening of the dialog when content-disposition part. The bug 777388 and this patch touches the code when user actually clicks ok-button, and I did not find the way yet to ensure that file was opened by foreign process.

Comment 20

[Yury Delendik (:yury)]

(In reply to Marco Bonardo [:mak] from comment #18)
> I think may be worth to start filing a bug for it,
> especially for these content-disposition cases

bug 790643 is filed

Comment 21

[Yury Delendik (:yury)]

Attachment: patch for checkin

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=c012043c704d
https://tbpl.mozilla.org/?tree=Try&rev=880b76e0a21f&noignore=1

https://hg.mozilla.org/integration/mozilla-inbound/rev/25b53713d895

Should this have a test?

Comment 23

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to Ryan VanderMeulen from comment #22)
> Should this have a test?

see comment 20

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/25b53713d895

Comment 25

[Mihai Morar, (:MihaiMorar)]

I have tested this issue on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0(20120821030519) and I could directly open PDF with Foxit 5.4 and Adobe Reader X too.

It also worked for me using FF 18.b2 with both Foxit and Adobe Reader.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 (20121128060531)

Comment 26

[Bruce]

Just downloaded a fresh nightly install - Dec 6.

Im still seeing the same results as posted in comment 3.

PDF handling in FX nightly 20.0 is inconsistent.  These links result in different actions.  The first is opened in the browser as expected.  The next two will prompt to download only.

This opens in the browser as expected.
  http://www.irs.gov/pub/irs-pdf/f1040ez.pdf


These two links open the download manager without opening in the browser.
  http://vexillium.org/dl.php?/Windows_Kernel-mode_GS_Cookies_subverted.pdf

  www.aneca.es/content/download/10522/118049/file/academia_guiadeayuda_110310.pdf

Additionally the last link does not prepend the http:// when cut/pasting from the URL line.

Comment 27

[Bruce]

In Fx 18.0b3 this link does not open - new tab is blank:

  http://www.irs.gov/pub/irs-pdf/f1040ez.pdf

Comment 28

[juan becerra [:juanb]]

Using Firefox 17.0.1 and Firefox 18.0b3 on Windows 7 and the information in comment #26:

Firefox 17.0.1: 
- Link 1 opens in the browser.
- Link 2 prompts to open or save, and if you opt to open it will open in a separate Adobe Reader window. If you additionally opt for "Do this automatically ... from now on" and retry, it will prompt you again. This would be a bug.
- Link 3, when you copy/paste the link in the browser it will behave like link 2.

Firefox 18.0b3: Same as Fx17.0.1

So as far as my testing goes, this works, except for bug 820013.

Comment 29

[Mihai Morar, (:MihaiMorar)]

(In reply to juan becerra [:juanb] from comment #28) 
> If you additionally opt for "Do this automatically ... from now on" and retry, 
> it will prompt you again. This would be a bug.
> So as far as my testing goes, this works, except for bug 820013.

As said in Comment 25 it works for me for FF 18b6 too on Windows 7 x64:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 (20121227074833)

Based on this and on Comment 28 I consider this issue Verified Fixed for FF 18, because the exception Juan was talking in comment 28 I guess will be treated in bug 820013 .