|Summary:||Dropzone effects don't work in non-file documents|
|Product:||WebKit||Reporter:||Alexey Proskuryakov <ap>|
|Component:||WebCore Misc.||Assignee:||Alexey Proskuryakov <ap>|
|Severity:||Normal||CC:||benjamin, commit-queue, darin, esprehn+autocc, kangil.han, rniwa, sam|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Alexey Proskuryakov 2014-04-16 15:44:46 PDT
We have dropzone implemented (with prefix), and fast/events/dropzone-004.html verifies that setting effects works. But the test passes almost by accident - the code only works in local files, because it uses DataTransfer.files, and only local documents can read that while dragging.
Comment 1 Alexey Proskuryakov 2014-04-24 15:09:37 PDT
Created attachment 230110 [details] proposed fix
Comment 2 Darin Adler 2014-04-24 16:51:13 PDT
Comment on attachment 230110 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=230110&action=review r=me as is, although I have a few suggestions for future consideration > Source/WebCore/ChangeLog:10 > + 2. On Mac, sandbox doesn't prevent File object creation, as we alraedy have the access. typo: alraedy > Source/WebCore/dom/DataTransfer.cpp:187 > +bool DataTransfer::hasFileOfType(const String& type) Sure would be nice if this took a StringView instead of a const String&. > Source/WebCore/dom/DataTransfer.cpp:189 > + ASSERT_WITH_SECURITY_IMPLICATION(canReadTypes()); Why not just return false in this case instead of asserting? Are callers really in a good position to alway preflight correctly? > Source/WebCore/dom/DataTransfer.cpp:199 > +bool DataTransfer::hasStringOfType(const String& type) Sure would be nice if this took a StringView instead of a const String&. > Source/WebCore/dom/DataTransfer.cpp:201 > + ASSERT_WITH_SECURITY_IMPLICATION(canReadTypes()); Same question. > Source/WebCore/page/EventHandler.cpp:2068 > + return dataTransfer.hasFileOfType(keyword.substring(5)); If hasFileOfType took a StringView, then this would not need to allocate memory. Could just do StringView(keyword).substring(5) or have this function itself take a StringView. > Source/WebCore/page/EventHandler.cpp:2071 > + return dataTransfer.hasStringOfType(keyword.substring(7)); Ditto.
Comment 3 Alexey Proskuryakov 2014-04-24 17:37:18 PDT
Comment 4 Alexey Proskuryakov 2014-04-24 17:49:15 PDT
It would be nice to teach equalIgnoringCase about StringViews I guess - without that, switching to StringView was somewhat too annoying here. Committed <http://trac.webkit.org/r167784>.