Bug 19893 – event.(dataTransfer|clipboardData).getData('text/html') (onpaste, ondrop)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

I asked about this in #webkit. I was having trouble getting event.dataTransfer.getData('text/html') in a drop event, apparently it is not available. Basically I'm left with either text/plain or parsing text/rtf (the former allowing for no handling of html structure at all, the latter being generally inaccurate for anything as simple as even a structure like <li><a>[text]<br>[text]</a></li> (the rtf format treats the <br> as closing the anchor and list item and opening a new set), making parsing rtf not very useful to try to discern the html structure of a drop.

[Mark Rowe (bdash)]

<rdar://problem/6055546>

[Tony Chang]

Created attachment 50877 [details]
Patch

[Tony Chang]

(In reply to comment #2)
> Created an attachment (id=50877) [details]
> Patch

It turns out that text/html wasn't returning anything because webkit on mac doesn't add text/html to the pasteboard (it uses WebArchive for html).  In fact, it was currently possible to copy from chrome mac and paste into safari and use getData('text/html')!

I originally tried to handle reading from LegacyWebArchive in the getData method, but it seemed non-trivial (the WebArchive code I needed was in WebKit/mac/).

I'm not sure which other platforms this will fail on, but I will try to fix win and chromium platforms after these tests land.

[Oliver Hunt]

Comment on attachment 50877 [details]
Patch

You need to test the effect of copy/pasting say
<div onload="alert(1)"></div>

I suspect the getData("text/html") query will currently produce a string that contains the event handler which is _probably_ unsafe.

[Darin Adler]

Comment on attachment 50877 [details]
Patch

This reverses a change we made in 2004, <http://trac.webkit.org/changeset/6793>.

The symptom of that bug was:

- Copy some text from any web page.
- Try to paste into an Excel spreadsheet.
- Get error message: "Microsoft Excel cannot paste the data."

Please don't make this change to WebKit unless you have some other solution for that problem. We want people to be able to copy in Safari and paste into Excel.

[Tony Chang]

(In reply to comment #5)
> Please don't make this change to WebKit unless you have some other solution for
> that problem. We want people to be able to copy in Safari and paste into Excel.

Thanks for letting me know about the history of this behavior.

I am able to copy and paste with this patch into Excel 2008.  Which version of Excel do we want to make sure still works?  It looks like the versions before 2008 were 2004 and 2001.

(In reply to comment #4)
> (From update of attachment 50877 [details])
> You need to test the effect of copy/pasting say
> <div onload="alert(1)"></div>
> 
> I suspect the getData("text/html") query will currently produce a string that
> contains the event handler which is _probably_ unsafe.

Yes, the string will contain the event handler, which the website could do something unsafe with (say, assign to innerHTML).  I think it's up to the site to sanitize the string or just use text/plain.  Lots of stuff gets removed by webkit when pasted (css, event handlers, smart handling of some formatting), but perhaps the site has some other good reason to want to use the original html.

Julie, do you have some example uses cases of wanting text/html?

[Julie Parent]

Tony - Nearly every product team at Google that uses rich text has asked for this.  They currently implement it in a number of hacky ways, like putting the user's cursor in another contentEditable region so the paste goes there, then grabbing those contents, sanitizing them, and inserting back into the original element.  Or letting the paste go through, and processing the changes afterwards, which can make the contents change out from under the user.

The use cases is to be able to change the pasted contents before it is pasted into the final destination, for any number of reasons: some want to strip out certain types of content (for example, they don't want to allow images in their rich text region), others want to clean up the HTML (for example, convert <strong> to <b> because execCommand('bold') can't unbold <strong> tags).

[Tony Chang]

(In reply to comment #7)
> The use cases is to be able to change the pasted contents before it is pasted
> into the final destination, for any number of reasons: some want to strip out
> certain types of content (for example, they don't want to allow images in their
> rich text region), others want to clean up the HTML (for example, convert
> <strong> to <b> because execCommand('bold') can't unbold <strong> tags).

As far as I can tell, clipboardData.setData doesn't change the clipboard contents during onpaste (it's probably too late, you have to do it in onbeforepaste).  You can however cancel the paste and manually handle the paste into the DOM.  Be aware that at this point, you lose all the code in webkit's for handling the paste (i.e., removing the extra style attributes, removing event handlers, special merge handling that happens for lists, tables, etc).

I guess the main benefit of this is that you can work around webkit bugs without having to wait for new browser releases, although it'll be a lot of duplicated effort with what the browser does.

[Ryosuke Niwa]

It seems like we should fix this bug given Tony's new research. It's not great that getData('text/html') isn't available.

[Ojan Vafai]

Bug 104179 is the similar bug for setData.

[Daniel Cheng]

Is this still a bug on just the Mac port of WebKit or is it a problem for other platforms as well? I'm fairly certain that the WebKit port of Chromium supports getData() and setData() for HTML.

[Daniel Cheng]

(In reply to comment #11)
> Is this still a bug on just the Mac port of WebKit or is it a problem for other platforms as well? I'm fairly certain that the WebKit port of Chromium supports getData() and setData() for HTML.

(I mean the Chromium port of WebKit of course...)

[Ojan Vafai]

Oh, I suppose this bug as listed is fixed. I couldn't get at the clipboardData from the copy event though, which seems unnecessarily different.

[Piotr Jasiun]

I also met this issue working on drag and drop in the contenteditable element. I am not able to get html data when user drop same html into the contenteditable.

[Piotrek Koszuliński (Reinmar)]

For the first time, I've seen this ticket about 3-4 years ago and even then it was very sad that Webkit still didn't provide HTML on paste. I'm now working on a totally new version of CKEditor and, in the meantime, I totally forgot about this problem (I kinda optimistically believed that it must all work by now). Imagine my surprise when I found out that it's still open :(.

Any chance that this gets under your radar again? I see (https://bugs.webkit.org/show_bug.cgi?id=19893#c9) that it's been already accepted by Ryosuke.

[Chris Dumez]

(In reply to comment #15)
> For the first time, I've seen this ticket about 3-4 years ago and even then
> it was very sad that Webkit still didn't provide HTML on paste. I'm now
> working on a totally new version of CKEditor and, in the meantime, I totally
> forgot about this problem (I kinda optimistically believed that it must all
> work by now). Imagine my surprise when I found out that it's still open :(.
> 
> Any chance that this gets under your radar again? I see
> (https://bugs.webkit.org/show_bug.cgi?id=19893#c9) that it's been already
> accepted by Ryosuke.

@rniwa / @darin: Any opposition to reviving this assuming fixing this does not break copy/pasting to Excel anymore as indicated by Tony in [1]?

[1] https://bugs.webkit.org/show_bug.cgi?id=19893#c6

[Darin Adler]

No objection. My objection was reversing the change without reading the change history and figuring out what kind of testing was needed.

[Chris Dumez]

(In reply to comment #17)
> No objection. My objection was reversing the change without reading the
> change history and figuring out what kind of testing was needed.

Using Excel 2011 Mac, copying from Safari and pasting to Excel still works with the patch. There is a behavior change though, it now pastes as HTML instead of simple text. Excel supports displaying HTML so this is not an issue.

[Darin Adler]

Sounds fine.

[Chris Dumez]

Created attachment 292643 [details]
Patch

[Alexey Proskuryakov]

While you are at it, could you please also test other Office products?

[Ryosuke Niwa]

Comment on attachment 292643 [details]
Patch

r=me assuming this doesn't let web content insert an arbitrary HTML content into the system's pasteboard by setData, or we don't start allowing getData to access text/html type in the pasteboard both of which pose privacy and security problems.

[Chris Dumez]

(In reply to comment #22)
> Comment on attachment 292643 [details]
> Patch
> 
> or we don't start allowing getData
> to access text/html type in the pasteboard both of which pose privacy and
> security problems.

Isn't this exactly what this patch is doing though? (allowing JS to access call getData('text/html') to get it from the pasteboard)

[Chris Dumez]

(In reply to comment #23)
> (In reply to comment #22)
> > Comment on attachment 292643 [details]
> > Patch
> > 
> > or we don't start allowing getData
> > to access text/html type in the pasteboard both of which pose privacy and
> > security problems.
> 
> Isn't this exactly what this patch is doing though? (allowing JS to access
> call getData('text/html') to get it from the pasteboard)

To be clear though, this patch merely add HTML to the pasteboard (in addition to what WebKit already added, such as RTFD). As a result, getData('text/html') will be able to read the HTML we now write. Therefore, I do not think I am exposing more information, I am merely providing an extra format on the pasteboard.

[Chris Dumez]

(In reply to comment #21)
> While you are at it, could you please also test other Office products?

I will do so before landing (FYI, I also tried Apple's Keynote and it worked).

[Chris Dumez]

(In reply to comment #25)
> (In reply to comment #21)
> > While you are at it, could you please also test other Office products?
> 
> I will do so before landing (FYI, I also tried Apple's Keynote and it
> worked).

Works in Powerpoint 2011 and Word 2011 too.

[WebKit Commit Bot]

Comment on attachment 292643 [details]
Patch

Clearing flags on attachment: 292643

Committed r207797: <http://trac.webkit.org/changeset/207797>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Piotrek Koszuliński (Reinmar)]

Thanks for taking care of this so quickly :)!

Does it mean that the following is going to work now?

1. Open https://jsfiddle.net/kamxex2q/3/
2. Copy some text from the editable element. (On #paste we write some content to the clipboard)
3. Paste it.

Expected: 'x <strong>y</strong z' was pasted.
Actual: the text that was selected was pasted.

Or does it require a new ticket?

[Chris Dumez]

(In reply to comment #29)
> Thanks for taking care of this so quickly :)!
> 
> Does it mean that the following is going to work now?
> 
> 1. Open https://jsfiddle.net/kamxex2q/3/
> 2. Copy some text from the editable element. (On #paste we write some
> content to the clipboard)
> 3. Paste it.
> 
> Expected: 'x <strong>y</strong z' was pasted.
> Actual: the text that was selected was pasted.
> 
> Or does it require a new ticket?

This does not seem to work in *any* browser I tested?

[Chris Dumez]

(In reply to comment #30)
> (In reply to comment #29)
> > Thanks for taking care of this so quickly :)!
> > 
> > Does it mean that the following is going to work now?
> > 
> > 1. Open https://jsfiddle.net/kamxex2q/3/
> > 2. Copy some text from the editable element. (On #paste we write some
> > content to the clipboard)
> > 3. Paste it.
> > 
> > Expected: 'x <strong>y</strong z' was pasted.
> > Actual: the text that was selected was pasted.
> > 
> > Or does it require a new ticket?
> 
> This does not seem to work in *any* browser I tested?

I believe you need to cancel the event if you want your call to setData not to be ignored. Like so:
https://jsfiddle.net/kamxex2q/4/

In latest WebKit, this pastes as "x y z" with "y" in bold, So it looks correct to me.

[Chris Dumez]

(In reply to comment #31)
> (In reply to comment #30)
> > (In reply to comment #29)
> > > Thanks for taking care of this so quickly :)!
> > > 
> > > Does it mean that the following is going to work now?
> > > 
> > > 1. Open https://jsfiddle.net/kamxex2q/3/
> > > 2. Copy some text from the editable element. (On #paste we write some
> > > content to the clipboard)
> > > 3. Paste it.
> > > 
> > > Expected: 'x <strong>y</strong z' was pasted.
> > > Actual: the text that was selected was pasted.
> > > 
> > > Or does it require a new ticket?
> > 
> > This does not seem to work in *any* browser I tested?
> 
> I believe you need to cancel the event if you want your call to setData not
> to be ignored. Like so:
> https://jsfiddle.net/kamxex2q/4/
> 
> In latest WebKit, this pastes as "x y z" with "y" in bold, So it looks
> correct to me.

This is explained in this section of the specification:
- https://www.w3.org/TR/clipboard-apis/#the-copy-action

[Piotrek Koszuliński (Reinmar)]

Right! Thanks :D It's been so long since the last time that I forgot about preventing the default action. I only new it was working in CKEditor 4, but I couldn't find why :D.

Anyway, I confirm that I can set the data in Safari 10.0.1 and that this data is used on paste.

Playground if anyone's interested: https://jsfiddle.net/kamxex2q/5/.

Thanks!

Good job with fixing it on a desktop version. It still doesn't work on ios though, as it does not include text/html. It includes type `public.html` with no content. Only types `text/plain` and `public.rtf` have some content while copying and pasting formatted text within Safari.