search

bmcmahen / react-wysiwyg

retain some control over contenteditable input
169 stars 37 forks source link

HTML entities appearing in content #33

Open ygravrand opened 9 years ago

ygravrand commented 9 years ago

In index.js at line 330, text content is html escaped. Could you elaborate on why it was required? It seems to lead to a bug with your example when you enter a double quote character, for example, just before the limit: see attached gif below. test

ygravrand commented 9 years ago

Another way to reproduce: simply enter a single quote (') --> creates a ' probably because of the hashtag in the entity.

bmcmahen commented 9 years ago

Since the module renders the input content with dangerouslySetInnerHTML the content is escaped to avoid things like <b> from actually rendering as a dom element. Thanks for pointing out the unintended consequences. I'm starting to think the general mechanism of using innerHTML isn't really suitable.

ygravrand commented 9 years ago

Got it.

I figured out a way to make it work in my own examples by modifying your line 330 to pass e.target and escaping HTML of its textContent afterwards, once in output, once in overflow.

I would suggest an API change for onChange to pass a target element as third parameter instead of innerHTML. That way, simple examples could keep on using (escaped and thus safe) textContent, while more advanced ones could use target to extract either innerHTML or textContent depending on the use case.

See PR #37 for a suggestion of implementation. I commented out the regex section and did not add unit tests for this particular issue...

BTW it seems that it also fixes an other bug I'd seen: before, you could enter this text: <li>item</li>, copy it, paste it and it would create a real bullet!

bmcmahen commented 9 years ago

That's a really clever idea. I'll give it a more detailed look when I get some time, and do a major semver bump when I've tested it a bit. Thanks!