[webkit-reviews] review granted: [Bug 131770] Dropzone effects don't work in non-file documents : [Attachment 230110] proposed fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 24 16:50:51 PDT 2014
Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 131770: Dropzone effects don't work in non-file documents
https://bugs.webkit.org/show_bug.cgi?id=131770
Attachment 230110: proposed fix
https://bugs.webkit.org/attachment.cgi?id=230110&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
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.
More information about the webkit-reviews
mailing list