Bug 96462 - [EFL] Support download attribute feature
Summary: [EFL] Support download attribute feature
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 102785
  Show dependency treegraph
 
Reported: 2012-09-11 20:03 PDT by KyungTae Kim
Modified: 2012-11-20 02:31 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.35 KB, patch)
2012-09-11 20:22 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (11.04 KB, patch)
2012-09-12 03:32 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (11.07 KB, patch)
2012-09-12 04:47 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (10.92 KB, patch)
2012-09-12 05:04 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (10.83 KB, patch)
2012-09-12 16:46 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description KyungTae Kim 2012-09-11 20:03:25 PDT
Download attribute feature is to download a content instead of navigating it.
For enabling this, the FrameLoaderClientEfl::startDownload should get a suggestedName, too.
Additionally, for layout tests, a callback for "download,request" is needed on DumpRenderTree for EFL.
Comment 1 KyungTae Kim 2012-09-11 20:22:51 PDT
Created attachment 163508 [details]
Patch
Comment 2 Gyuyoung Kim 2012-09-11 21:07:38 PDT
Comment on attachment 163508 [details]
Patch

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

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:897
> +     Evas_Object* newView = browser->createView();

IMO, it would be good if you say why this callback needs to create new view via a comment.
Comment 3 Chris Dumez 2012-09-11 22:53:04 PDT
Comment on attachment 163508 [details]
Patch

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

> LayoutTests/platform/efl/Skipped:-154
> -fast/dom/HTMLAnchorElement/anchor-download-unset.html

Please check that those tests pass for WebKit2 EFL. If they don't, move them to LayoutTests/efl-wk2/TestExpectations.
We don't want to make the WK2 bot red.
Comment 4 KyungTae Kim 2012-09-12 03:32:07 PDT
Created attachment 163571 [details]
Patch
Comment 5 Gyuyoung Kim 2012-09-12 03:52:18 PDT
Comment on attachment 163571 [details]
Patch

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

> Source/cmake/OptionsEfl.cmake:57
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_DOWNLOAD_ATTRIBUTE ON)

Wrong alphabetical order.
Comment 6 KyungTae Kim 2012-09-12 04:47:32 PDT
Created attachment 163588 [details]
Patch
Comment 7 Chris Dumez 2012-09-12 04:56:24 PDT
Comment on attachment 163588 [details]
Patch

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

> LayoutTests/platform/efl-wk2/TestExpectations:199
> +BUGWKEFL : fast/dom/HTMLAnchorElement/anchor-download-unset.html = TEXT PASS

Passes for me on WK2, did you check?

> LayoutTests/platform/efl-wk2/TestExpectations:200
> +BUGWKEFL : fast/dom/HTMLAnchorElement/anchor-download.html = TEXT PASS

Fails on WK2 but displays "PASS". Needs to be investigated to have a proper comment in TestExpectations.

> LayoutTests/platform/efl-wk2/TestExpectations:201
> +BUGWKEFL : fast/dom/HTMLAnchorElement/anchor-nodownload-set.html = TEXT PASS

Ditto.

> LayoutTests/platform/efl-wk2/TestExpectations:202
> +BUGWKEFL : fast/dom/HTMLAnchorElement/anchor-nodownload.html = TEXT PASS

Passes for me on WK2.
Comment 8 KyungTae Kim 2012-09-12 05:04:25 PDT
Created attachment 163593 [details]
Patch
Comment 9 KyungTae Kim 2012-09-12 05:12:24 PDT
The test cases anchor-download-unset.html and anchor-nodownload.html is passed although the download_attribute is not supported.
But what I intended was the 4 test cases should be checked althogether (I mentioned it on a comment).
As your comment, because that can be confused, I modified to add only failed test cases on a TestExpectation file.
Comment 10 KyungTae Kim 2012-09-12 05:28:06 PDT
Additionally, I'll explain the test cases below.
anchor-download.html is for an anchor tag that has 'download' as attribute - <a download>
anchor-download-unset.html is a test for removing the download attribute by scripts.
anchor-nodownload-set.html is for adding the download attribute to the anchor tag that have no download attribute by scripts.
anchor-nodownload.html is for anchor tag that does not have download attribute.

So, 'anchor-download.html' and 'anchor-nodownload-set.html' should have same result,
and 'anchor-download-unset.html' and 'anchor-nodownload.html' should have same result.

In the test cases, the blob url that the anchor tag linked is a new document that has "PASS" text as contents.

If there is no download attribute on the anchor tag ('anchor-download-unset.html' and 'anchor-nodownload.html'), 
when the anchor tag is clicked, the current view should load the blob url, that has "PASS" text as contents.

If there is download attribute on the anchor tag ('anchor-download.html' and 'anchor-nodownload-set.html'), 
when the anchor tag is clicked, the blob url should not be loaded on a current view (should be downloaded),
so the current contents "Blob URL" should be remained.

So, if the test case 'anchor-download.html' has "PASS" text as contents, that means it was failed.
Comment 11 Gyuyoung Kim 2012-09-12 08:07:10 PDT
Comment on attachment 163593 [details]
Patch

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

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:900
> +     Evas_Object* newView = browser->createView();

Nit : wrong indentation.
Comment 12 KyungTae Kim 2012-09-12 16:46:03 PDT
Created attachment 163734 [details]
Patch
Comment 13 Gyuyoung Kim 2012-09-12 17:28:00 PDT
Comment on attachment 163734 [details]
Patch

Looks fine now.
Comment 14 WebKit Review Bot 2012-09-12 17:56:28 PDT
Comment on attachment 163734 [details]
Patch

Clearing flags on attachment: 163734

Committed r128393: <http://trac.webkit.org/changeset/128393>
Comment 15 WebKit Review Bot 2012-09-12 17:56:32 PDT
All reviewed patches have been landed.  Closing bug.