[Webkit-unassigned] [Bug 64580] Add support for download='filename' in anchors

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64580





--- Comment #29 from sadrul at chromium.org  2011-07-17 11:28:14 PST ---
(From update of attachment 101111)
View in context: https://bugs.webkit.org/attachment.cgi?id=101111&action=review

>> Source/WebCore/html/HTMLAnchorElement.idl:29
>> +        attribute [Reflect] DOMString download;
> 
> Doe this API need to be conditional on something?  Do all the ports understand how to use the suggested file name?  From reading the code, it seems like this feature might be half-implemented on most ports because it will trigger the download but not suggest the file name.  Maybe I'm misunderstanding?

Nope, you are correct. The suggested filename is not used in any of the ports yet. I will be submitting another CL to use this name in chromium (once it is updated to handle the suggested name). Should this be conditional on PLATFORM(CHROMIUM) in that case?

>>> Source/WebCore/loader/FrameLoader.cpp:1243
>>> +    } else
>> 
>> Why are we adding an Origin header to GET requests?  Normally we don't do that unless we're using CORS.  This request doesn't seem to be a CORS request, however, because it's not using the CORS functions for preparing the request (which do things like remove authentication information).
> 
> The more I think about this code, the more I realize that it's not correct.  For example, if the document is sandboxed (e.g., with the HTML5 sandbox attribute), this code will give use a non-"null" Origin header, which is wrong because the document is in a unique origin.  Instead, assuming we want to add an Origin header, we should generate the header from the document's SecurityOrigin object instead of creating a fake one from the Referer.  That will do the right thing in all cases, and is what the updateRequestForAccessControl function does.

Ah, I see. I am going to remove the Origin header since this is, indeed, not a CORS request. (I wasn't aware of the implications you point out. Thanks)

>> Source/WebCore/loader/FrameLoader.h:275
>> +    void prepareResourceRequest(ResourceRequest&, ReferrerPolicy);
> 
> This function name is very confusing.  Should we call this function all the time when sending a request, or only in some circumstances?  With a name like "prepareResourceRequest" it sounds like we should call this all the time because we always want to prepare resource requests before issuing them.

I can rename it (e.g. prepareResourceRequestForAnchorDownload) to make it more obvious what it does, or get rid of the function altogether and update the ResourceRequest in HTMLAnchorElement::handleClick. WDYS?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.