Bug 156057 – Create test infrastructure to support <a download> feature

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

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.

[Brent Fulgham]

Local testing confirms that data: URLs work properly. I'll add tests for blob URLs the Content-Disposition headers to confirm.

[Brent Fulgham]

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

[Brent Fulgham]

Created attachment 275377 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Brent Fulgham]

Created attachment 275419 [details]
Patch

[Brent Fulgham]

Created attachment 275420 [details]
Patch

[Brent Fulgham]

Created attachment 275423 [details]
Patch

[Build Bot]

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

[Build Bot]

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

[Brent Fulgham]

Created attachment 275425 [details]
Patch

[Andy Estes]

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.

[Brent Fulgham]

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!

[Brent Fulgham]

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.

[Brent Fulgham]

Committed r198955: <http://trac.webkit.org/changeset/198955>