Skip to content

Fix textarea undo crash - #6687

Open
guillaumelgf wants to merge 1 commit into
Textualize:mainfrom
guillaumelgf:fix-textarea-undo-stale-cursor
Open

guillaumelgf wants to merge 1 commit into
Textualize:mainfrom
guillaumelgf:fix-textarea-undo-stale-cursor

Conversation

@guillaumelgf

@guillaumelgf guillaumelgf commented Aug 6, 2026 •

Copy link
Copy Markdown

This PR fixes #6686

Summary

Fixes a TextArea undo 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() can
raise 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.

@guillaumelgf
guillaumelgf marked this pull request as ready for review August 6, 2026 15:42
@goransh-walia

Copy link
Copy Markdown

This looks good to me at a glance — thanks for putting it together.

@Phantom8420 Phantom8420 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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:

  1. CHANGELOG.md isn't updated — the PR checklist requires it. A ### Fixed entry with the PR link, matching the existing format, is missing.
  2. 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_offset is never called with an invalid location in the first place? The clamp is a fine safety net either way, just flagging the alternative.
  3. 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_offset call 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

No deployments
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.

TextArea undo can crash when a multiline paste is undone after scrollbar state changes

3 participants