Bug 156057

Summary:

Create test infrastructure to support <a download> feature

Product:

WebKit

Reporter:

Brent Fulgham <bfulgham>

Component:

WebKit Misc.

Assignee:

Brent Fulgham <bfulgham>

Status:

RESOLVED FIXED

Severity:

Major

CC:

adam, bfulgham, buildbot, eoconnor, rniwa

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

156056, 156099, 156100, 156583

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews100 for mac-yosemite	none
Archive of layout-test-results from ews104 for mac-yosemite-wk2	none
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Archive of layout-test-results from ews117 for mac-yosemite	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews102 for mac-yosemite	none
Patch	aestes: review+

Brent Fulgham

Reported 2016-03-30 22:19:32 PDT

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.

Attachments
Patch (30.27 KB, patch) 2016-04-01 00:18 PDT, Brent Fulgham	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-yosemite (816.59 KB, application/zip) 2016-04-01 01:03 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.04 MB, application/zip) 2016-04-01 01:07 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (636.52 KB, application/zip) 2016-04-01 01:13 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews117 for mac-yosemite (849.54 KB, application/zip) 2016-04-01 01:17 PDT, Build Bot	no flags	Details
Patch (30.54 KB, patch) 2016-04-01 11:01 PDT, Brent Fulgham	no flags	Details Formatted Diff Diff
Patch (33.56 KB, patch) 2016-04-01 11:12 PDT, Brent Fulgham	no flags	Details Formatted Diff Diff
Patch (33.46 KB, patch) 2016-04-01 11:30 PDT, Brent Fulgham	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-yosemite (949.49 KB, application/zip) 2016-04-01 12:14 PDT, Build Bot	no flags	Details
Patch (33.62 KB, patch) 2016-04-01 12:26 PDT, Brent Fulgham	aestes: review+	Details Formatted Diff Diff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.

Brent Fulgham

Comment 1 2016-03-31 00:24:04 PDT

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

Brent Fulgham

Comment 2 2016-03-31 12:44:35 PDT

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

Comment 3 2016-04-01 00:18:51 PDT

Created attachment 275377 [details] Patch

Build Bot

Comment 4 2016-04-01 01:03:07 PDT

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

Comment 5 2016-04-01 01:03:10 PDT

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 6 2016-04-01 01:07:40 PDT

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

Comment 7 2016-04-01 01:07:42 PDT

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 8 2016-04-01 01:13:00 PDT

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

Comment 9 2016-04-01 01:13:03 PDT

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 10 2016-04-01 01:16:58 PDT

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

Comment 11 2016-04-01 01:17:01 PDT

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

Comment 12 2016-04-01 11:01:13 PDT

Created attachment 275419 [details] Patch

Brent Fulgham

Comment 13 2016-04-01 11:12:09 PDT

Created attachment 275420 [details] Patch

Brent Fulgham

Comment 14 2016-04-01 11:30:33 PDT

Created attachment 275423 [details] Patch

Build Bot

Comment 15 2016-04-01 12:14:00 PDT

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

Comment 16 2016-04-01 12:14:03 PDT

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

Comment 17 2016-04-01 12:26:55 PDT

Created attachment 275425 [details] Patch

Andy Estes

Comment 18 2016-04-01 13:00:50 PDT

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 19 2016-04-01 13:06:35 PDT

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 20 2016-04-01 13:08:10 PDT

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

Comment 21 2016-04-01 14:10:36 PDT

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

Note You need to log in before you can comment on or make changes to this bug.