Skip to content

Fix DropdownMenu throwing TextEditingController disposed error on select (#139266) - #139268

Closed
josh-burton wants to merge 2 commits into
flutter:masterfrom
josh-burton:nested-dropdownmenu-texteditingcontroller
Closed

Fix DropdownMenu throwing TextEditingController disposed error on select (#139266)#139268
josh-burton wants to merge 2 commits into
flutter:masterfrom
josh-burton:nested-dropdownmenu-texteditingcontroller

Conversation

@josh-burton

@josh-burton josh-burton commented Nov 29, 2023

Copy link
Copy Markdown
Contributor

When a DropdownMenu exists within another menu and a MenuItem is selected, the TextEditingController of the DropdownMenu is used after it has been disposed.

This PR checks whether the controller has been disposed before using it in the onPressed callback.

Note I'm not sure whether the hasListeners method is the correct way to check if the textEditingController is disposed.

Fixes #139266
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages labels Nov 29, 2023
@josh-burton
josh-burton changed the base branch from master to main November 29, 2023 22:35
@josh-burton
josh-burton changed the base branch from main to master November 29, 2023 22:35
navaronbracke
navaronbracke reviewed Nov 30, 2023
View reviewed changes
Comment thread packages/flutter/test/material/dropdown_menu_test.dart Outdated
@gspencergoog gspencergoog mentioned this pull request Dec 1, 2023
Merged
@gspencergoog

gspencergoog commented Dec 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Unfortunately, this won't work because hasListeners is a protected method. However, there are more issues here. It looks like the controller isn't really managed all that well. Rather than try and edit via review comments, I created a new PR that fixes this issue, and handles the controller better, especially when it gets set after the first build and the state gets reused.

I used your tests, I hope you don't mind. (If you do mind, I'm also happy to write my own).

See #139385

@josh-burton

josh-burton commented Dec 1, 2023

Copy link
Copy Markdown
Contributor Author

Unfortunately, this won't work because hasListeners is a protected method. However, there are more issues here. It looks like the controller isn't really managed all that well. Rather than try and edit via review comments, I created a new PR that fixes this issue, and handles the controller better, especially when it gets set after the first build and the state gets reused.

I used your tests, I hope you don't mind. (If you do mind, I'm also happy to write my own).

See #139385

No problem, thank you!

@josh-burton josh-burton closed this Dec 1, 2023
@josh-burton
josh-burton deleted the nested-dropdownmenu-texteditingcontroller branch December 1, 2023 19:20
auto-submit Bot pushed a commit that referenced this pull request Dec 5, 2023
@gspencergoog
Fix DropdownMenu throwing TextEditingController disposed error on…
8ec4c59 
… select (#139385)

## Description

When a DropdownMenu exists within another menu and a MenuItem is selected, the `TextEditingController` of the DropdownMenu is used after it has been disposed.

This PR manages the local `TextController` instance better, making sure that the one used when building is the current one, and handling the case where the `controller` is set on the widget after initial creation.

Also, places where we were setting the text and selection separately were converted to use `TextEditingValue` and set the value atomically.

## Related Issues
 - Fixes #139266

## Tests
 - Added tests (Created by @josh-burton  in #139268 - Thanks Josh!)
chunhtai pushed a commit to chunhtai/packages that referenced this pull request Jun 12, 2026
@gspencergoog
Fix DropdownMenu throwing TextEditingController disposed error on…
15ec188 
… select (#139385)

## Description

When a DropdownMenu exists within another menu and a MenuItem is selected, the `TextEditingController` of the DropdownMenu is used after it has been disposed.

This PR manages the local `TextController` instance better, making sure that the one used when building is the current one, and handling the case where the `controller` is set on the widget after initial creation.

Also, places where we were setting the text and selection separately were converted to use `TextEditingValue` and set the value atomically.

## Related Issues
 - Fixes flutter/flutter#139266

## Tests
 - Added tests (Created by @josh-burton  in flutter/flutter#139268 - Thanks Josh!)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@navaronbracke navaronbracke navaronbracke left review comments
@gspencergoog gspencergoog Awaiting requested review from gspencergoog
@QuncCccccc QuncCccccc Awaiting requested review from QuncCccccc

Assignees

No one assigned

Labels

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nested MenuAnchors throw 'A TextEditingController was used after being disposed' when MenuItem is selected

3 participants

@josh-burton @gspencergoog @navaronbracke