Fix textarea undo crash - #6687
Open
guillaumelgf wants to merge 1 commit into
Open
guillaumelgf wants to merge 1 commit into
guillaumelgf wants to merge 1 commit into
Conversation
guillaumelgf
marked this pull request as ready for review
August 6, 2026 15:42
|
This looks good to me at a glance — thanks for putting it together. |
Phantom8420
reviewed
Sep 15, 2026
Phantom8420
left a comment
There was a problem hiding this comment.
The approach is reasonable — _recompute_cursor_offset only maintains a visual cache (self._cursor_offset), so clamping the location used for that calculation without touching self.selection is proportionate. clamp_visitable is already the established idiom for this (_clamp_selection, move_cursor_relative both use it), so this isn't introducing a new pattern.
A few things before this merges:
- CHANGELOG.md isn't updated — the PR checklist requires it. A
### Fixedentry with the PR link, matching the existing format, is missing. - Would you rather fix the root ordering instead — the selection getting restored to a stale location while the document is already shortened during undo — so
_recompute_cursor_offsetis never called with an invalid location in the first place? The clamp is a fine safety net either way, just flagging the alternative. - The test asserts
selection == Selection.cursor((0, 0))after undo but doesn't check that the resulting cursor offset/visual position ends up correct once the undo flow settles. If a later_recompute_cursor_offsetcall with the now-restored location fixes it up, an assertion for that (or a comment explaining why it's guaranteed) would lock it in rather than just proving the crash is gone.
Nit: a comment on why the test needs size=(120, 40) + height: 8 would help — presumably to force wrapping/scrolling so the scrollbar-refresh path that triggers the crash actually runs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes #6686
Summary
Fixes a
TextAreaundo crash that can happen after undoing a large multiline paste.During undo, the document may already be restored to its shorter previous state while the cursor location still temporarily points into the pasted text. If scrollbar refresh tries to scroll that stale cursor location into view,
WrappedDocument.location_to_offset()canraise because the row no longer exists.
This change clamps the cursor location used for the visual offset calculation, without mutating the actual selection. The undo flow can then finish restoring the selection normally.