115440 - File/Save Page as doesn't work for PDF Files [also text, movie, image bugs]

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect, P2)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.6+) Gecko/20011213
BuildID:    2001121417, self-compiled

When a PDF File is displayed using the Adobe Acrobat PDF plugin (nppdf.so),
File|Save Page As doesn't do anything (at least the filepicker doesn't appear).

Reproducible: Always
Steps to Reproduce:
1. Visit the above URL with nppdf.so installed
2. Click File/Save Page As


Actual Results:  Nothing

Expected Results:  Filepicker shows up and on clicking OK the file is saved.

I get the following message in JS Console:
Error: aDocument has no properties
Source File: chrome://communicator/content/contentAreaUtils.js
Line: 284

Comment 1

[Boris Zbarsky [:bzbarsky]]

La dee dee.

Over to Ben.

Comment 2

[Ben Goodger (use ben at mozilla dot org for email)]

yikes. ok, saving a document when there is none. easy. will just call saveURL
instead in this case. patch soon. 

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ben, could we just do saveURL for _all_ non-html content?

That would also fix the issues with saving plaintext and images incorrectly...

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch to fix

removes duplication of code & creates single 'saveInternal' method that always
sniffs for content-type. This ensures that 'savePage' on a image document or
pdf file will save as the right type rather than trying to save as text/html.
Also removes the not-hooked-up-yet Text only filter.

Comment 5

[David Hyatt]

sr=hyatt

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

fix checked in.

Comment 7

[Asa Dotzler [:asa]]

Ben, did this (should this) make it into the 0.9.7 branch?

Comment 8

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.7 (hoping it will make 115132
better)

Comment 9

[Brendan Eich [:brendan]]

Checked into the 0.9.7 branch.

/be

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Still does not work in a recent CVS build from some hours ago (linux)

reopening

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: actually fix

acquire location to save from root content view if document does not exist. 

(mixed in with some other changes)

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 115395 has been marked as a duplicate of this bug. ***

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 115210 has been marked as a duplicate of this bug. ***

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 115087 has been marked as a duplicate of this bug. ***

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 64031 [details] [diff] [review]
actually fix

r=bzbarsky

Comment 16

[Bill Law]

nsIPref is deprecated; we should be using nsIPrefService/nsIPrefBranch instead.

Comment 17

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch that uses nsIPrefService/nsIPrefBranch

Comment 18

[Ben Goodger (use ben at mozilla dot org for email)]

Can I get r/sr on this new patch?

Comment 19

[sairuh (rarely reading bugmail)]

updating summary to remind me to verify for the cases in the dups of this bug.

Comment 20

[Bill Law]

Ben, I think the getBranch("") call should ask for "browser.download.", or, the
getComplexValue("dir") should use "browser.download.dir".

If you change the getBranch, then don't you need to change all the other users
of that branch to drop the "browser.download." prefix?

Comment 21

[Ben Goodger (use ben at mozilla dot org for email)]

oops, that's a typo. I changed the getComplexValue call to call
"browser.download.dir"

Comment 22

[Bill Law]

Comment on attachment 64199 [details] [diff] [review]
patch that uses nsIPrefService/nsIPrefBranch

r=law

with the fix described in the bug comments

Comment 23

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: use pref branches to better advantage

Comment 24

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: er, the right patch this time

Comment 25

[Brendan Eich [:brendan]]

Comment on attachment 64925 [details] [diff] [review]
er, the right patch this time

Noting law's r= and adding sr=[e-mail].

/be

Comment 26

[Ben Goodger (use ben at mozilla dot org for email)]

fix checked in

Comment 27

[sairuh (rarely reading bugmail)]

okay, using 2002.01.24.0x comm bits i've tested the following scenarios, where
save page works. what i did was

1. load a given file
2. selected File > Save Page As [accepted whatever filetype was listed]
3. downloaded it
4. reopen it in browser to make sure the browser actually saved the file sensibly

scenarios that passed:

* linux rh7.2: text file, shockwave flash file

* win2k: text, pdf, flash

* mac os 10.1.2: text, quicktime, flash
* mac os 9.2: pdf

Christian, could you verify pdf on linux, pls? thx a lot!

Comment 28

[Boris Zbarsky [:bzbarsky]]

Verified that PDF can be saved on linux, current CVS trunk build.

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

verified on linux, cvs build 2002012422, using steps described by sairuh.
pdf from the url field is correctly saved.