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

use real beforeinput events in all browsers that fire them #2060

Closed
ianstormtaylor opened this issue Aug 8, 2018 · 4 comments
Closed

use real beforeinput events in all browsers that fire them #2060

ianstormtaylor opened this issue Aug 8, 2018 · 4 comments

Comments

@ianstormtaylor
Copy link
Owner

ianstormtaylor commented Aug 8, 2018

Do you want to request a feature or report a bug?

Improvement.

What's the current behavior?

Right now we're only using real beforeinput events on iOS. In both Chrome and Safari on the desktop we're still using React's fake onBeforeInput event, which is actually just a polyfill that fires with keydown and textInput events. It hasn't yet been hooked up to use the real beforeinput event if one exists. I've opened an issue a while back for this here: facebook/react#11211 (If anyone wants to take a stab at pull requesting that functionality, that would be amazingly helpful!)

What's the expected behavior?

We should change the core logic to use real beforeinput events on all browsers that support them, since this is what browsers are converging to. Right now they are supported in:

And not supported in:

For now, I think it would make sense to aim for Level 1 support (which are non-preventable) and later we can refactor to add Level 2 support once browsers other than Safari start supporting it.

@ianstormtaylor
Copy link
Owner Author

For more information about what we currently do... it helps to first understand the background on how "input events" work in browsers (or at least the ones we're concerned with).

  • All of them fire some sort of "input" event when text is inserted into the DOM, either by a user pressing a key directly, or by multi-character pieces of text being inserted at once like spellcheck corrections, autocomplete, etc. But the lowest common denominator here doesn't actually have any useful information on the input event, so we have to use DOM diffing to see what changed.

  • Browsers that implement the Input Events Level 1 spec (ie. Chrome & Opera) fire more helpful input events, as well as beforeinput events before any change to the DOM has occured. These beforeinput events can be used to figure out what changes we need to make to our internal model, without having to diff the DOM. However, many of these beforeinput events cannot be cancelled with preventDefault().

  • Browsers that implement the Input Events Level 2 spec (ie. Safari) fire even more helpful beforeinput events that can (almost always) be prevented with preventDefault() such that we don't even need to allow the DOM mutation to occur, since we can detect what changes to apply and prevent the browser from applying the changes itself. It also adds beforeinput events for compositions in IME's.

Firefox and Edge do not support either Input Events spec, unfortunately. Although Edge has committed previously to supporting it, and Firefox has an (old) open issue for supporting it.

Chrome on the other hand, supports Level 1, but has previously publicly fought against Level 2 support, so it's unclear if they will ever support Level 2 in the future or not.

And a final complicating factor... React doesn't actually support the beforeinput event. What they call their onBeforeInput event is actually just a polyfill that fires with keydown and textInput events. It hasn't yet been hooked up to use the real beforeinput event if one exists. I've opened an issue a while back for this here: facebook/react#11211 (If anyone wants to take a stab at pull requesting that functionality, that would be amazingly helpful!)

@ianstormtaylor
Copy link
Owner Author

Also, some information about how we currently handle spellcheck corrections, which would change with Input Events as well...

When correcting a word with spellcheck, browsers that haven't yet implemented the Input Events Level 1 spec (eg. Firefox & Edge) don't actually fire any DOM event before the spellcheck correction edits the DOM. They only fire the input event after the DOM has been edited.

Because of this, there's no way to prevent the correction from happening, so we have to run a "diff" on the DOM to figure out what changed.

Currently, we use the input event to detect these scenarios, using a series of steps:

  1. Resolve the current DOM selection point, and use that to resolve the Leaf the selection is in, which is the one that was just edited.

  2. Get the text content from the Leaf and compare it to the text content of the DOM node that was just changed.

  3. Replace the Leaf's content in our internal data model with the newly corrected text content of the DOM node, and update the selection to be at the end of that newly correct piece of text.

This works for the most part.

(Note: since we have to map DOM nodes to Leaf instances in Slate, this is why sometimes spellcheck can have issues around leaf boundaries where marks are involved. There are probably bugs right now in our spellcheck leaf-resolving-and-comparing logic that create these issues.)

In browsers that have implemented Input Events Level 1 however, they should fire a beforeinput event with an event.inputType === 'insertReplacementText' that we can hook into and use to update our internal model before the DOM has been changed, and prevent the change from occuring. (We do not currently do this, but it would be a good idea to try for improved behavior.)

@SmilinBrian
Copy link
Contributor

f812816 was a good step toward this. Unfortunately I am seeing some problems with composition input in Safari. The Before plugin "eats" the beforeinput event, so none of the logic in the After plugin's onBeforeInput() ever executes. I find better results in Safari by commenting out the if (isSynthetic && HAS_INPUT_EVENTS_LEVEL_2) test.

That led to a further problem with the input event and DOM change checking you describe above. One use case we have is modeled closely after the Code Highlighting example. In that case the syntax Decorations result in the text being rendered with Leaves that do not match what After's onInput() is looking at, and I could not find any simple way of preserving the list of Decorations that is built at render time for onInput()s call to node.getLeaves(). To get the "real" leaves, it needs to call node.getLeaves(renderedDecorations).

I made it "mostly work" with a hack to check if the model text we compare the DOM text of anchorNode against is longer, then see if the native DOM node corresponding to node contains(anchorNode) and instead use the DOM text of that one, but the real fix would be to somehow get those rendered Decorations.

@ianstormtaylor
Copy link
Owner Author

Fixed by #3093.

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

Successfully merging a pull request may close this issue.

2 participants