Bug 58448

That remind me of something. Which spec is that?

Created attachment 89640 [details] Add initial implementation of datatransferitems Add initial implementation of datatransferitems. It is based on chrome implementation. And right now it is not supporting images.

Comment on attachment 89640 [details] Add initial implementation of datatransferitems View in context: https://bugs.webkit.org/attachment.cgi?id=89640&action=review Interesting stuff. :) There seems to be a lot of code duplication in the DataTransferItemsQt implementation, maybe we could share the common paths with DataTransferItemsChromium somehow? On a related note, when adding derived code like this, you should leave the original copyright (as well as adding your own if applicable.) > Source/WebCore/bindings/js/JSDataTransferItemsCustom.cpp:14 > +bool JSDataTransferItems::deleteProperty(ExecState *exec, const Identifier& propertyName) > +{ > + return Base::deleteProperty(exec, propertyName); > +} This should be avoidable using a conditional in the IDL file. Though what does Chromium do in their custom deleteProperty? > Source/WebCore/platform/qt/ClipboardQt.cpp:369 > + RefPtr<DataTransferItemsQt> items = DataTransferItemsQt::create(this, m_frame->document()->scriptExecutionContext()); m_frame->document() is guaranteed to be non-null here? > Source/WebCore/platform/qt/ClipboardQt.h:90 > + Frame *m_frame; Coding style, Frame* m_frame; > Source/WebCore/platform/qt/DataTransferItemQt.cpp:29 > +#include <QtCore/QDebug> Remove. > Source/WebCore/platform/qt/DataTransferItemQt.cpp:104 > + } What about other MIME types? > Source/WebCore/platform/qt/DataTransferItemsQt.cpp:29 > +#include <QtCore/QDebug> Remove. > Source/WebCore/platform/qt/DataTransferItemsQt.cpp:55 > + qDebug() << m_items.size(); Remove. > Source/WebCore/platform/qt/DataTransferItemsQt.h:55 > + friend class ClipboardQt; Unnecessary friend declaration.

Created attachment 91710 [details] Patch Patch

Attachment 91710 [details] did not build on qt: Build output: http://queues.webkit.org/results/8524237

Created attachment 91712 [details] Patch Fix build error.

Comment on attachment 91712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91712&action=review Looks reasonable. > Source/WebCore/platform/qt/DataTransferItemQt.cpp:103 > + QByteArray rawData = mimeData->data(type()); > + data = QTextCodec::codecForName("UTF-16")->toUnicode(rawData); I would have probably just written this as one line, but this is fine too.

Comment on attachment 91712 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91712&action=review > Source/WebCore/dom/DataTransferItems.idl:38 > +#if defined(V8_BINDING) && V8_BINDING > CustomDeleteProperty, > +#endif actually... why is this needed here? Why haven't other ports needed/wanted this idl change?

(In reply to comment #9) > (From update of attachment 91712 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=91712&action=review > > > Source/WebCore/dom/DataTransferItems.idl:38 > > +#if defined(V8_BINDING) && V8_BINDING > > CustomDeleteProperty, > > +#endif > > actually... why is this needed here? Why haven't other ports needed/wanted this idl change? The other ports haven't needed this idl change because just Chromium has implemented DataTransferItems support :). And DataTransferItems are not accessed outside event handler so IMHO i think we do not need to do safe guards for deletions from cross domain access.

Comment on attachment 91712 [details] Patch OK. Thanks for the update.

The commit-queue encountered the following flaky tests while processing attachment 91712 [details]: http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: ap@webkit.org) http/tests/appcache/main-resource-hash.html bug 59902 (author: ap@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 91712 [details] Patch Clearing flags on attachment: 91712 Committed r85648: <http://trac.webkit.org/changeset/85648>

All reviewed patches have been landed. Closing bug.