Bug 19893 - event.(dataTransfer|clipboardData).getData('text/html') (onpaste, ondrop)
Summary: event.(dataTransfer|clipboardData).getData('text/html') (onpaste, ondrop)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Enhancement
Assignee: Chris Dumez
URL:
Keywords: InRadar, ReviewedForRadar
Depends on:
Blocks:
 
Reported: 2008-07-04 01:22 PDT by gnosis
Modified: 2016-10-25 09:25 PDT (History)
15 users (show)

See Also:


Attachments
Patch (12.37 KB, patch)
2010-03-16 23:34 PDT, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (11.22 KB, patch)
2016-10-24 12:45 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description gnosis 2008-07-04 01:22:29 PDT
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.
Comment 1 Mark Rowe (bdash) 2008-07-04 19:55:56 PDT
<rdar://problem/6055546>
Comment 2 Tony Chang 2010-03-16 23:34:46 PDT
Created attachment 50877 [details]
Patch
Comment 3 Tony Chang 2010-03-16 23:40:20 PDT
(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.
Comment 4 Oliver Hunt 2010-03-17 01:27:14 PDT
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.
Comment 5 Darin Adler 2010-03-17 13:58:40 PDT
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.
Comment 6 Tony Chang 2010-03-17 18:34:54 PDT
(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?
Comment 7 Julie Parent 2010-03-18 15:28:24 PDT
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).
Comment 8 Tony Chang 2010-03-18 17:49:07 PDT
(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.
Comment 9 Ryosuke Niwa 2012-05-02 10:12:45 PDT
It seems like we should fix this bug given Tony's new research. It's not great that getData('text/html') isn't available.
Comment 10 Ojan Vafai 2012-12-05 15:33:49 PST
Bug 104179 is the similar bug for setData.
Comment 11 Daniel Cheng 2012-12-05 15:43:02 PST
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.
Comment 12 Daniel Cheng 2012-12-05 15:43:23 PST
(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...)
Comment 13 Ojan Vafai 2012-12-05 15:53:22 PST
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.
Comment 14 Piotr Jasiun 2014-06-23 08:33:29 PDT
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.
Comment 15 Piotrek Koszuliński (Reinmar) 2016-10-24 09:34:09 PDT
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.
Comment 16 Chris Dumez 2016-10-24 09:46:56 PDT
(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
Comment 17 Darin Adler 2016-10-24 10:14:37 PDT
No objection. My objection was reversing the change without reading the change history and figuring out what kind of testing was needed.
Comment 18 Chris Dumez 2016-10-24 10:55:35 PDT
(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.
Comment 19 Darin Adler 2016-10-24 12:28:33 PDT
Sounds fine.
Comment 20 Chris Dumez 2016-10-24 12:45:49 PDT
Created attachment 292643 [details]
Patch
Comment 21 Alexey Proskuryakov 2016-10-24 15:49:46 PDT
While you are at it, could you please also test other Office products?
Comment 22 Ryosuke Niwa 2016-10-24 16:45:47 PDT
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.
Comment 23 Chris Dumez 2016-10-24 16:58:11 PDT
(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)
Comment 24 Chris Dumez 2016-10-24 17:03:53 PDT
(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.
Comment 25 Chris Dumez 2016-10-24 17:04:41 PDT
(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).
Comment 26 Chris Dumez 2016-10-24 18:25:57 PDT
(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.
Comment 27 WebKit Commit Bot 2016-10-24 18:51:31 PDT
Comment on attachment 292643 [details]
Patch

Clearing flags on attachment: 292643

Committed r207797: <http://trac.webkit.org/changeset/207797>
Comment 28 WebKit Commit Bot 2016-10-24 18:51:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 29 Piotrek Koszuliński (Reinmar) 2016-10-25 00:42:21 PDT
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?
Comment 30 Chris Dumez 2016-10-25 09:10:31 PDT
(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?
Comment 31 Chris Dumez 2016-10-25 09:15:32 PDT
(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.
Comment 32 Chris Dumez 2016-10-25 09:24:36 PDT
(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
Comment 33 Piotrek Koszuliński (Reinmar) 2016-10-25 09:25:23 PDT
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!