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

[SuperEditor] Fix crash with selection across nodes when using the tags plugin (Resolves #2479) #2480

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

angelosilvestre
Copy link
Collaborator

[SuperEditor] Fix crash with selection across nodes when using the tags plugin. Resolves #2479

To reproduce the issue:

Select a paragraph node completely (all text) and then continue the selection to another paragraph node above it. Or simply select multiple nodes partially.

Exception:

Exception has occurred.
RangeError (RangeError (end): Invalid value: Not in inclusive range 10..56: 68)

The issue is that we are trying to use the caret position to extract the text. However, when multiple paragraphs are selected, we are always trying to extract the text using the end position of the selection, which will be invalid when one paragraph is larger then another.

We discussed about modifying the tag find logic for the action tags to only care about collapsed selections, because an action tag only makes sense while the user is typing, and when the user is typing, the selection is always collapsed.

There are some tests that ensure we extract tags with expanded selections, so I had to modify those tests to ensure the opposite.

Also, I suggest we change the composing reaction to only start composing if the user is actually typing. Currently, if a text contains a substring that is matched as a tag, for example "/hello", just by placing the caret somewhere in the middle of "hello" the reaction already starts a tag composition. We have a ticket for that: #2392.

@angelosilvestre
Copy link
Collaborator Author

@alterhuman Would you mind trying this PR?

@alterhuman
Copy link

@angelosilvestre selection issue is fixed while the original fix is still working correctly. #2392 is still pending, not sure if that issue is in scope of this PR or not. If not, then LGTM.

Copy link
Contributor

@matthew-carroll matthew-carroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matthew-carroll matthew-carroll merged commit 591af26 into main Jan 3, 2025
16 checks passed
@matthew-carroll matthew-carroll deleted the 2479_actions-tag-crash branch January 3, 2025 02:20
github-actions bot pushed a commit that referenced this pull request Jan 3, 2025
matthew-carroll pushed a commit that referenced this pull request Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Crash when selecting across paragraphs with action tag plugin
3 participants