Bug 170637 – REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an empty string when dragging an image

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

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.

[Wenson Hsieh]

<rdar://problem/31347248>

[Brady Eidson]

*** Bug 174259 has been marked as a duplicate of this bug. ***

[Brady Eidson]

BTW this broke in https://trac.webkit.org/changeset/210287/webkit

[Wenson Hsieh]

Created attachment 319243 [details]
First pass

[Wenson Hsieh]

Created attachment 319245 [details]
Fix iOS 10 build

[Ryosuke Niwa]

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.

[Wenson Hsieh]

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.

[Wenson Hsieh]

Created attachment 319335 [details]
Patch for landing

[Wenson Hsieh]

Created attachment 319337 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 319337 [details]
Patch for landing

Clearing flags on attachment: 319337

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