We should be sure our implementation meets the functional and safety criteria defined in <https://html.spec.whatwg.org/multipage/semantics.html#downloading-resources>: Specifically we want to ensure: 1. Can be used with blob: URLs and data: URLs 2. If the HTTP header Content-Disposition: is present and gives a different filename than this attribute, the HTTP header has priority over this attribute. 3. If this attribute is present and Content-Disposition: is set to inline, we should probably match Chrome and give priority to the download attribute. 4. We should only honor this attribute for links to resources with the same-origin.
Local testing confirms that data: URLs work properly. I'll add tests for blob URLs the Content-Disposition headers to confirm.
These existing tests now pass: fast/dom/HTMLAnchorElement/anchor-nodownload.html fast/dom/HTMLAnchorElement/anchor-download.html fast/dom/HTMLAnchorElement/anchor-nodownload-set.html fast/dom/HTMLAnchorElement/anchor-download-unset.html
Created attachment 275377 [details] Patch
Comment on attachment 275377 [details] Patch Attachment 275377 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1079646 New failing tests: http/tests/security/anchor-download-allow-sameorigin.html http/tests/security/anchor-download-allow-data.html
Created attachment 275384 [details] Archive of layout-test-results from ews100 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 275377 [details] Patch Attachment 275377 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1079654 New failing tests: fast/dom/HTMLAnchorElement/anchor-download.html fast/dom/HTMLAnchorElement/anchor-nodownload-set.html http/tests/security/anchor-download-allow-sameorigin.html http/tests/security/anchor-download-allow-data.html
Created attachment 275385 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 275377 [details] Patch Attachment 275377 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1079652 New failing tests: http/tests/security/anchor-download-allow-sameorigin.html http/tests/security/anchor-download-allow-data.html
Created attachment 275387 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 275377 [details] Patch Attachment 275377 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1079656 New failing tests: http/tests/security/anchor-download-allow-sameorigin.html http/tests/security/anchor-download-allow-data.html
Created attachment 275388 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 275419 [details] Patch
Created attachment 275420 [details] Patch
Created attachment 275423 [details] Patch
Comment on attachment 275423 [details] Patch Attachment 275423 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1082450 New failing tests: fast/dom/HTMLAnchorElement/anchor-nodownload-set.html fast/dom/HTMLAnchorElement/anchor-download.html
Created attachment 275424 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 275425 [details] Patch
Comment on attachment 275425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275425&action=review > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleNavigationAction.h:73 > + bool m_hasDownloadAttribute; Would it make sense to store the download attribute string instead? Do you foresee that ever being useful? > Tools/WebKitTestRunner/TestController.cpp:1676 > + String message = String::format("Download started.\n"); Why String::format()? > Tools/WebKitTestRunner/TestController.cpp:1701 > + String message = String::format("Download failed.\n"); Ditto. > Tools/WebKitTestRunner/TestController.cpp:1723 > + String message = String::format("Download cancelled.\n"); Ditto. > Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:70 > + void waitUntilDownload(); waitUntilDownload() is a little ambiguous. I can't tell by reading this if it'll wait until the download starts or finishes. How about something like waitUntilDownloadDone(), waitUntilDownloadFinished(), or waitUntilDownloaded()? I presume this name came from Blink, and changing it might make it harder to import their tests, but I think a good name is worth that inconvenience.
Comment on attachment 275425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275425&action=review >> Tools/WebKitTestRunner/TestController.cpp:1676 >> + String message = String::format("Download started.\n"); > > Why String::format()? Yeah, that's dumb! I'll fix this (and the other places). >> Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:70 >> + void waitUntilDownload(); > > waitUntilDownload() is a little ambiguous. I can't tell by reading this if it'll wait until the download starts or finishes. How about something like waitUntilDownloadDone(), waitUntilDownloadFinished(), or waitUntilDownloaded()? > > I presume this name came from Blink, and changing it might make it harder to import their tests, but I think a good name is worth that inconvenience. Agreed. I'll change it to "waitUntilDownloadFinished". > Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:294 > + Whoops!
Comment on attachment 275425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275425&action=review >> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleNavigationAction.h:73 >> + bool m_hasDownloadAttribute; > > Would it make sense to store the download attribute string instead? Do you foresee that ever being useful? Sure. That allows a tri-state value (no 'download' attribute, 'download' without an argument, or 'download="abc"'), which might be of interest in tests. I was just trying to make the smallest possible change.
Committed r198955: <http://trac.webkit.org/changeset/198955>