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

NEW
Unassigned

Status

()

Firefox
File Handling
13 years ago
11 months ago

People

(Reporter: Phil, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

13 years ago
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.)
(Reporter)

Comment 1

13 years ago
(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 ;-)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

13 years ago
Created attachment 164833 [details] [diff] [review]
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.
(Reporter)

Updated

13 years ago
Attachment #164833 - Flags: review?(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....
Attachment #164833 - Flags: review?(bzbarsky) → review+
(Reporter)

Comment 4

13 years ago
(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?
Attachment #164833 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 5

13 years ago
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.
Attachment #164833 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview-
(Reporter)

Comment 6

13 years ago
Created attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

As requested, using a much tidier regex
Attachment #164833 - Attachment is obsolete: true
(Reporter)

Updated

13 years ago
Attachment #165403 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #165403 - Flags: review?(bzbarsky)

Comment 7

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

You probably don't need quite so many brackets ;-)
Attachment #165403 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Comment on attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

r=bzbarsky
Attachment #165403 - Flags: review?(bzbarsky) → review+
Blocks: 160454
Comment on attachment 165403 [details] [diff] [review]
Include also https in this strategy (using regex)

I just checked this patch in.
Attachment #165403 - Attachment is obsolete: true

Comment 10

13 years ago
Could this bug be resolved as FIXED?
See comment 3 and comment 4
Assignee: file-handling → nobody
QA Contact: ian → file-handling
Component: File Handling → File Handling
Product: Core → Firefox
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.