263839 - 'Save Link Target As' may automatically add .htm to the target save-as filename

Status: NEW

(Firefox :: File Handling, defect)

Description

[Phil]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a4) Gecko/20041010
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a4) Gecko/20041010

With the checkin of the patch (attachment 160223 [details] [diff] [review]) for bug 160454, there is now
no HEAD request done before saving a link target. Previously, the HEAD response
was used (if possible) to determine the suggested target filename (when saving a
url).

However now, when the target is an http address such as the root note of a
website (and not specifically an html page, file or otherwise), there is a
problem. Without a known content type or document object, the browser can only
/guess/ a sensible name for the target filename.
For example, if the target URL is http://www.xyz.com, the target filename would
be given the name "http_www.xyz.com_", which will be wrong in basically all
cases. Therefore with patch 160223, the extension ".htm" is added to the
suggested filename IF, at the end of the method that determines the filename,
the following is true:
a) a file extension has not yet been determined,
b) the url starts with "http://",
c) the content type is unknown, and
d) there is no document object.

NB (c) and (d) are true when performing a 'save link target'.

This should cause no problem when linking to a website (apparently not
specifically to a particular file) where the protocol is http, as it would be
most common for the webserver to present an html file as the default file.

While this default action (adding the ".htm" extension) should cause no problem
in the majority of cases, can we come up with a better heuristic?


Reproducible: Couldn't Reproduce
Steps to Reproduce:
Somebody should be able to confirm if there's a problem by making the default
page served up by a web site something other than an html file. (Sorry, I cannot
easily.)

Comment 1

[Phil]

(In reply to comment #0)
> However now, when the target is an http address such as the root note of a

barrggg, root note = root node ;-)

Comment 2

[Phil]

Attachment: Include also https in this approach

This small patch applies the same logic to https:// urls that was introduced by
the patch for bug 160454.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 164833 [details] [diff] [review]
Include also https in this approach

r=bzbarsky, but this really isn't a patch for _this_ bug....

Comment 4

[Phil]

(In reply to comment #3)
> (From update of attachment 164833 [details] [diff] [review])
> r=bzbarsky, but this really isn't a patch for _this_ bug....
> 

No, it isn't a patch to resolve the problem, but it is a patch nonetheless to
make the code more consistent before a better alternative is found.
Who should I ask for a SR?

Comment 5

[[e-mail]]

Comment on attachment 164833 [details] [diff] [review]
Include also https in this approach

Sorry, I'm not letting you get away with duplicating bad code, those aURL tests
can all be handled by a fairly simple regexp.

Comment 6

[Phil]

Attachment: Include also https in this strategy (using regex)

As requested, using a much tidier regex

Comment 7

[[e-mail]]

Comment on attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

You probably don't need quite so many brackets ;-)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

r=bzbarsky

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

I just checked this patch in.

Comment 10

[OstGote!]

Could this bug be resolved as FIXED?

Comment 11

[Boris Zbarsky [:bzbarsky]]

See comment 3 and comment 4