Skip to content

Treat selection hits on DOM-detached widgets as misses#6645

Open
uttam12331 wants to merge 1 commit into
Textualize:mainfrom
uttam12331:fix/selection-detached-widget-6643
Open

Treat selection hits on DOM-detached widgets as misses#6645
uttam12331 wants to merge 1 commit into
Textualize:mainfrom
uttam12331:fix/selection-detached-widget-6643

Conversation

@uttam12331

Copy link
Copy Markdown

Fixes #6643.

Problem

As diagnosed by @abs1u in the issue: during Markdown.update(), an old MarkdownParagraph can be detached from the DOM while the compositor still returns it from get_widget_and_offset_at(). A MouseDown during that window crashes:

File "src/textual/screen.py", line 1914, in _forward_event
    event.screen_offset - container.region.offset,
AttributeError: 'NoneType' object has no attribute 'region'

because container = content_widget.parent is None for the detached widget.

Fix

Following the direction the reporter verified locally ("ignoring a hit when widget.is_attached is false prevents the crash while preserving normal text selection"): in both mouse selection paths of Screen._forward_event (MouseDown start-selection and MouseMove update-selection), if the hit widget is no longer attached to the DOM, treat the hit as a miss.

Verification

  • Ran the issue's race reproducer against main: crashes at screen.py:1914 exactly as reported. With this change: the race window is still observed (detached at step: 3) and there is no crash.
  • Added test_mouse_down_on_detached_widget_does_not_crash, a deterministic regression test (removes a widget, then simulates the compositor still mapping it). It fails on main with the reported AttributeError and passes with this change.
  • Full tests/test_selection.py suite passes (15 passed); black --check clean; CHANGELOG entry added.

The compositor can briefly return a widget that has been removed from the
DOM (e.g. while Markdown.update swaps children). Starting or updating a
text selection on such a widget crashed with
AttributeError: 'NoneType' object has no attribute 'region'
because the detached widget's parent is None.

Guard both mouse selection paths in Screen._forward_event: if the hit
widget is no longer attached, treat the hit as a miss.

Fixes Textualize#6643
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.

Markdown.update can leave a detached MarkdownParagraph in the compositor and crash MouseDown selection

1 participant