fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled #151969

rkishan516 · 2024-07-18T16:06:46Z

DropdownMenu throws RangeError when both filter and search are enabled because when we search for elements, we have some highlighted element, but if there is no element to highlight it tries to access 0th element is the filtered entries, but entries are empty.

Fixes #151878

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].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

PurplePolyhedron · 2024-07-19T18:27:34Z

packages/flutter/lib/src/material/dropdown_menu.dart

+    if (searchText.isEmpty || entries.isEmpty) {
      return null;
    }
    if (currentHighlight != null && entries[currentHighlight!].label.toLowerCase().contains(searchText)) {


It is still possible to have out of range error when entries reduce in size but still non-empty.
For example, highlight the second item, then enter a search that only matches a single item would cause out of range error in entries[currentHighlight!].

bleroux

Thanks for the contribution.

Moving the new test just after the last one introduced in #147294 would make sense.

bleroux · 2024-07-22T08:32:10Z

packages/flutter/test/material/dropdown_menu_test.dart

    expect(textField.keyboardType, TextInputType.text);
  });
+
+  // Regression test for https://github.com/flutter/flutter/issues/151878


Suggested change

// Regression test for https://github.com/flutter/flutter/issues/151878

// Regression test for https://github.com/flutter/flutter/issues/151878.

bleroux · 2024-07-22T08:32:23Z

packages/flutter/test/material/dropdown_menu_test.dart

+      ),
+    ));
+
+    // Open the menu


Suggested change

// Open the menu

// Open the menu.

PurplePolyhedron · 2024-07-25T00:42:28Z

Sorry for the inconvenience. I think I've made a mistake in #147294. The checking of current selection should not be put in search.

After some further reading of the original code, I think search was intended to be "static"(it use the value type <T> so couldn't be made static) and agnostic about the _DropdownMenuState. So the logic regarding currentHighlight should not be put in search. Furthermore it doesn't fix the issues it intended to fix (#147253, #147516) if a custom searchCallback is provided, since it doesn't have easy access of the current highlight.

QuncCccccc

Thanks a lot for the contribution! Seems there are still some test failures. Could you help take a look:)?

QuncCccccc · 2024-07-29T19:30:28Z

packages/flutter/lib/src/material/dropdown_menu.dart

-      } else {
-        currentHighlight = search(filteredEntries, _localTextEditingController!);
+      final bool shouldUpdateCurrentHighlight = _shouldUpdateCurrentHighlight(filteredEntries, _localTextEditingController!);
+      if(shouldUpdateCurrentHighlight){


Suggested change

if(shouldUpdateCurrentHighlight){

if (shouldUpdateCurrentHighlight){

bleroux · 2024-07-30T07:38:02Z

@PurplePolyhedron Does this PR conflict with your work on #152378?

PurplePolyhedron · 2024-07-30T11:59:25Z

@PurplePolyhedron Does this PR conflict with your work on #152378?

Yes, there are some conflicts, #152378 removed the currentHighlight checking in search, because currentHighlight matching checks cannot be used for custom search, where we don't know the match conditions.

Piinks · 2024-08-14T18:13:17Z

@rkishan516 has the conflict with #152378 been resolved? Asking if this is ready for another review. :)

rkishan516 · 2024-08-15T01:15:50Z

@rkishan516 has the conflict with #152378 been resolved? Asking if this is ready for another review. :)

No, @Piinks, I am waiting for #152378 to merged and based on that I will do required changes.

Piinks · 2024-08-15T15:10:52Z

Ah ok! Thank you very much for clarifying. We'll follow up on this once #152378 lands. 👍

PurplePolyhedron

I decided to no longer change the search function in #152378 (Pending review). There shouldn't be conflict any more.

Apologies for troubles.

PurplePolyhedron · 2024-08-16T17:47:07Z

packages/flutter/lib/src/material/dropdown_menu.dart

-        currentHighlight = widget.searchCallback!(filteredEntries, _localTextEditingController!.text);
-      } else {
-        currentHighlight = search(filteredEntries, _localTextEditingController!);
+      final bool shouldUpdateCurrentHighlight = _shouldUpdateCurrentHighlight(filteredEntries, _localTextEditingController!);


We shouldn't use shouldUpdateCurrentHighlight when a searchCallback is provided, since we don't know its match condition.

QuncCccccc · 2024-09-09T22:43:31Z

Please let me know once this is ready to review:) Once the PR comments above are addressed and get the second approval, we are good to go!

rkishan516 · 2024-09-10T00:06:22Z

Please let me know once this is ready to review:) Once the PR comments above are addressed and get the second approval, we are good to go!

I have changed things other than the test bit.
If I don't add await tester.enterText(find.byType(TextField), '');, its still reading older value, even though just after that I am pumping new app.

PurplePolyhedron · 2024-09-10T04:41:03Z

Please let me know once this is ready to review:) Once the PR comments above are addressed and get the second approval, we are good to go!

I have changed things other than the test bit. If I don't add await tester.enterText(find.byType(TextField), '');, its still reading older value, even though just after that I am pumping new app.

Can you try to run the test on the newest version without await tester.enterText(find.byType(TextField), '')? I couldn't reproduce the issue, perhaps there was an issue in a earlier version and it has been resolved.

bleroux · 2024-09-11T12:55:47Z

@rkishan516 Looking at the new diff, it looks like you applied autoformat to dropdown_menu_test.dart. Can you update the PR without this formatting change which leads to a huge diff.

…xist when search and filters are enabled

rkishan516 · 2024-09-11T13:24:38Z

@rkishan516 Looking at the new diff, it looks like you applied autoformat to dropdown_menu_test.dart. Can you update the PR without this formatting change which leads to a huge diff.

Hey @bleroux, Sorry for that. I have fixed and pushed the change.

bleroux · 2024-09-11T13:33:36Z

Hey @bleroux, Sorry for that. I have fixed and pushed the change.

No worry, every contributor hit this at one point 😅

QuncCccccc

LGTM:) Thanks for the contribution! Once we have the second approval, we are good to go!

bleroux

LGTM!

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

Roll Flutter from 2e221e7 to 303f222 (77 revisions) flutter/flutter@2e221e7...303f222 2024-09-12 [email protected] Manual roll to 48ddaf578fb0c8326d5b4b680b0f49ea72e33216 (flutter/flutter#155070) 2024-09-12 [email protected] Externalize and update onboarding instructions (flutter/flutter#154730) 2024-09-12 [email protected] when setting up the log reader for a device during `flutter run`, discard any `RPCError` thrown due to the device being disconnected (flutter/flutter#155049) 2024-09-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "iOS: update provisioning profile for 2024-2025 cert (#155052)" (flutter/flutter#155059) 2024-09-12 [email protected] iOS: update provisioning profile for 2024-2025 cert (flutter/flutter#155052) 2024-09-11 [email protected] Factor out `Container` objects (flutter/flutter#153619) 2024-09-11 [email protected] Move (`dev/tools`), complete v0 of `native_driver` (Android) (flutter/flutter#154843) 2024-09-11 [email protected] Roll Flutter Engine from ade8ef293bc6 to ee5adf6d2ee1 (2 revisions) (flutter/flutter#155046) 2024-09-11 [email protected] Fix `flutter run` on Mac x64 hosts if Swift Package Manager is enabled (flutter/flutter#154645) 2024-09-11 [email protected] Roll Packages from bb53e5d to 4c18648 (1 revision) (flutter/flutter#155033) 2024-09-11 [email protected] Roll Flutter Engine from 4eb729b7a5c4 to ade8ef293bc6 (3 revisions) (flutter/flutter#155031) 2024-09-11 [email protected] fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled (flutter/flutter#151969) 2024-09-11 [email protected] Marks Linux build_tests_3_5 to be unflaky (flutter/flutter#154993) 2024-09-11 [email protected] Add 'direction' allow to 'SegmentedButton' oriented vertically (flutter/flutter#150903) 2024-09-11 [email protected] Marks Linux build_tests_5_5 to be unflaky (flutter/flutter#154995) 2024-09-11 [email protected] Update the signature of DDS launcher callback. (flutter/flutter#154949) 2024-09-11 [email protected] Migrate Color.toString() test, improves `equalsIgnoringHashCodes` (flutter/flutter#154934) 2024-09-11 [email protected] Update material and cupertino localizations (flutter/flutter#154959) 2024-09-11 [email protected] Marks Linux build_tests_1_5 to be unflaky (flutter/flutter#154991) 2024-09-11 [email protected] Marks Linux build_tests_2_5 to be unflaky (flutter/flutter#154992) 2024-09-11 [email protected] Fix `flutter create` warning regarding Java compatibility (flutter/flutter#152836) 2024-09-11 [email protected] Roll Flutter Engine from 54757dab9462 to 4eb729b7a5c4 (1 revision) (flutter/flutter#155022) 2024-09-11 [email protected] Fix java version used by `build_aar_module_test` (flutter/flutter#154967) 2024-09-11 [email protected] Roll Flutter Engine from 0a14c519ea4f to 54757dab9462 (1 revision) (flutter/flutter#155015) 2024-09-11 [email protected] Roll Flutter Engine from 35a3171b72c5 to 0a14c519ea4f (1 revision) (flutter/flutter#154984) 2024-09-11 [email protected] Roll Flutter Engine from b9c0b96c3316 to 35a3171b72c5 (1 revision) (flutter/flutter#154980) 2024-09-11 [email protected] Roll Flutter Engine from 52eeea075767 to b9c0b96c3316 (1 revision) (flutter/flutter#154976) 2024-09-11 [email protected] Roll Flutter Engine from a26075f9b1e6 to 52eeea075767 (1 revision) (flutter/flutter#154973) 2024-09-11 [email protected] Roll Flutter Engine from 60c15bc0f40e to a26075f9b1e6 (6 revisions) (flutter/flutter#154969) 2024-09-11 [email protected] Migrate `apple-mobile-web-*` to `mobile-web-*`. (flutter/flutter#154964) 2024-09-11 [email protected] Roll Flutter Engine from 8a038a6f7099 to 60c15bc0f40e (15 revisions) (flutter/flutter#154960) 2024-09-10 [email protected] Adds dart fixes for Color opacity functions (flutter/flutter#154953) 2024-09-10 [email protected] Missing benchmarks for `foundation/all_elements_bench.dart` (flutter/flutter#154954) 2024-09-10 [email protected] Update color assertions (flutter/flutter#154752) 2024-09-10 [email protected] handle EAGAIN (macOS) in ErrorHandlingProcessManager (flutter/flutter#154306) 2024-09-10 [email protected] fix unpack freezing app with animation duration zero (flutter/flutter#153890) 2024-09-10 [email protected] Remove last `--disable-dart-dev` in `flutter/flutter`. (flutter/flutter#154948) 2024-09-10 [email protected] Remove scheduler: luci from new `build_aar_module_test` (flutter/flutter#154945) 2024-09-10 [email protected] Roll pub packages (flutter/flutter#154939) 2024-09-10 [email protected] `CupertinoSlidingSegmentedControl` update (flutter/flutter#152976) 2024-09-10 [email protected] Roll pub packages (flutter/flutter#154933) 2024-09-10 [email protected] fix test `chrome.close can recover if getTab throws a StateError` (flutter/flutter#154889) 2024-09-10 [email protected] SearchBar context menu (flutter/flutter#154833) 2024-09-10 [email protected] Fix `flutter build aar` for modules that use a plugin (flutter/flutter#154757) 2024-09-10 [email protected] Roll Packages from b4e0fc1 to bb53e5d (4 revisions) (flutter/flutter#154926) 2024-09-10 [email protected] Clean up `SnackBar` inherit theme data test (flutter/flutter#154921) ...

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 18, 2024

rkishan516 force-pushed the dropdown-range-error branch from 8f2ee49 to a5109c9 Compare July 18, 2024 16:07

Piinks requested a review from QuncCccccc July 18, 2024 22:14

rkishan516 force-pushed the dropdown-range-error branch 2 times, most recently from 828ec84 to b4a1f79 Compare July 19, 2024 00:26

PurplePolyhedron reviewed Jul 19, 2024

View reviewed changes

rkishan516 force-pushed the dropdown-range-error branch 2 times, most recently from 6942bf6 to c997c14 Compare July 20, 2024 04:28

bleroux reviewed Jul 22, 2024

View reviewed changes

rkishan516 force-pushed the dropdown-range-error branch 2 times, most recently from 20afbe2 to 77bc9f1 Compare July 22, 2024 13:36

rkishan516 force-pushed the dropdown-range-error branch 2 times, most recently from 0e678ee to 5c65cde Compare July 25, 2024 07:27

QuncCccccc reviewed Jul 29, 2024

View reviewed changes

rkishan516 force-pushed the dropdown-range-error branch 3 times, most recently from 2091d43 to dd453ca Compare July 30, 2024 03:28

rkishan516 force-pushed the dropdown-range-error branch from dd453ca to 56f32e4 Compare August 2, 2024 02:41

PurplePolyhedron mentioned this pull request Aug 16, 2024

Disable Dropdown menu search during keyboard navigation #152378

Merged

9 tasks

PurplePolyhedron reviewed Aug 16, 2024

View reviewed changes

rkishan516 force-pushed the dropdown-range-error branch from 66dad07 to cda835c Compare August 21, 2024 14:41

Piinks requested a review from QuncCccccc August 21, 2024 18:29

rkishan516 force-pushed the dropdown-range-error branch from cda835c to b6bbbe9 Compare August 22, 2024 01:22

rkishan516 force-pushed the dropdown-range-error branch from 1ed211f to 8b2bfda Compare September 9, 2024 23:59

rkishan516 force-pushed the dropdown-range-error branch from 8b2bfda to cbfedbc Compare September 10, 2024 16:01

fix: Dropdown menu trying to access highlight element which doesn't e…

265a349

…xist when search and filters are enabled

rkishan516 force-pushed the dropdown-range-error branch from cbfedbc to 265a349 Compare September 11, 2024 13:23

QuncCccccc approved these changes Sep 11, 2024

View reviewed changes

bleroux approved these changes Sep 11, 2024

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 11, 2024

auto-submit bot merged commit 04e1b17 into flutter:master Sep 11, 2024

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to 04e1b174ea6d (66 revisions) flutter/packages#7630

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

db937bb

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 50a190caec0c (74 revisions) flutter/packages#7634

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

487d9dc

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to d8f8613a5b94 (75 revisions) flutter/packages#7635

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

0a936ff

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 90aa59f6c32f (76 revisions) flutter/packages#7637

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

f4d44ae

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 303f222e17e3 (77 revisions) flutter/packages#7638

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

885f63a

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

Piinks mentioned this pull request Sep 14, 2024

Flutter Framework Team: Notable Commits #121415

Closed

GamalOthman mentioned this pull request Oct 28, 2024

Dropdownmenu RangeError Exception when using enableFilter:true with enableSearch:true (default) #157697

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

6e65c2a

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

04e1b17 fix: Dropdown menu trying to access highlight element which d…

87de737

…oesn't exist when search and filters both are enabled (flutter/flutter#151969)

rkishan516 deleted the dropdown-range-error branch July 26, 2025 05:46

	// Regression test for https://github.com/flutter/flutter/issues/151878
	// Regression test for https://github.com/flutter/flutter/issues/151878.

	if(shouldUpdateCurrentHighlight){
	if (shouldUpdateCurrentHighlight){

fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled #151969

fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled #151969

Uh oh!

Conversation

rkishan516 commented Jul 18, 2024

Pre-launch Checklist

Uh oh!

PurplePolyhedron Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

bleroux Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

PurplePolyhedron commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

QuncCccccc Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux commented Jul 30, 2024

Uh oh!

PurplePolyhedron commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks commented Aug 14, 2024

Uh oh!

rkishan516 commented Aug 15, 2024

Uh oh!

Piinks commented Aug 15, 2024

Uh oh!

PurplePolyhedron left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PurplePolyhedron Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

QuncCccccc commented Sep 9, 2024

Uh oh!

rkishan516 commented Sep 10, 2024

Uh oh!

PurplePolyhedron commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bleroux commented Sep 11, 2024

Uh oh!

rkishan516 commented Sep 11, 2024

Uh oh!

bleroux commented Sep 11, 2024

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PurplePolyhedron Jul 19, 2024 •

edited

Loading

PurplePolyhedron commented Jul 25, 2024 •

edited

Loading

PurplePolyhedron commented Jul 30, 2024 •

edited

Loading

PurplePolyhedron left a comment •

edited

Loading

PurplePolyhedron commented Sep 10, 2024 •

edited

Loading