Bug 156583 - Make <a download> a runtime enabled option
Summary: Make <a download> a runtime enabled option
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on: 102914 156057
Blocks: 156056
  Show dependency tree / graph
 
Reported: 2016-04-14 10:49 PDT by Brent Fulgham
Modified: 2016-04-14 17:45 PDT (History)
10 users (show)

See Also:

Attachments
Patch (19.43 KB, patch)
2016-04-14 13:27 PDT, Brent Fulgham 		achristensen: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-yosemite (1.26 MB, application/zip)
2016-04-14 14:14 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews114 for mac-yosemite (849.60 KB, application/zip)
2016-04-14 14:35 PDT, Build Bot 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2016-04-14 10:49:53 PDT 
The <a download> feature isn't very useful without Blob download support, so we don't want to ship this with production builds of WebKit. However, since we want to encourage experimentation with the feature, we should switch from a compile-time flag to a runtime option.
Comment 1 Radar WebKit Bug Importer 2016-04-14 13:17:00 PDT 
<rdar://problem/25733449>
Comment 2 Brent Fulgham 2016-04-14 13:27:40 PDT 
Created attachment 276416 [details]
Patch
Comment 3 Alex Christensen 2016-04-14 13:37:41 PDT 
Comment on attachment 276416 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276416&action=review

> Source/WebCore/html/HTMLAnchorElement.cpp:560
> +    if (RuntimeEnabledFeatures::sharedFeatures().downloadAttributeEnabled())
> +        downloadAttribute = fastGetAttribute(downloadAttr);

Is this the only place we need to check if it's enabled before doing something?
Comment 4 Build Bot 2016-04-14 14:14:49 PDT 
Comment on attachment 276416 [details]
Patch

Attachment 276416 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1157956

New failing tests:
imported/w3c/web-platform-tests/html/dom/interfaces.html
js/dom/dom-static-property-for-in-iteration.html
imported/w3c/web-platform-tests/html/dom/reflection-text.html
Comment 5 Build Bot 2016-04-14 14:14:52 PDT 
Created attachment 276427 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-04-14 14:35:50 PDT 
Comment on attachment 276416 [details]
Patch

Attachment 276416 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1158039

New failing tests:
js/dom/dom-static-property-for-in-iteration.html
Comment 7 Build Bot 2016-04-14 14:35:53 PDT 
Created attachment 276433 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Brent Fulgham 2016-04-14 17:07:32 PDT 
To enable the feature after this patch, do the following (on Mac) to test with Safari.

defaults write com.apple.Safari com.apple.Safari.ContentPageGroupIdentifier.WebKit2DownloadAttributeEnabled -bool yes
Comment 9 Brent Fulgham 2016-04-14 17:43:30 PDT 
I identified the cause of the WK1 test failures and fixed them locally prior to landing. I am able to run WK1 and WK2 tests without errors.
Comment 10 Brent Fulgham 2016-04-14 17:45:41 PDT 
Committed r199571: <http://trac.webkit.org/changeset/199571>