Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using <input> inside a contenteditable="false" is broken in Firefox #1744

Closed
zhouzi opened this issue Mar 30, 2018 · 24 comments
Closed

Using <input> inside a contenteditable="false" is broken in Firefox #1744

zhouzi opened this issue Mar 30, 2018 · 24 comments

Comments

@zhouzi
Copy link
Contributor

zhouzi commented Mar 30, 2018

We noticed a bug in Firefox, rendering a node as follows is broken:

<div contenteditable="false">
  <input type="text" />
</div>

There are two things to note:

  1. Left/right arrow keys don't move the caret
  2. Pressing a character key (or backspace) throws an error:

Permission denied to access property "nodeType"

I created a repository reproducing the error with Slate and a minimal setup:
http://gabinaureche.com/slate-firefox-bug/

The error comes from isInEditor which tries to read window.getSelection().anchorNode.nodeType but anchorNode is "Restricted".

This bug is reproducible without Slate, here's a pretty simple scenario:
https://jsfiddle.net/hsqebsk2/1/

I filed an issue to Firefox:
https://bugzilla.mozilla.org/show_bug.cgi?id=1449936

Probably related to #843

@cameracker
Copy link
Collaborator

cameracker commented Mar 30, 2018

This can be worked around by using


<div contenteditable>
    <div contenteditable="false">
        <input contenteditable type="text" value="Some text content">
    </div>
</div>

Or by using the componentDidThrow() react event handler to keep these errors from crashing the editor.

I also don't think this is a Slate bug.

@zhouzi
Copy link
Contributor Author

zhouzi commented Apr 18, 2018

Thanks for the suggestion @CameronAckermanSEL 👍

I gave it a try and it does work outside of Slate: https://jsfiddle.net/hsqebsk2/4/

But doesn't in the context of Slate 😕

@nkoder
Copy link

nkoder commented Jun 21, 2018

not sure whether it will help anyone, but I have similar issue, and here is my code sample to reproduce it on Firefox: https://codepen.io/nkoder/pen/pKLQWz

@Iuriy-Budnikov
Copy link

I have the same issue +1

@erquhart
Copy link
Contributor

erquhart commented Sep 4, 2018

Just wanted to outline how critical this is, as I'm not certain that's clear.

This bug means projects that support Firefox, which is most projects, can't use inputs in Slate, as Slate controls the contenteditable attributes within itself and I think they're always false for void nodes.

Best repro of this bug is in Slate's example site - try editing the YouTube video url in firefox: https://www.slatejs.org/#/embeds

I'm working on tracking down the commit where this broke - we ran into it by upgrading slate from ^0.30 to ^0.34 and slate-react from 0.10.11 to 0.12.9.

@Soreine
Copy link
Collaborator

Soreine commented Sep 11, 2018

Either we try to make Slate work around the bug, or we find a way to pressure Firefox into conforming to other browser's behavior. We have filed this bug to Mozilla, but it is not getting much attention...

@ianstormtaylor
Copy link
Owner

I'm open to not having this be the structure we use in Slate, I think the tricky part is finding a DOM structure for voids that satisfies the many different bugs/behaviors in browsers. It's hard to keep track of all of the pitfalls, which is why we need more inline comments/docs for this kind of thing probably. There was probably a good reason it was switched to this (now problematic) behavior in the first place.

@Soreine
Copy link
Collaborator

Soreine commented Sep 19, 2018

Thanks to #2178 , Firefox will no longer crashes. However, the inputs are nearly unusable, because you cannot move the cursor freely inside it, even by clicking somewhere.

Here is another error thrown when focusing the input:

Error: Permission denied to access property "window" index.js:25
isWindow                     index.js:25
getWindow                    index.js:39
findRange                    slate-react.es.js:1409
onEvent                      slate-react.es.js:2029
Content/</_this[handler]     slate-react.es.js:1936

@cameracker
Copy link
Collaborator

cameracker commented Sep 19, 2018

I imagine this might sound kinda dumb, but maybe the thing to do is to just use a simple inline or block that is styled to look like an input as a work around? I get that this is really frustrating and might get people by until mozilla addresses this issue. Though that does create issues because of our lack of atomic blocks

@Iuriy-Budnikov
Copy link

Any updates?

@dmitrizzle
Copy link

dmitrizzle commented Sep 29, 2018

This isn't just a FireFox issue.

The inputs inside void blocks do not work on any of the browsers I've tried on macOS. This example: https://www.slatejs.org/#/embeds does not work (I can not change the contents of the input element on any of my browsers).

@dmitrizzle
Copy link

Hi @ianstormtaylor, it seems that this issue and #1926 are the same, perhaps we could merge them?

I'm also hoping that participants here would be willing to test the embeds example in other browsers and confirm broken behaviour. Would be more than happy to contribute if I knew where to start; this is a blocker for my project and, presumably, any project that has <input /> elements inside void blocks with the newer Slate releases.

@Iuriy-Budnikov
Copy link

Iuriy-Budnikov commented Oct 1, 2018

I'll be able to help as well. I've the same blocker.

@skogsmaskin
Copy link
Collaborator

I think this might be related #2116

@aliblackwell
Copy link

Happy to help too! I've read the Firefox bug report and it seems to me we're reporting perfectly reasonable behaviour as a bug. It's Chrome and other browsers that are just being more forgiving. @ianstormtaylor could you guide us to the part of the source code that requires this nested structure? #2178 appears to swallow the error. What would it look like never having an input inside a contenteditable=false i.e. making them siblings? I appreciate nesting is a major part of Slate but surely we could do this using siblings?

@gnestor
Copy link

gnestor commented Oct 13, 2018

Related to #191 too

@gnestor
Copy link

gnestor commented Oct 16, 2018

In #2261, @klis87 points out that onChange events were firing for inputs in a void node in slate 0.41 and now they aren't in 0.42. This may be a separate issue than the Firefox one being discussed here as it affects all browsers as far as I can tell.

I'd be happy to hunt this regression down but where should I start looking?

@gnestor
Copy link

gnestor commented Oct 16, 2018

I was able to hunt this down in #2198.

Simply reverting this commit fixes the embed example and allows elements such as input elements within a void node to receives change, keydown, click, etc. events.

#2198 was intended to make copying behavior consistent for inline void with text and inline void without text. However, it mistakes void blocks as being inTheEditor.

@uipoptart Can this be implemented differently?

@uipoptart
Copy link
Contributor

Hey @gnestor, sorry that change is causing trouble for you. Your use case is another reason why #2072 would be great for the entire community. We all have complicated use cases and it is hard to keep them from colliding. Short of implementing #2072 we could add a single prop to the inline allowing it to opt in/out of being considered inTheEditor or maybe we could look to see if any children are their own inputs who'd want to be considered first.
Currently, I don't have time approved to work on writing a commit to bridge both behaviors but I am happy to help brainstorm solutions that could solve both our problems. Thanks for tagging me!

@ianstormtaylor
Copy link
Owner

I've rolled back the regression that made inputs inside voids unable to be focused in all browsers, so this issue can remain for the Firefox case.

@CodePlayer
Copy link

I have the same issue +1.
I'm a new comer. However, the slate is still considered buggy.

@zhouzi
Copy link
Contributor Author

zhouzi commented Mar 13, 2019

The bug report on Firefox's side was just marked as resolved. I wasn't able to reproduce the bug in the two following scenarios, with Firefox 65:

So it does seem to be fixed. Could someone else confirm?

@josephluck
Copy link

So it does seem to be fixed. Could someone else confirm?

It seems a little better, however if you edit from within the youtube link (i.e not at the end of the link) the cursor jumps to the end.

@cameracker
Copy link
Collaborator

I think this got fixed in a recent PR. Going to close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

15 participants