Closed iEchoic closed 6 years ago
I'm seeing the exact same behavior with draftjs. I am guessing it has to do with autocorrect. It probably doesn't send keydown events. I am using Google keyboard on Samsung A3.
Considering switching to Slate. DraftJS had the same issue but this PR supposedly improves things https://github.com/facebook/draft-js/pull/1013 - is this helpful?
Running into this exact issue right now, happy to submit a PR. @ianstormtaylor can you point me to any pieces of code that might be the culprit?
Digging into this, this is the error being thrown when pressing "Space":
Essentially, what this tells us is: findDeepestNode
is called from components/content.js
without an element. The componentDidUpdate
lifecycle calls updateSelection
which thries to findDeepestNode(anchorSpan)
, but anchorSpan
is null.
anchorSpan
is querySelector('[data-offset-key="' + anchorKey + '-' + anchorIndex + '"]')
, that's about all I got so far. Any ideas why that could be undefined?
Here's the same issue described on draftjs issue tracker: https://github.com/facebook/draft-js/issues/1077 - my understanding is that Chrome Android isn't sending the expected input events for text coming from the software keyboard, but is changing the content of the contentEditable element. The space key appears to send the expected input events, resulting in the described behavior where a space drops the previously input text (since the space was the only character coming in through the normal input events).
This appears to be due to the intended Chrome behavior described here: https://bugs.chromium.org/p/chromium/issues/detail?id=118639#c260 - I suppose this makes sense for software keyboards like Swiftkey or Swype where there's no concept of individual keypresses, just a final word that is inserted all at once, but it wreaks havoc with any editor expecting hardware keyboard input.
Suggested workarounds involve using composition* events to detect the content being changed (without expected input events) - however the draftjs PRs/forks that do so have some unintended side-effects.
Additionally, I can second the exception that @mxstbr is seeing. I'm seeing it from users as:
Cannot read property 'firstChild' of null
Location /node_modules/slate/lib/utils/find-deepest-node.js, anchorSpan:15
Subsequent backtrace:
/node_modules/slate/lib/components/content.js:503 in updateSelection
/node_modules/slate/lib/components/content.jsl:435 in cal
/node_modules/react-dom/lib/CallbackQueue.js:76 in notifyAll
/node_modules/react-dom/lib/ReactReconcileTransaction.js:80 in call
/node_modules/react-dom/lib/Transaction.js:209 in closeAll
/node_modules/react-dom/lib/Transaction.js:156 in call
/node_modules/react-dom/lib/Transaction.js:143 in call
/node_modules/react-dom/lib/ReactUpdates.js:89 in perform
/node_modules/react-dom/lib/ReactUpdates.js:172 in <anonymous>
Just wanted to throw in a possible solution.
It may be worthwhile to sidestep a lot of the issues by using an element as an alternative to capture text.
This would take some work to get it working this way, but it may solve most of the issues at once. Kind of similar to the way the SlateJS and other contentEditable editors side stepped the incosistent DOM issues by building an alternate document model. A lot of work but fixed a lot of other issues at once.
The idea is to constantly look at the contents of the input element to derive what should be happening after each action.
Cursor moves in mobile android would happen with a "touch" event and that's when we'd commit the text into the editor and throw away the content of the input box. While the text is being composed, we constantly update that portion of the text.
The box could be preseeded with text something like this:
^|$
Where | is the cursor position.
On the removal of the leading ^ (doesn't have to be these characters) we do the equivalent of a backspace event. When $ is removed, that's a delete event.
As the contents get inserted/updated through autocorrect, swipe text, etc. we don't have to worry about what events are coming through so much. Whatever funky stuff the browser decides to do in terms of events, is irrelevant as long as we can capture the results in the text box.
From what I've seen, the Android Chrome team is unwilling to change the way the browser currently works and so far, the editor projects are all saying they can't figure out a workaround.
Note that I did try implementing something like this before. I abandoned that because it was difficult to place the cursor after cursor up/down events... however, since this is dealing only with the mobile input, cursor keys shouldn't be an issue. Feels like this might possibly work.
I may be missing some other issues but I'd be curious if anybody else thinks this is worth pursuing. @ianstormtaylor maybe you've already though of this?
Here is the behaviour I see on a LG G6 phone (Android 7):
When pressing space
after a word, it removes the word and inserts a space. You need to click on an auto-complete suggestion for the word to not be deleted.
This renders Slate completely unusable on Android devices :(
It may be related to #1176 and #1205
I am available on Slate's Slack if you want to run some testing.
This is happening at our end as well. Find attached the gif here CC: @invalidred Current Slate Version: 0.30.1 OS: Android N (7.0) Moto G (4) Browser: Chrome 61.0.3163.98
This draft-js PR https://github.com/facebook/draft-js/pull/1500 purportedly fixes this behavior. I wonder if this is something that could be tried for slate as well?
I'm not sure a text editor that manages it's own state is going to be able to solve this problem with acceptable performance. I tested done different settings on my Android. If you turn off "site suggestion s strip" in input settings it solves the input issues on Android. The interesting this is you can still use swype style typing. You just have to turn off the suggestion bar at the top.
I'm not sure a text editor that manages it's own state is going to be able to solve this problem with acceptable performance. I tested different settings on my Android. If you turn off "show suggestions strip" in input settings it solves the input issues on Android. The interesting thing is you can still use swype style typing. You just have to turn off the suggestion bar at the top.
I've been playing around with this and here's what I've found. On my Essential PH-1 phone running Android 7.1.1, the Android default keyboard (Gboard) seems to be causing at least some the issues. I've seen the same problem using Gboard on Chrome, Firefox Focus, and Opera (normal Firefox just crashes). However, when I used the SwiftKey keyboard (3rd party keyboard), typing worked on Chrome. Firefox Focus and Opera still had issues with the autocomplete. Firefox still crashed. That said, we can't expect people to use a specific 3rd party keyboard just to be able to type on slate...
Anyhow, I've also looked at what events are being fired upon key presses. On Chrome on my laptop, the beforeInput
event is fired upon every keyboard press which handles the text insertion for all characters. However, on Chrome on the Android simulator and phone, the onCompositionEnd
function is called when you finish typing the word and hit [SPACE]
. However, since beforeInput
is never called, the word is never added to the slate value.
In other words, the reason hitting [SPACE]
clears the word (shown in the various animated images above) is that the word is not in the slate value since beforeInput
is never called when the user is typing the word. So when [SPACE]
is pressed, the space character is added to the slate value, slate renders and the original inputted word disappears.
I've modified the code to begin to insert the word that the user types here: https://github.com/humandx/slate/commit/559b51f702c6c678d6bb862d4f2b171655dfe32c
Typing with the Gboard on Chrome now works. However, deleting characters/words is still extremely buggy so I'm not making this a PR just yet. For some reason, when deleting a character, onInput
is called which deletes all text in the node and then subsequently inserts the entire text without the deleted character.
If anyone wants to look at addressing this, let me know.
I believe the root cause of this issue is due to how Gboard sends KeyboardEvents (same as with IMEs in general), and not with Chrome or Android in particular. Using Chrome on iOS with Slate seems to work okay, and third-party keyboards (such as the Hacker's keyboard) on Android seem to work fine too. Later versions of Gboard don't send any usable information in the KeyboardEvents, so Slate doesn't handle these interactions properly. I believe Chrome propagates any information received from the the virtual keyboard.
Keyboard interaction handling happens in Core plugin after's onKeyDown
. If onKeyDown
can't discern what key was pressed, then the default behavior is to let onChange
, then onInput
handle the event. This is how rather unexpected behavior happens, such as deleting and re-inserting entire node text.
I took a shot at using onCompositionEnd
and onInput
to better handle interaction with IMEs (although I've been focused on Gboard in particular). I've been able to get entry and very rudimentary backspace functionality on Gboard with a plugin to supplement/override core plugins, but the process is still experimental, and not without several quirks. The code can be viewed here. Currently working on getting a demo site up for quick tests.
A major problem I see with this process is having to discern what the user intent was given whatever information you can get as opposed to being told what the user wanted to do (by pressing a specific key, for example). However, if this process can be generalized and works out, I think it can be extended to handle IME issues as well.
Any thoughts, suggestions, or comments on the plugin or the process would be much appreciated! In the meantime, the plan is to start tackling various use cases where this plugin fails and fixing those.
Anyone have any thoughts on fixing this? This is majorly detrimental to have issues in both React-Native and as a web-app. As @jzhang1 mentions, turning off the suggestion strip does indeed fix the issue. Unfortunately simply setting autocorrect=false
has no effect.
@Slapbox we ran into the same issue and android support was critical. We moved on from the idea of using slate or draft as mobile support is not their priority, and chose to use quill.js. It has a tone of stars and has lots of community around it and works great with react, Android and iOS.
@invalidred thanks for weighing in. Are you using Quill for mobile and desktop, or are you supporting both systems? If both, what's the overhead on that like?
@javan opened this bug for IME behaviour on Android: https://bugs.chromium.org/p/chromium/issues/detail?id=812674
It seems that composition events will change again on Chrome 65, and will still need to be handled as edge cases. I dont have time to work on this, but this thread (and the linked ones) might help someone who wants to tackle this.
@slapbox we have a responsive PWA (web app) single code base for mobile and desktop though we are focusing on mobile at the moment. As long as you abstract out a reusable component around Quill I don’t see why it should be hard to work with no matter the environment. But then again I truly need to understand your context to make a more sound decision.
I was using Chrome Remote Desktop today and noticed that the keyboard seems to act as a proper keyboard for input. I wonder if there's some way to force this behavior in an app. In Chrome Remote Desktop, swipe and suggestions are disabled.
I created a Roadmap to Mobile Support issue https://github.com/ianstormtaylor/slate/issues/1720 in order to get official mobile support added to SlateJS.
This issue is one of the key issues to fix and any help would be appreciated.
I've also created a #mobile channel to Slack for people to discuss mobile related issues https://slate-slack.herokuapp.com/
Does anybody know how or if you can replicate this bug using an emulator on the Mac?
@thesunny You can download Android Studio and set up an Android Virtual Device with any of the more recent APIs. If you open up Chrome and check out the SlateJS demos, you should be able to replicate the issue easily.
https://www.genymotion.com/fun-zone/ works well too.
Thanks @nathanfu88 and @javan.
@nathanfu88 I'm not clear what the simplest way is to run a browser like Chrome in Android Studio. Could you point me in the right direction?
I'm not even sure how to setup and start an Android Virtual Device. So far Google is leading me to all the wrong places. Mostly information about how to create a web view in an Android app.
Install Android Studio: https://developer.android.com/studio/install.html
Make sure the options for "Android Emulator" and "Intel x86 Emulator Accelerator (HAXM installer)" are checked when it comes to SDK tools.
Follow this to create an AVD: https://developer.android.com/studio/run/managing-avds.html#createavd
When you create your device, in the Android Virtual Device Manager, click the Green Run arrow to start the emulator. After the emulator is up and running, you use it like any other Android device. Open Chrome (which should be installed by default in the later APIs) and visit the Slate demo page.
Thanks @nathanfu88 for your help in the Slack channel. There's a PR for Android Develpment Guide now and I can replicate this bug.
You'll want to test various keyboards on Android too. GBoard behaves differently than the stock Android keyboard. See https://bugs.chromium.org/p/chromium/issues/detail?id=818332 for more 😕.
@ianstormtaylor @nathanfu88 @vshia and anyone else who's deep dived into this issue, I'd like to get some feedback into a possible solution to this issue.
So, I've been following this issue for a long time and it's the most important issue to solve for the Roadmap to Mobile Support.
From all the research I've seen, here's what I know:
Because some really smart people are trying to analyze events and failing, and even the one that have succeeded are still having problems, I would like to suggest an alternative. I would like to run it by several smart contributors here to see if this seems feasible.
What I'd like to propose is instead of analyzing events to create Value
, we work backwards from the DOM instead.
setTimeout
to wait for the next tick after the DOM updates.Value
We don't depend on the event to give us definitive information about what has changed in the DOM because this is currently unreliable. Instead, we only use the event to inform us when a change in DOM has happened.
The nice thing (assuming this works) is that if the Chrome team changes how these events fire (which has been talked about), our code shouldn't have to change.
Assuming the above can work, one other thing that does worry me is what happens when a user hits ENTER. Usually, many plugins (including my own) look for the ENTER event and override it to create new blocks. If we use this watch-the-DOM technique, the default GBoard enter behavior may not be what we want. Presumably, we'd have to fix this by undoing the the part of the GBoard DOM change that handles the ENTER then we'd put the selection back where we want it. Then we create our own synthetic plugin event called onEnter
(or something) that we'd fire. We'd ask all plugin developers to use onEnter.
There are probably similar cases like SPACE but I think with ENTER, we'd solve enough of the problem that it would be acceptable for mobile use.
Anyways, I'd like to hear some feedback. Does this seem possible? I haven't delved deep into how events, translate to Value
translate to a DOM render and how React/Slate handle it so any input would help. If this is a waste of time, I don't want to spend a long time pursuing it. If you think it sounds possible, I'd like to hear too. Thanks.
Probably not helpful, but might give someone an idea for a more robust solution, see the two latest posts on the similar issue over at https://github.com/facebook/draft-js/pull/1500#issuecomment-392268125
Do we have any idea HOW QuillJS manages to work where all other projects fail? If we could get that knowledge into this thread, that would potentially be a great start towards resolving the issue.
FWIW, we (Trix) support Android reasonably well by handling composition events. It's not pretty and I don't know if this is useful to you at all, but here are the guts:
↓
This is really helpful. Thank you very much!
Are there any major gotchas or major inconveniences you've found this this method? I know it's not pretty, but on the surface it seems not as convoluted as I would think.
Are there any "lessons learned" in the process of implementing this that might be useful to know before laying down some code and giving it a shot?
Thanks again @javan !
This comment outlines some of the gotchas:
Here's our current list conditions for coping with Android's composition events so far:
Ignore
compositionend
whencompositionstart
data is not empty. Happens when pressing backspace at the end of word and updating it on older versions of System WebView.Ignore
compositionend
whencompositionupdate
was received, butcompositionstart
was not. Can happen when backspacing through an entire word.Ignore
compositionend
when nocompositionstart
orcompositionupdate
was received. Happens when moving the cursor in and out of a word.
And, as of Chrome 65, the fact that composition events no longer definitively mean that the user is typing: https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c9
Trix's composition tests spell most of these out: https://github.com/basecamp/trix/blob/e3f19b6b0a1342eda27069b7b9c3f17249bda26a/test/src/system/composition_input_test.coffee
ProseMirror takes a different approach to handling composition events that's worth a look: https://github.com/ProseMirror/prosemirror-view/blob/9516c1ff59c0f9a9d734162a2f11984de88b124a/src/input.js#L328-L357
And finally, I would look into handling DOM Level 2 beforeinput
/ input
events instead of composition events: https://www.w3.org/TR/input-events-2/. Browser support was non-existent when we created Trix, but it's pretty good now and we hope to switch over.
✌️
First of all, apologies for the long time between updates. I will commit some time to mobile in August if somebody doesn't fix it first.
If the issue gets resolved through the efforts of SlapBox and others, that's great. If not, I will commit to spending one to two weeks on this issue in August.
I thought it would be valuable to at least take some time to summarize my brain dump with SlapBox in case some others want to take a crack at the approach I was planning on taking.
Note: Just want to acknowledge that it's possible there is some untenable blocker to this strategy but still felt it was worthwhile to post
Based on @nathanfu's efforts and following issues in other editors, using Android's browser events to update Slate's internal state has been unreliable. Even when a strategy that works is discovered, the implementation of Android's event API can and has changed.
javan's post above from the Trix team suggests that using the new beforeinput
and input
events may be reliable but there is no implementation yet. I want that to work as that feels like the better approach if it's reliable; however, this design document outlines a different strategy in the case it is not.
Also the approach outlined here would be backward compatible with older Android browsers where as using the new input events will only work on updated browsers. The design document below can also use input
and beforeinput
events when they are available and fall back to a DOM analysis strategy when it is not.
We create a small library outside of Slate JS that is easy to test and reason about without all the noise of the editor code. One additional benefit of separating this library is that other editors can use the same library.
This library takes a ContentEditable node and emits its own virtual events to replace the inconsistent Android keyboard events. For now, we will call these "Virtual events" to identify that these are not fired natively. These events are only used in an Android environment since iOS works fine with the regular events.
It works like this:
We first look at the current selection (i.e. cursor position) and find its parent node. We assume the selection is collapsed for now. The parent node is the container
that we will be observing. Because the selection is collapsed, the parent node will be at the lowest level node in the DOM. We then look at two things: Its text and its current cursor position. It may look something like this (Note: the | denotes the cursor position)J:
<strong>Hello |World</strong>
We don't use Android's keyboard/composition/input events to tell us what Virtual events to fire but we do use them to tell us when to look for changes. The keyboard/composition/input fire reliably when changes happen but the events are unreliable when it comes to telling us what actually happened. We sidestep this by ignoring their event properties and we use the DOM and cursor position to tell us the the what instead.
Let's say that the user wants to type "Hello Wonderful World". So, he starts typing "W" and the DOM now looks like this.
<strong>Hello W|World</strong>
Based on the previous cursor position, the current cursor position and the fact that the DOM now contains an additional "W", we emit an "insert" Virtual event with the character "W". More on how we determine what happened in the algorithm section later.
Virtual events can be one of two types: "insert" which passes one or more characters, "backspace" (backwards) which passes the number of characters to backspace over. We may need other events but assuming that Android always leaves the cursor at the end of what we just edited, these might be the only two we need.
User types another letter "o":
<strong>Hello Wo|World</strong>
We fire virtual event "Insert" with the character "o".
Now the user has the option to autocomplete and the autocomplete suggests lowercase "w" because it doesn't think it should be a capital "W". The user chooses the autocomplete and we end up with:
<strong>Hello wonderful |World</strong>
We analyze the text and cursor position and come up with "backspace" 2 characters and "insert" "wonderful ".
The diffing algorithm works like this.
First we ignore all the characters that match on the left and the ones that match on the right. So for example:
Hello Wo|World
Hello wonderful World
We end up with
Wo|
wonderful |
So the first two letters Wo
don't match so we backspace the length of those letters which is 2.
Then we insert the second part wonderful
.
One nice benefit of the Virtual events approach is that it should be possible to use input
and beforeinput
if they become reliable in the future (assuming they aren't already). So if they don't work now, and we build this virtual events system, we just need to update the library. We can also use the dual approach of using DOM analysis like here for older Android browsers and upgrade to input
and beforeinput
events for Android browsers that support it.
This is the part I think it would be helpful to get some people more familiar with the internals involved.
The flow of browser events to internal Immutable structure changes and then those changes going back to the Editor (including when they do or don't adjust the DOM) is not documented. There are probably a handful of people that have stepped through it and already understand it pretty well.
I think the library I just outlined is something somebody without this deep understanding of Slate can build and test but it could take some time to work through how Slate works.
Assuming somebody can build the first part, it would be great to have the completion done by somebody well versed in internals or at least can spend some time helping to guide.
I haven't had time to read this whole post yet, but the primary issues I'm seeing on my end are:
So, priority # 1, we need to know how to debug the core packages. In particular, slate-react
in the context of the rich text example or some other comparable environment. I have an issue open for that here https://github.com/ianstormtaylor/slate/issues/1907
@ianstormtaylor, how are you debugging packages during development? Is there some additional environment setup you can share?
I have the additional question of whether Input Events Level 2 are meant solely for IME input and what kind of timeline we're looking at for final specs. If these will be usable for our purposes, that would inform our approach. It seems kind of unlikely to me that either the level 1 or 2 specs will be finalized in the near future based on this thread
I'm in the process of rewriting the logger as a separate React component that can be included in any React app: https://github.com/danburzo/react-event-logger . It's not a big deal, (the original code is quite short), but I'm travelling and will probably not get to that until next week.
However, when I (sloppily) wrote the tool last year, my idea was to:
I haven't been in touch with the Slate codebase for the last ...9 months? (by the way, sorry about disappearing out of the blue 😬 because reasons) but I wonder how much extra value would we derive from integrating such a tool at the react-slate level
@Slapbox no I'm not doing anything additional. Sounds like someone just needs to investigate how to get sourcemaps with Webpack working with the Chrome debugger.
@ianstormtaylor I figured as much regarding your environment. Rest of my reply in the relevant ticket here: https://github.com/ianstormtaylor/slate/issues/1907#issuecomment-398218498
@danburzo thanks very much for springing right back into action! My feeling is that your logger is a crucial tool to have in all of this. I think even your base tool being connected to the editor, even without would be a tremendous help in fully grasping the flow of events. Do you have something in mind as far as integrating with Slate directly?
Will be working on this the week of August 13-17 and created a separate issue to track progress: https://github.com/ianstormtaylor/slate/issues/2047
Closing in favor of https://github.com/ianstormtaylor/slate/issues/2062 where we have research about the exact events that Android fires. If this issue affects you, please help! That's the only way we're going to see progress on this.
@thesunny @ianstormtaylor I am using Slate v47
and it's working perfectly on desktop and iOS devices but onChange
is not triggering on Android devices. I would greatly appreciate any suggestion. Thanks.
This issue has returned since ^0.94.1
, although this did fix issues with typing mention triggers, which didn't work before on android, which now does work, but now the text duplication issues has arisen again...
I just did some testing on Chrome on a Samsung Galaxy J3, and the input was pretty wonky (iPhone was ok). It's hard to nail down exactly what's happening, but I see a couple things:
This project looks great so far, and I'd love to use it, but I need Android support. I don't know anything about the nuances of input on contenteditable and/or Android, but if you have a rough direction/hypothesis for why this may be happening and don't have time to fix it now, I can dig in more and see what I can do.
Thanks!