158598 - Save Page As fails on pages with no title, filename or URL e.g. JavaScript generated popup windows

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Ed Catmur]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.1b) Gecko/20020721
BuildID:    2002072108

If I use JavaScript to create a new document by a call to window.open() without
any arguments, this will create a new blank page in its own window; I can then
use the DOM to add content to that page.

If I then select File->Save Page As..., nothing happens.

If, however, I use document.title to give the page a title, the Save Page As
dialog will appear.

Similarly, if I visit about:blank and select File->Save Page As..., nothing happens.

Reproducible: Always
Steps to Reproduce:
1.Visit about:blank or call window.open();
2.Select File->Save Page As...

Actual Results:  Nothing happens in Navigator.

The following error is generated in the JavaScript Console:

Error: [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)" 
location: "JS frame :: chrome://communicator/content/contentAreaUtils.js ::
getDefaultFileName :: line 662"  data: no]
Source File: chrome://communicator/content/contentAreaUtils.js
Line: 662

Expected Results:  Save As dialog should appear with some suitable filename
(e.g. blank.html)

I believe there are quite a few bugs out there which express symptoms of this bug.

Comment 1

[Ed Catmur]

Attachment: 4 pages - 2 fail, 2 work

This demonstrates the bug. 
The file itself will not kill Save Page As, as it has a URL or filename.
It opens a window to about:blank, which kills Save Page As.
It creates a document by window.open(), which kills Save Page As.
It creates a document by window.open(), but then gives it a title by
document.title; as a result, Save Page As succeeds.

Comment 2

[Eli Friedman]

I see your problem.  It looks like there are actually two problems: one, that
trying to access the hostname gives an uncaught error, and two, there is nothing
else for Mozilla to use.

I made a patch that solves the error, and defaults to an empty file name (it
will end up as .html).  A different default file name would be nice, but that
would have to be added by i18n.

Comment 3

[Eli Friedman]

Attachment: Patch to handle host error and default to empty filename

Comment 4

[Boris Zbarsky [:bzbarsky]]

file handling.

Could we default the filename to "index" (no extension) please?  And yes, 
please localize it.

Comment 5

[Eli Friedman]

Where should the localized string be located?  I'm not sure where to put it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

contentAreaCommands.properties; see the getStringBundle() function and its uses
in contentAreaUtils.js

Comment 7

[Eli Friedman]

Attachment: Updated patch

Here's the updated patch.

With the patch, attempts at getting a filename for a document come in this
order (copied from the function; parentheses added):
1) Use the name suggested by the HTTP headers
2) Use the actual file name, if present
3) Use the document title
4) Use the caller-provided name, if any
5) Use the host (crashed before patch; now in a try/catch)
6) Use the default file name (added by this patch; currently index)

If everything above is correct, this patch is ready for review.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92309 [details] [diff] [review]
Updated patch

One other thing, please... if the GetStringFromName throws an exception
(because the localization does not provide that string), could you fall back to
"index" in the js?

Comment 9

[Eli Friedman]

Attachment: Another update

Fallback to "index" added

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 92312 [details] [diff] [review]
Another update

put a semicolon after that last return statement, and sr=bzbarsky

Comment 11

[Eli Friedman]

Attachment: Update to Patch

Comment 12

[Jesse Ruderman]

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

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

Comment 14

[Boris Zbarsky [:bzbarsky]]

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

Comment 15

[timeless]

Comment on attachment 92313 [details] [diff] [review]
Update to Patch

r=timeless

for c++ we try to aim for 80 cols, for xul/js i try to aim for 100 cols (80 of
course is still prefered, but recognized as impractical).

personally I'd prefer for your comments to be one line above or below the
affected statement.

but that's me. thanks for the patch.

Comment 16

[Eli Friedman]

Attachment: Final Patch

The only change is to the comment layout, as requested by timeless. (No
functionality change)

timeless, will you handle checkin?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Checked in on the trunk.

Comment 18

[sairuh (rarely reading bugmail)]

using the test case, Save Page As provides a suggested filename for all 4 tests.
verified fixed using 2002.09.19.08 comm trunk builds on all platforms.

Comment 19

[Matthew Elvey]

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

Comment 20

[Matthew Elvey]

Is Save Page As... still working for folks?  I'm seeing something like it in
1.5.  See bug 200741.  Re. the attachment "4 pages - 2 fail, 2 work": for me all
of them fail. (Had to enable pop-ups to get it to show the pages.)  The changes
that the patch made work - the filenames that Save Page As... offers (but fails)
to save are correctly named, and a Save dialog box does pop up.