Content-Disposition header filename is ignored when 'download' attribute is specified in HTML. This is not as per HTML specification: - https://html.spec.whatwg.org/#as-a-download (Step 2) Content-Disposition header filename is supposed to override the value of the download attribute. Firefox and Chrome follow the specification.
<rdar://problem/31789855>
Created attachment 307993 [details] Patch
Created attachment 308001 [details] Patch
Comment on attachment 308001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308001&action=review > Source/WebKit2/UIProcess/Downloads/DownloadProxy.cpp:161 > + m_suggestedFilename = String(); Shouldn't this be response.suggestedFilename() instead of an empty String? > LayoutTests/http/tests/download/resources/content-disposition-pass.php:2 > +header("Content-Disposition: attachment; filename=PASS.txt"); Let's do more interesting things than just all lowercase attachment and filename to test the case-insensitivity of our checks. > LayoutTests/http/tests/security/anchor-download-allow-sameorigin.html:13 > -<a id="dl" href="/security/resources/attachment.php" download="foo.pdf">the link</a> is in the same origin. > +<a id="dl" href="/media/resources/test.pdf" download="foo.pdf">the link</a> is in the same origin. This change is not in the ChangeLog. Why was it made?
Comment on attachment 308001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308001&action=review >> Source/WebKit2/UIProcess/Downloads/DownloadProxy.cpp:161 >> + m_suggestedFilename = String(); > > Shouldn't this be response.suggestedFilename() instead of an empty String? I do not think so. m_suggestedFilename is initialized when starting the download with the value of the download attribute. It is then used instead of the suggested filename in DownloadProxy::decideDestinationWithSuggestedFilename() if it is not null. So if I reset to null here, it means there is no override and we will end up using the resource's suggested filename in DownloadProxy::decideDestinationWithSuggestedFilename(). >> LayoutTests/http/tests/security/anchor-download-allow-sameorigin.html:13 >> +<a id="dl" href="/media/resources/test.pdf" download="foo.pdf">the link</a> is in the same origin. > > This change is not in the ChangeLog. Why was it made? I'll add it to the Changelog. /security/resources/attachment.php was previously used from convenience but it sets a Content-Disposition header so it is no longer suitable to use for testing the download attribute.
Created attachment 308036 [details] Patch
Comment on attachment 308036 [details] Patch Attachment 308036 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3598478 New failing tests: http/tests/download/anchor-download-attribute-content-disposition.html
Created attachment 308048 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 308049 [details] Patch
Comment on attachment 308049 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308049&action=review > Source/WebKit2/UIProcess/Downloads/DownloadProxy.cpp:157 > +#if !USE(NETWORK_SESSION) Then double check that this works with network session, too, before landing. EWS runs on El Capitan where we don't use network session
(In reply to Alex Christensen from comment #10) > Comment on attachment 308049 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=308049&action=review > > > Source/WebKit2/UIProcess/Downloads/DownloadProxy.cpp:157 > > +#if !USE(NETWORK_SESSION) > > Then double check that this works with network session, too, before landing. > EWS runs on El Capitan where we don't use network session This code is #if !USE(NETWORK_SESSION) so it is covered by EWS. Locally, I am using NETWORK_SESSION and the tests are passing (including my new one).
Comment on attachment 308049 [details] Patch Clearing flags on attachment: 308049 Committed r215736: <http://trac.webkit.org/changeset/215736>
All reviewed patches have been landed. Closing bug.