133108 – setData() of DataTransfer has a void return type

RESOLVED FIXED Bug 133108

setData() of DataTransfer has a void return type

https://bugs.webkit.org/show_bug.cgi?id=133108

Summary setData() of DataTransfer has a void return type

Jinwoo Song

Reported 2014-05-19 23:26:36 PDT

According to HTML5 spec, setData() of DataTransfer does not return value. http://www.w3.org/TR/html/editing.html#the-datatransfer-interface

Attachments
Patch (9.70 KB, patch) 2014-05-19 23:34 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Patch (11.37 KB, patch) 2014-05-21 03:50 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Patch (11.59 KB, patch) 2014-05-22 00:23 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Patch (12.17 KB, patch) 2014-05-22 03:25 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (654.19 KB, application/zip) 2014-05-22 17:45 PDT, Build Bot	no flags	Details
Patch (12.38 KB, patch) 2014-05-22 22:46 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Patch (12.35 KB, patch) 2014-05-25 18:21 PDT, Jinwoo Song	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Jinwoo Song

Comment 1 2014-05-19 23:34:59 PDT

Created attachment 231753 [details] Patch

Dirk Schulze

Comment 2 2014-05-20 00:58:46 PDT

Comment on attachment 231753 [details] Patch It lacks a test case but looks good to me in general. Is WebKit the only engine exposing a boolean function? Someone with more experience in the code path should take a look over it.

Jinwoo Song

Comment 3 2014-05-20 02:19:11 PDT

(In reply to comment #2) > (From update of attachment 231753 [details]) > It lacks a test case but looks good to me in general. Is WebKit the only engine exposing a boolean function? Someone with more experience in the code path should take a look over it. Blink also returns a boolean type as WebKit but I could not look into Firefox. I'll prepare a test case in following patch.

Jinwoo Song

Comment 4 2014-05-21 03:50:27 PDT

Created attachment 231822 [details] Patch Add a test case.

Dirk Schulze

Comment 5 2014-05-21 05:55:35 PDT

This build problem on Win is real. Could you investigate please?

Radu Stavila

Comment 6 2014-05-21 07:36:57 PDT

Comment on attachment 231822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231822&action=review > Source/WebCore/platform/win/PasteboardWin.cpp:413 > + return; This return doesn't seem necessary anymore.

Jinwoo Song

Comment 7 2014-05-22 00:23:37 PDT

Created attachment 231864 [details] Patch Fix build for window port.

Grzegorz Czajkowski

Comment 8 2014-05-22 02:35:34 PDT

Comment on attachment 231864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231864&action=review > LayoutTests/editing/pasteboard/set_data_typeof_return.html:13 > + if (typeof setData == "undefined") Nit: You can use shouldBeUndefined from js-test-pre.js and get rid of log() function.

Jinwoo Song

Comment 9 2014-05-22 03:25:44 PDT

Created attachment 231873 [details] Patch Update testcase applying Greg's advice.

Alexey Proskuryakov

Comment 10 2014-05-22 09:22:58 PDT

Comment on attachment 231873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231873&action=review > Source/WebCore/platform/win/PasteboardWin.cpp:400 > - return WebCore::writeURL(m_writableDataObject.get(), URL(URL(), data), String(), false, true); > + WebCore::writeURL(m_writableDataObject.get(), URL(URL(), data), String(), false, true); This loses an early return (which does not affect behavior here, but is a tiny bit inefficient). > Source/WebCore/platform/win/PasteboardWin.cpp:413 > + return; > } > - return true; > + return; These early returns are unnecessary. > LayoutTests/editing/pasteboard/set_data_typeof_return-expected.txt:3 > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". There is no "TEST COMPLETE" here, as promised. To fix that, add js-test-post.js as the last script in <body>. We have a script that creates a correct template for a test, named make-new-script-test.

Build Bot

Comment 11 2014-05-22 17:45:17 PDT

Comment on attachment 231873 [details] Patch Attachment 231873 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5382812232318976 New failing tests: media/media-fragments/TC0001.html

Build Bot

Comment 12 2014-05-22 17:45:24 PDT

Created attachment 231925 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Jinwoo Song

Comment 13 2014-05-22 22:46:59 PDT

Created attachment 231943 [details] Patch Applied AP's comments.

Alexey Proskuryakov

Comment 14 2014-05-23 09:53:05 PDT

Comment on attachment 231943 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231943&action=review > Source/WebCore/platform/efl/PasteboardEfl.cpp:104 > + return; > } More unneeded "return" here.

Jinwoo Song

Comment 15 2014-05-25 18:21:47 PDT

Created attachment 232048 [details] Patch Patch for land.

WebKit Commit Bot

Comment 16 2014-05-25 19:54:12 PDT

Comment on attachment 232048 [details] Patch Clearing flags on attachment: 232048 Committed r169327: <http://trac.webkit.org/changeset/169327>

WebKit Commit Bot

Comment 17 2014-05-25 19:54:19 PDT

All reviewed patches have been landed. Closing bug.

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

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component HTML Editing

Assignee

Jinwoo Song

Reported

2014-05-19 23:26 PDT

Modified

2014-05-29 04:14 PDT History

CC List

13 users Show

URL

Keywords

Depends on

Blocks