Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DataTransfer's setData, getData, clearData should strip whitespaces on type #2946

Open
rniwa opened this issue Aug 19, 2017 · 7 comments
Open

DataTransfer's setData, getData, clearData should strip whitespaces on type #2946

rniwa opened this issue Aug 19, 2017 · 7 comments
Labels
topic: drag and drop

Comments

@rniwa
Copy link

rniwa commented Aug 19, 2017

it looks some ports of WebKit and Blink strips away whitespaces around the type specified to DataTranfer's setData, getData, and clearData methods. The comment indicates this was needed for IE compatibility.

Since iOS WebKit seems to have always had this behavior, I'm worried that not stripping whitespaces would not be backwards compatible.

Given the options, I think we should change the spec to strip whitespaces around the type name.

It would be great if someone could check the behavior of IE/Edge on Windows as well.
@zcorpan
Copy link
Member

zcorpan commented Aug 21, 2017

Do you have a test case or demo already?

@rniwa
Copy link
Author

rniwa commented Aug 22, 2017

I have a very simple test case added in https://bugs.webkit.org/show_bug.cgi?id=175810

@zcorpan
Copy link
Member

zcorpan commented Aug 22, 2017

OK I've run a modified test with live dom viewer:

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5318

In EdgeHTML 15 (via browserstack) I get

error: Element not found.
 on line 15
Test case EdgeHTML Blink Gecko WebKit
clipboardData.setData(" text/PLAIN ", "hello"); error true false false
clipboardData.setData(" tEXT/pLaIN", "world"); error true false false
clipboardData.setData("text/plain; charset=utf-8", "hello"); error true false true

@zcorpan
Copy link
Member

zcorpan commented Aug 22, 2017

It seems Edge throws an exception if there's leading whitespace in setData. With leading whitespace removed, EdgeHTML gives

log: true
log: false
log: true

@rniwa
Copy link
Author

rniwa commented Aug 22, 2017

As confusing as it sounds, iOS WebKit behaves differently from macOS WebKit. On iOS, we always stripped leading & trailing whitespaces. Given that I suspect there could be mobile content out there that depends on this behavior.

hubot pushed a commit to WebKit/WebKit-http that referenced this issue Aug 23, 2017
Consolidate the code to normalize MIME type in DataTransfer
3a241fb 
https://bugs.webkit.org/show_bug.cgi?id=175810

Reviewed by Wenson Hsieh.

Source/WebCore:

Factored out the code to convert MIME type to lowercase after stripping whitespace,
and treat "text" as "text/plain" and "url" as "text/uri-list".

Specifications:
https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-getdata-2
https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-setdata-2
https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-cleardata-2

Stripping of whitespace only happens in WebKit/Blink but it's probably required for compatbility.
Spec bug: whatwg/html#2946

Test: editing/pasteboard/datatransfer-getdata-plaintext.html

* dom/DataTransfer.cpp:
(WebCore::normalizeType):
(WebCore::DataTransfer::clearData):
(WebCore::DataTransfer::getData const):
(WebCore::DataTransfer::setData):
* platform/gtk/PasteboardGtk.cpp:
(WebCore::selectionDataTypeFromHTMLClipboardType):
* platform/ios/PasteboardIOS.mm:
(WebCore::cocoaTypeFromHTMLClipboardType):
* platform/mac/PasteboardMac.mm:
(WebCore::cocoaTypeFromHTMLClipboardType):
* platform/win/PasteboardWin.cpp:
(WebCore::clipboardTypeFromMIMEType):

LayoutTests:

Added a regression test. Some test cases were failing on some platforms.

* editing/pasteboard/datatransfer-getdata-plaintext-expected.txt: Added.
* editing/pasteboard/datatransfer-getdata-plaintext.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@221063 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@annevk
Copy link
Member

annevk commented Aug 23, 2017

FWIW, stripping whitespace around the MIME type seems reasonable to me. We'll also be doing that for data: URL MIME types. See whatwg/fetch#579.

@annevk
Copy link
Member

annevk commented Aug 23, 2017 •
edited
Loading

Any tests should make sure to test all ASCII whitespace: U+0009, U+000A, U+000C, U+000D, and U+0020.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: drag and drop
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zcorpan @rniwa @annevk and others