Bug 170637 - REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an empty string when dragging an image
Summary: REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an em...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
: 174259 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-04-07 20:09 PDT by Wenson Hsieh
Modified: 2017-09-05 07:32 PDT (History)
13 users (show)

See Also:


Attachments
Reduced test case (84.26 KB, application/zip)
2017-04-07 20:09 PDT, Wenson Hsieh
no flags Details
First pass (13.25 KB, patch)
2017-08-28 21:48 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Fix iOS 10 build (13.52 KB, patch)
2017-08-28 22:03 PDT, Wenson Hsieh
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (10.55 KB, patch)
2017-08-29 21:18 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for landing (10.59 KB, patch)
2017-08-29 21:39 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2017-04-07 20:09:59 PDT
Created attachment 306564 [details]
Reduced test case

On drop, event.dataTransfer.getData("text") returns an empty string when dragging an image

See the attached test case.
1. Drag the img into the blue bordered area
2. Observe that the text reads "The text data is ''" when it should be "The text data is 'https://www.apple.com'"

Regression introduced in Safari 10.1.
Comment 1 Wenson Hsieh 2017-04-07 20:12:41 PDT
<rdar://problem/31347248>
Comment 2 Brady Eidson 2017-07-07 17:12:10 PDT
*** Bug 174259 has been marked as a duplicate of this bug. ***
Comment 3 Brady Eidson 2017-07-10 07:08:52 PDT
BTW this broke in https://trac.webkit.org/changeset/210287/webkit
Comment 4 Wenson Hsieh 2017-08-28 21:48:35 PDT
Created attachment 319243 [details]
First pass
Comment 5 Wenson Hsieh 2017-08-28 22:03:32 PDT
Created attachment 319245 [details]
Fix iOS 10 build
Comment 6 Ryosuke Niwa 2017-08-29 17:45:15 PDT
Comment on attachment 319245 [details]
Fix iOS 10 build

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

> Source/WebCore/dom/DataTransfer.h:99
> +    enum class SuppressDataAccessReason { None, ContainsFileURL };

I'm not certain this enum needs to say "SuppressDataAccess".
I think it's probably sufficient to have an enum like `enum class ContainFiles { Yes, No }`.
It's also okay to just use boolean m_containsFiles if the only place where we set this to true was simply calling a method named containsFiles().

> Source/WebCore/platform/DragData.cpp:62
> +bool DragData::containsFilePaths() const
> +{
> +    Vector<String> filenames;
> +    asFilenames(filenames);
> +    return filenames.size();
> +}
> +

Can't we do this check inside DataTransfer object since it also has access to Pasteboard::getFilenames()?

> LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html:1
> +<style>

Missing DOCTYPE.
Comment 7 Wenson Hsieh 2017-08-29 18:20:23 PDT
Comment on attachment 319245 [details]
Fix iOS 10 build

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

>> Source/WebCore/dom/DataTransfer.h:99
>> +    enum class SuppressDataAccessReason { None, ContainsFileURL };
> 
> I'm not certain this enum needs to say "SuppressDataAccess".
> I think it's probably sufficient to have an enum like `enum class ContainFiles { Yes, No }`.
> It's also okay to just use boolean m_containsFiles if the only place where we set this to true was simply calling a method named containsFiles().

Sounds good -- fixed.

>> Source/WebCore/platform/DragData.cpp:62
>> +
> 
> Can't we do this check inside DataTransfer object since it also has access to Pasteboard::getFilenames()?

Yes -- I'll remove this DragData helper then.

>> LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html:1
>> +<style>
> 
> Missing DOCTYPE.

Added.
Comment 8 Wenson Hsieh 2017-08-29 21:18:22 PDT
Created attachment 319335 [details]
Patch for landing
Comment 9 Wenson Hsieh 2017-08-29 21:39:54 PDT
Created attachment 319337 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2017-08-29 22:09:27 PDT
Comment on attachment 319337 [details]
Patch for landing

Clearing flags on attachment: 319337

Committed r221343: <http://trac.webkit.org/changeset/221343>