Skip to content

Don't orphan SplitContainer content when the assignment is invalid - #4633

Open
Dev9269 wants to merge 2 commits into
beeware:mainfrom
Dev9269:fix-splitcontainer-content-setter
Open

Don't orphan SplitContainer content when the assignment is invalid#4633
Dev9269 wants to merge 2 commits into
beeware:mainfrom
Dev9269:fix-splitcontainer-content-setter

Conversation

@Dev9269

@Dev9269 Dev9269 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

SplitContainer.content setter detached the currently-shown widgets before validating the new value:

for old_content in self._content:
    if old_content is not None:
        old_content.app = None
        old_content.window = None
try:
    if len(content) != 2:
        raise TypeError()
    ...

If the assignment was invalid (wrong length, duplicate widget, wrong element type), the exception propagated but the old content had already been orphaned — app/window were reset to None, so the previously-visible widgets detached from the widget tree even though content still reported the old value.

This PR moves the detach to after the new content is fully validated, so a failed assignment leaves the current content attached and intact.

Fixes #4631

PR Checklist:

  • I will abide by the BeeWare Code of Conduct
  • I have read and have followed the CONTRIBUTING.md file
  • This PR was generated or assisted using an AI tool

Assisted-by: Claude Opus 4.5

@Dev9269
Dev9269 force-pushed the fix-splitcontainer-content-setter branch from 6622e84 to 45ba723 Compare August 5, 2026 22:05
Comment thread core/tests/widgets/test_splitcontainer.py Outdated

@freakboy3742 freakboy3742 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch - thanks for the report and PR.

However - this has clearly broken CI. I'm guessing this is because the test app lifecycle isn't being honoured because you're manually constructing the SplitContainer widget rather than using the fixture.

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.

SplitContainer.content setter orphans current content when the assignment is invalid

2 participants