ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.71k stars 3.24k forks source link

Pasting external HTML does not work with Slate example #1129

Closed dmitrizzle closed 6 years ago

dmitrizzle commented 7 years ago

Current behaviour

Right now pasting HTML into PasteHTML example works only if I was to copy HTML from Slate editor:

slate-to-slate-paste

However, if I am to paste HTML from any other source Chrome 60 throws an error:

html-to-slate-paste

build.prod.js:4902 Uncaught TypeError: Cannot read property 'find' of undefined
    at Object.deserialize (build.prod.js:4902)
    at Html.deserializeElement (build.prod.js:98382)
    at build.prod.js:98341
    at Array.forEach (<anonymous>)
    at Html.deserializeElements (build.prod.js:98340)
    at next (build.prod.js:98368)
    at Html.deserializeElement (build.prod.js:98402)
    at build.prod.js:98341
    at Array.forEach (<anonymous>)
    at Html.deserializeElements (build.prod.js:98340)

Expected behaviour

Pasting HTML from any source should work.

Context

I am using Chrome 60 on macOS 10.12.6, presumably Slate v0.24.1 - since that's the latest version posted (but I can't guarantee that's the same version as what slatejs.org uses in the above examples; above gifs were created shortly after v0.24.1 release was posted).

There's an issue #734 which addresses a similar problem but I don't think it's relevant anymore, since it described actions with much older versions of Slate. It was opened mentioning v0.19.16.

More context

v0.20.7 > v0.21.0 parse5 replaced cheerio but pasting caused JavaScript errors (see #948) which were fixed with #985. I’m not sure if pasting HTML has been tested for with the above PR, since the functionality never returned for pasting form the external sources.

My understanding

After DOMParser has replaced Cherio in Slate pasting HTML from external sources became broken. Most likely it's because there's no test to check for this case and a bug has slipped in.

ianstormtaylor commented 7 years ago

Hey @dmitrizzle thanks for opening this. Can you try with copying a more regularly structured piece of content? Like maybe just a section of a GitHub readme. It could be just a problem with certain bits of HTML in the copy.

travisperson commented 7 years ago

A simple example is pasting a div with an inline link.


This is a link inline with text

Source

<div>This is a <a href="#">link</a> in line with text</div>
dmitrizzle commented 7 years ago

It seems that pasting any text that isn't plaintext causes this error. Below are gifs of me pasting a large section of readme and Travis' simple link example:

Readme copy-paste:

paste-readme

Simple link copy-paste:

paste-link

Pasting text containing no HTML (works):

paste-plaintext

YurkaninRyan commented 7 years ago

Hey everyone,

This bug is due to to the fact that the upgrade makes it so we now get HTML-like elements. .find won't work for two reasons.

  1. node.attrs isn't a thing anymore, you need to do .attributes
  2. .find isn't going to work, because you get a named node map back from .attributes so you would have to turn it into an object.

Basically, the example just needs to be updated to handle the new nodes

ppvolto commented 6 years ago

Hey everyone, i current have the same Problem with Pasting and this error only triggers when i have All elements in the Editor selected for Replacement when one Element is not Fully Replaced then it works without a Problem...

ianstormtaylor commented 6 years ago

Hey @ppvolto can you open a new issue with a GIF explaining your use case? I believe this one was solved.