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

onDOMSelectionChange causes crash during concurrent updates #5694

Closed
gblaketx opened this issue Aug 7, 2024 · 1 comment · Fixed by #5746
Closed

onDOMSelectionChange causes crash during concurrent updates #5694

gblaketx opened this issue Aug 7, 2024 · 1 comment · Fixed by #5746
Labels

Comments

@gblaketx
Copy link

gblaketx commented Aug 7, 2024

Description
The editor can crash if the user is typing while other updates are being applied (for instance, applying remote user updates or automated edits). The user's typing triggers an onDOMSelectionChange call, but the selection update from this function is stale, which can cause the cursor to jump or crash the editor.

Recording
https://github.com/user-attachments/assets/a68453b1-df47-4324-a542-9313569958ca

Sandbox
https://codesandbox.io/s/slate-selection-change-repro-ztssmp

Steps
To reproduce the behavior:

  1. Go to https://codesandbox.io/s/slate-selection-change-repro-ztssmp
  2. In Chrome developer tools, under the Performance tab, enable CPU throttling. This approximates the behavior of a large document and makes the issue easier to repro, although it still occurs without throttling.
  3. Click the "Simulate ops" button, which repeatedly adds new elements to the middle of the document.
  4. Type continuously in the last line of the document
  5. A "Cannot resolve a DOM point from Slate point" error is triggered

Expectation
Basic concurrent updates shouldn't crash the editor. In the example posted, the user should be able to continue to type on the last line while new lines are added above.

Environment

  • Slate Version: 0.103.0
  • Operating System: Mac
  • Browser: Chrome

Context
I suspect the root cause of this issue is a stale NODE_TO_INDEX weak map. Because that map is updated after nodes rerender, if the editor content changes and a selection event fires before a re-render, the map can be stale.

Looking at the code, the sequence of events is:

  1. A new line is inserted in the editor. React does not yet re-render
  2. The user's typing triggers the onDOMSelectionChange handler
  3. The handler attempts to convert the user's selected DOM node to a Slate range in toSlateRange, which calls findPath on the resulting Slate Node
  4. NODE_TO_INDEX is used to retrieve the path for the given node. However, because the editor has not yet re-rendered, this map is stale
  5. The resulting path to the node is wrong (Say, [2, 0] instead of [3, 0]) and an invalid selection is set. Downstream code that attempts to use this selection triggers a crash.
@DustinMackintosh
Copy link
Contributor

Thank you for your write up, @gblaketx, it was invaluable in helping me when I encountered this same issue. Hoping we can get some attention on my proposed fix.

dylans pushed a commit that referenced this issue Oct 24, 2024
…5746)

* fix: invalidate node maps when nodes change in-between paints #5694.

* Add patch changeset

* Catch additional invalide reference on Android
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants