Open robbertbrak opened 7 years ago
Thanks for reporting this @robbertbrak - we have talked about improving the copy/paste feature and this seems potentially related. I'll post here when I get an update about that - @mitermayer might have a more detailed response though.
@flarnie: I just implemented a fix for this in our fork of draft. https://github.com/textioHQ/draft-js/commit/86b8a4194f0e46f1bf9aa44467bb464da4bda4f6
It's something I'd like to consider getting into the core part of draft. The high level idea is to move focus during the paste event to a temporary div, and grabbing the HTML from there.
I had to work around a few bugs to implement this: https://github.com/facebook/react/issues/8909 https://github.com/facebook/fbjs/issues/190
I also ran into some IE/Edge specific quirks: The pasteTrap div has to be present and not hidden or display:none at the time of focus. The event has to be either captured, or bubbling with eventTarget no higher than the contenteditable div.
Otherwise it seems to work pretty well!
The reason I used a temporary div is to avoid any flashing behavior where the DOM redraws without the correct content
@AnilRedshift great to hear that other people are working on this as well! My own solution, similar to yours, is implemented in https://github.com/robbertbrak/draft-js/commit/3aee43f060a745d8ff379f7b698f1d7e10d5e06b.
I ran in the same issues that you mentioned. In addition, and just for reference, I noticed that Draft adds the following CSS to the contenteditable div: -webkit-user-modify: read-write-plaintext-only
(see https://github.com/facebook/draft-js/blob/master/src/component/base/DraftEditor.css#L17). I'm not sure why that attribute is necessary for Draft, but if it's there, it prevents Edge from pasting HTML into the element.
@robbertbrak: Yes the pattern is pretty similar. You are able to get away with creating the clone div in the context of the paste handler and having it work? My testing was unable to get that to happen.
Also doesNotSupportHTMLFromClipboard should include safari < 10.
Lastly, does your fix work for Edge? I just want to make sure I'm not crazy. I was unable to get this working on Edge without changing the paste handler to not be delegated from the root #document
Thanks!
Perhaps we can start a slack channel and hash out a PR, and also for anyone else who has looked into this space.
There's a similar bug that we've found which is that if you enable the group policy to disable programmatic clipboard access, then the vanilla draft-js no longer can paste correctly.
This approach has the potential to fix that as well.
@AnilRedshift My fix does work for Edge, as you can ascertain for yourself.
As for Safari: we did some experiments on Safari 9 and 10 and found that its behaviour is slightly different from IE/Edge, although the same technique can be used to solve its problems. In Safari, it turns out that HTML on the clipboard is supported, it's just not always present:
In light of these inconsistencies, the best results are produced when using the technique I used in my commit. So yeah, doesNotSupportHTMLFromClipboard
should also include Safari (including version 10, according to the results we found).
Up. Is there a solution to this problem?
@robbertbrak or @AnilRedshift have you tried submitting a PR to get this fixed? This is an annoying issue and I would rather no fork draft-js if possible...
@ironosity I haven't, but you are of course welcome to submit a PR of your own, based on our forks!
@ironosity I am slowly moving the textioHQ fork forward so that I can make PRs for features like this one. Our implementation of copy/paste, though, may need a bit of work to become generic enough for Draft itself since it relies on a fair number of tricks and depends on some specific CSS.
I'm going to try and look into this. Thanks to those who have posted - please let me know if someone has a PR ready for this, otherwise will be debugging it in my spare time.
@flarnie I'm happy to help out, the biggest concern I had was the fact that I was adding a new DOM element (the paste trap) to the draft editor.
Particular care needs to be taken with both IE11 and Edge to work around their various issues. It's been awhile since I looked at this and perhaps there's an easier way than my above PR.
Let me know if you need my contact info.
@flarnie Great to see this issue being activated again. Unfortunately I don't have a PR ready, as we're somewhat behind on our fork. However, you might get some inspiration from looking at our implementation of editOnPaste
: https://github.com/robbertbrak/draft-js/blob/0.9-stable-itrp/src/component/handlers/edit/editOnPaste.js.
We have used this implementation successfully in production for quite some time now. It works well on the three browsers that we consider problematic for pasting HTML: IE11, Edge and Safari.
@flarnie, I am also very happy to help. The paste mechanism we have had in place that @AnilRedshift mentioned has been working well for us but it is a significant departure from what Draft currently does.
When pasting HTML in IE, only plain text is preserved.
To reproduce:
IE does not expose
text/html
on the clipboardData, so this may be difficult to fix. There are working workarounds, though, such as the one described on http://stackoverflow.com/a/6804718/815176. This workaround involves:Would such a workaround be considered for inclusion in the framework?