120174 - Land file save as improvements patch

Status: VERIFIED FIXED

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

Description

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

Comment 1

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

Attachment: patch

Comment 2

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

OK. My changes from yesterday were backed out so this patch is large as it
contains all yesterday's change (plus the fix for the smoketest blocker) as well
as the items I want to land. Here's a breakdown of what's happening in this patch:

1) saveInternal and saveURL are modified to take a parameter to indicate whether
or not the cache should be bypassed in favour of reading data from the network

2) in saveDocument, 'aDocument' is null checked before any attempts are made to
access properties on it. (This is from the reviewed patch for 115440, nothing new)

3) a 'bypassCache' property is added to the data object that is passed through
the header sniffer to the callback.

4) remember the saved to directory & filter index. As with previous patches.
nothing changed.

5) remembering directory saved into. change here is that the complex value set
is the /parent/ of fp.file, not fp.file as in earlier patches. this was the
cause of the smoketest blocker on mac as mac is unable to generate a persistent
descriptor for a file that does not exist (fp.file) but is for the directory
(fp.file.parent). 

6) ensure that we pass the DOM document to the download dialog in the case that
text-only save mode is selected as well, because wbp needs the document to be
able to do the serialization. 

7) if we're saving in text only mode, pass a content type of "text/plain" to the
download dialog

8) only save with postData if we're saving a document. 

9) reordering of default filename selections per 115176. 

Need r and sr asap on this. 

Comment 3

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

Further comments. 

nsProgressDlg.js - 

add a check to dialog load to see if load has already completed before
attempting to do anything. If so, close the dialog (or not, depending on
preference.. this is handled by processEndOfDownload) & finish up. 

add flags to prevent auto-conversion of files (like zip files), allow saves over
existing files to overwrite, and (when appropriate) ignore cache. 

modify saveDocument code to only pass files folder if we're saving as other than
text/plain

modify saveDocument call to take encoding flags for text conversion

nsContextMenu.js - 

- patch callers to saveURL to make
   images - use cache
   links  - bypass cache 

  per API change described in previous comment

Comment 4

[Jeremy M. Dolan]

Possibility of sneaking in a hidden pref for not dataloss'ing the file name for
"Save Page As..."?

I look forward to testing the rest of this on the branch.

Comment 5

[Joe Hewitt (gone)]

Comment on attachment 65112 [details] [diff] [review]
patch

sr=hewitt

Comment 6

[Bill Law]

Comment on attachment 65112 [details] [diff] [review]
patch

r=law

Comment 7

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

Checked in. Thanks!

Comment 8

[Jeremy M. Dolan]

Pretty good fix Ben. First time Save has been usable enough for me to bother to
QA it. I've filed a handful of bugs on save after this patch was in: 120307
120309 120312 120313 120316 120321 120327. 07 and 12 might be 0.9.8 material.

Comment 9

[sairuh (rarely reading bugmail)]

rs vrfy. remaining/new/specific issues covered in other bugs.