User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120324 Firefox/14.0a1 Build ID: 20120324031100 Steps to reproduce: Open a PDF file using new integrated PDF Viewer, then use the "Save as..." Firefox functionnality Actual results: The saved file can not be used (not a pdf file) Expected results: The saved file should be the original PDF file = the same that user can get using the Download button from the PDF Viewer dedicated toolbar

The file is saved as HTML (with .pdf.html), and the Download Manager always says its size is 53,5 KB.

Related: bug 744379.

Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120712 Firefox/15.0a2 In addition to comment #1: opening the pdf.html file, it contains only blank pages

The Download button doesn't work either.

My bug talks about the download button not working either maybe it should not be duplicate?

Ignore my previous comments… I was doing it wrong…

This works for me with Firefox 19.0b5 and the latest Nightly. Can someone please confirm?

It doesn't work for me on either of those builds (Mac 10.7.5, Ubuntu 12.10 x64, Win 7 x64): the saved document is html and has only blank pages

Screenshot of what I see when I hit Command-S while viewing a PDF. Note the .pdf.html extension and the 'Format: Web Page, Complete'.

See previous comment for a screenshot. STR: Start firefox with new/clean profile Open URL pointing to PDF file Hit command-s Expected: PDF is saved Actual: A file called foo.pdf.html and directory called foo.pdf_files are created. The foo.pdf_files directory contain things like pdf.js

Same here, except that on Windows it shows only the name.pdf (without .html) in the save as dialog, but the result is the same

I'm not clear on why this has dataloss keyword and critical severity. Yes, the access key does not work as expected in the context of the PDF Viewer, but it's not like the document is lost. You can still click on the download button (I know it was stated a while back that the download button wasn't working but it does work in current builds -- at least on my end). We should probably be a bit smarter to trigger the download behaviour when Save Page As is triggered from a tab with a PDF loaded, but I don't think that was in scope for Firefox 19. Would someone mind providing justification for the severity?

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #19) > Would someone mind providing justification for the severity? Dataloss can happen if you don’t make sure that the file was saved properly. The file can become unavailable or disappear (even be deleted by yourself).

(In reply to Aleksej [:Aleksej] from comment #20) > Dataloss can happen if you don’t make sure that the file was saved properly. > The file can become unavailable or disappear (even be deleted by yourself). I think that's a bit of a stretch to consider this a "dataloss" bug. I would agree if the download button wasn't working, as earlier reported. But this is no longer the case. I'm open to be convinced otherwise but I have yet to see a compelling argument.

This should be fixed before Fx 19 with PDF.js is released (regardless of severity), or PDF.js needs to be dealyed once again. The current behaviour is really confusing. The download button in the PDF.js bar may not be obvious for many users thus they may try to download the displayed PDF via the menu bar (many esp. not tech-savvy users use menu items even when there are buttons available because they learned it that way) – which doesn't download the PDF at all (but rather the PDF.js code). Plus, this bug is probably (AFAICS) not that easy to solve (i. e. requiring quite a few code changes, compared to just a few LOC). Considering Bug 806057 and Bug 761539 (both supposedly fixed upstream), I think we should delay PDF.js for release and let all the fixes be tested on Beta 20 (we're just too close to release to fix and test all those bugs/code changes). Otherwise, this will be a PR disaster. This should be blocker for Fx 19 (or rather PDF.js pref'd off for Fx 19).

(In reply to Florian Bender from comment #22) > is really confusing. The download button in the PDF.js bar may not be obvious for many users thus they may try to download the displayed PDF via When I want to click the download button, I always wonder if I am looking at the right button. bug 762371

Yeah, the download button is not obvious at all. It should be a floppy disk symbol and preferably the word “Save” beside it in text, since there is *plenty* of space.

Just for the record, this bug also applies to right click -> save page as. Testimonial from my (tech-savvy) gf: "I hate the PDF viewer because it doesn't let me save my PDFs." She was trying to use the menubar -> save as thus saving the HTML (in a file with the .pdf extension) + ressources. This needs to be fixed, it's a huge UX issue for many many Fx users, and PDF.js needs to be disabled for Fx 19. Please!

(In reply to Florian Bender from comment #26) > Testimonial from my (tech-savvy) gf: "I hate the PDF viewer because it > doesn't let me save my PDFs." She was trying to use the menubar -> save as > thus saving the HTML (in a file with the .pdf extension) + ressources. You can use the download button (down arrow) on the right of the PDF Viewer header. > PDF.js needs to be disabled for Fx 19. Too late.

(In reply to Scoobidiver from comment #27) > You can use the download button (down arrow) on the right of the PDF Viewer > header. I do know that, but most users do not (or at least will need some time trying to figure this out). I showed it to her, but I ended up disabling the viewer for her. This stuff needs to be as intuitive as possible, and currently it's not. (In reply to Scoobidiver from comment #27) > > PDF.js needs to be disabled for Fx 19. > Too late. Yes and no. There's already a hotfix available (Bug 839239) which just needs to ship.

Is there any way to disable the pdf viewer? It crashes the browser a lot. I want it the way it was. The way that worked. With the plugin.

(In reply to ranunculoid from comment #29) > Is there any way to disable the pdf viewer? It crashes the browser a lot. I > want it the way it was. The way that worked. With the plugin. Please ask your question in the support forum to disable PDF Viewer or file a new bug to report crashes if there is none related (see the Related Bugs section in crash reports of about:crashes).

Some user stories: https://input.mozilla.org/opinion/3579955 https://input.mozilla.org/opinion/3579939 (probably due to PDF.js) https://input.mozilla.org/opinion/3579945 (This is just the last 10 minutes.)

Asking for tracking as it confuses many users (see also duplicates).

Given the volume of user complaints (and the fact that we should be able to resolve fairly easily), we'll track for upcoming releases.

Brian - let us know if the PDF.js team needs any tips from the FF frontend team about how to accomplish this integration (CC Gavin).

We looked into to doing this awhile back and if I remember correctly bz said that it wasn't a trivial task. It would probably be best if we could get someone from the FF team to work on this or at least give us some concrete pointers on implementing this. (looking for the previous conversation now)

Actually, I think capturing the "Ctrl+s" short-cut would be trivial, and a fix like this could be easily uplifted to beta? Yes, it would still be broken from the Firefox menu (which should be fixed later), but it's what web-apps are expected to do: make common shortcuts work as expected.

> but it's what web-apps are expected to do: make common shortcuts work as expected. For the user, pdf.js is not a web-app - its integrated into the firefox, and therefore is expected to function exactly as a native app would do.

I like the idea of fixing at least the shortcut – for Release (if there will be a 19.0.1). However for Fx 20 this bug should be fixed thoroughly.

This adds a simple JS module that allows pdf.js to easily override saveDocument on a per-document basis. The idea here is that pdf.js would set PDFJSSaveAs.callback to a callback that: - checks whether the document in question is a pdf.js document (not sure how this should happen exactly) - if so, return true, and then handle the document saving itself - otherwise, returns false to let the toolkit code perform the default save The callback in question is passed: - the chrome window where saveDocument was invoked - the content document that was intended to be saved (this could be a sub frame, in the case of "Save frame as...") Brendan, is this sufficient for PDF.js to fix this properly?

Fixes one minor bug with the missing 'this' PDFJSSaveAs.jsm line 33. Gavin, We're going to land the fixes on the pdf.js side then bring over a full update of pdf.js for mozilla central in another bug. We'll then create cherry-picked patches for uplift in this bug.

Comment on attachment 719071 [details] [diff] [review] toolkit patch v2 Given that I wrote most of this I don't think I should review it, but thanks for actually testing it :)

[20130222-testday] edwardb linux-32bit A bandaid solution is found: While executing https://moztrap.mozilla.org/runtests/run/847/env/150/ case #665, selected "All Files" filter, and forcing .pdf as the file ending to be saved. As a result, the file IS saved as a .pdf and can be opened by a pdf reader / editor. it's a step in the right direction, but the fact that the dialog box doesn't display ".pdf" as the default filter to choose means that work still is needed to eradicate this bug.

Comment on attachment 719071 [details] [diff] [review] toolkit patch v2 >--- a/toolkit/content/contentAreaUtils.js >+++ b/toolkit/content/contentAreaUtils.js >@@ -1,15 +1,16 @@ > # -*- Mode: javascript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > Components.utils.import("resource://gre/modules/Services.jsm"); > Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm"); >+Components.utils.import("resource://gre/modules/PDFJSSaveAs.jsm"); > > var ContentAreaUtils = { > > // this is for backwards compatibility. > get ioService() { > return Services.io; > }, contentAreaUtils.js is loaded in many chrome documents that don't need access to PDFJSSaveAs. As a private utility it should be tacked on the ContentAreaUtils object.

Yeah, good point. We should fix that.

Comment on attachment 719736 [details] [diff] [review] toolkit patch v3 After a discussion from bz I've found a better way forward on this. In particular, a full-page PDF plugin document has a contentType of application/pdf which saves as you would expect. So, we need to get pdfjs to store a custom property on the channel which we read back in StartDocumentLoad when we set the content type.

I've never used github so I don't know how to submit this change.

To further complicate things in the near future we may not always be using the original stream to fetch the PDF. We'll be doing a second request to see if the server supports range requests. If it does we'll then be cancelling the original request and then downloading chunks of the file with an XHR in the worker. This is so we can render pages without downloading the whole file.

That's a good thing to keep in mind. On save, the original stream will have to be refetched if the second (range-supported) request is active.

Comment on attachment 719750 [details] [diff] [review] Proposed patch I can live with this, I think...

(In reply to neil@parkwaycc.co.uk from comment #48) > Created attachment 719752 [details] [diff] [review] > PdfStreamConverter.js changes > > I've never used github so I don't know how to submit this change. I'll upstream the changes and they'll come to MC in a few days when we update the pdf component. As for the uplift pieces I'll create separate patches and nominate in this bug.

(In reply to neil@parkwaycc.co.uk from comment #53) > https://hg.mozilla.org/integration/mozilla-inbound/rev/1cd1a9fdf8fb Can you please nominate this for uplift already(asap) if we fine with the testing/bake-time it has had on m-c ? We would like to get it into Fx20 beta3 which is going to build tomorrow.

[Approval Request Comment] Bug caused by (feature/regressing bug #): n/a User impact if declined: save as doesn't work properly for pdfs Testing completed (on m-c, etc.): only the changes to nsDocument have been tested on m-c, local testing of the full patch Risk to taking this patch (and alternatives if risky): I can only speak for the pdf.js side of things which is very low risk, for the changes to nsDocument someone else will have to comment. String or UUID changes made by this patch: none

[Approval Request Comment] Bug caused by (feature/regressing bug #): n/a User impact if declined: save as doesn't work properly for pdfs Testing completed (on m-c, etc.): only the changes to nsDocument have been tested on m-c, local testing of the full patch Risk to taking this patch (and alternatives if risky): I can only speak for the pdf.js side of things which is very low risk, for the changes to nsDocument someone else will have to comment. String or UUID changes made by this patch: none

(In reply to Brendan Dahl from comment #57) > Created attachment 720980 [details] [diff] [review] > beta save as support Looks like you attached the wrong patch for beta?

Comment on attachment 720979 [details] [diff] [review] aurora save as support r=me

Comment on attachment 720980 [details] [diff] [review] beta save as support Definitely the wrong patch.

Sorry about that.

Comment on attachment 721057 [details] [diff] [review] beta save as support v2 r=me

Comment on attachment 721057 [details] [diff] [review] beta save as support v2 Request to land by tomorrow(3/5) to make it into FX20 beta 3.

https://hg.mozilla.org/releases/mozilla-aurora/rev/70dbd1a57c50 https://hg.mozilla.org/releases/mozilla-beta/rev/bbb75e4bd6ad

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0 Verified the fix on Firefox 20beta3: CTRL/CMD + S and "Save page" from file menu work as expected - the file is correctly saved as PDF.

Verified as fixed on Friefox 21 Beta 1 (20130401192816): Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20100101 Firefox/21.0

The problem seems related to the option chosen when saving the page containing the PDF. If I choose "Save Web page as" --> "Web page, complete", then the saved PDF results corrupted (this behavior still persists in Firefox 21beta2). If I choose "Save Web pagee as" --> "All files", the saved PDF is OK both in Firefox 20 and Firefox 21beta2. I'm on Mac OSX 10.8.2.

This bug is fixed. You shouldn't see the "Save Web pagee as" choice in the first place. If not, it is a different bug. Please file a new bug with a detailed information.

> You shouldn't see the "Save Web pagee as" choice Correction: "Web page, complete" choice

Verified as fixed on Firefox 22 beta 1: 20130514181517. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:22.0) Gecko/20100101 Firefox/22.0 Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0