fix: clamp markdown cursor range offsets - #770
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
@suneox can you please test the change in the App? |
Yes, I'll take a look at this one soon |
suneox
left a comment
suneox
left a comment
There was a problem hiding this comment.
Changeset LGTM, the functionality still works as expected
|
will review & test today / tomorrow |
jmusial
left a comment
jmusial
left a comment
There was a problem hiding this comment.
Changes look good, just one comment, will merge after it's resolved.
| return node.nodeType === Node.TEXT_NODE ? (node.textContent ?? '').length : node.childNodes.length; | ||
| } | ||
|
|
||
| function getClampedRangeOffset(node: ChildNode, offset: number): number { |
There was a problem hiding this comment.
Would be good to have jsdoc here with some context.
|
@jmusial Thanks for the feedback and it has been resolved, you can take another look |
Awesome, merged. Thank you for your contribution :) |
4 participants
Details
This fixes a web crash during markdown cursor restoration.
setCursorPositionclamped the cursor index against the markdown tree length, but then passed the derived offset directly to the actual DOM node used byRange.setStart/Range.setEnd.When the markdown tree and rendered DOM are temporarily out of sync, the offset can be valid for the tree but invalid for the DOM node, causing
IndexSizeError.This change clamps the final offset against the actual Range container node bounds before applying it.
Related Issues
Expensify/App#95347
Manual Tests
IndexSizeErroris thrown and the editor remains usable.Linked PRs
Expensify/App PR: Expensify/App#96638