Make sure that a DesktopTextSelectionToolbar doesn't crash in 0x0 env… #173928

ahmedsameha1 · 2025-08-17T13:39:29Z

This is my attempt to handle #6537 for the DesktopTextSelectionToolbar UI control.

gemini-code-assist

Code Review

This pull request adds a test case to ensure that DesktopTextSelectionToolbar does not crash when rendered in a zero-sized environment, which is a good addition to prevent regressions. My feedback focuses on improving the new test by adding an explicit assertion to make its intent clearer and more robust.

gemini-code-assist · 2025-08-17T13:40:25Z

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

+  testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      MaterialApp(
+        home: SizedBox.shrink(
+          child: DesktopTextSelectionToolbar(
+            anchor: const Offset(10, 10),
+            children: const <Widget>[Text('X')],
+          ),
+        ),
+      ),
+    );
+  });


While this test correctly checks that pumping the widget in a zero-sized area doesn't cause a crash, it would be more robust and clearer to add an assertion to verify that the widget is actually present in the widget tree.¹

Adding an expect call ensures that the test is not passing simply because the widget failed to render silently, making the test's purpose more explicit.

testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async { await tester.pumpWidget( MaterialApp( home: SizedBox.shrink( child: DesktopTextSelectionToolbar( anchor: const Offset(10, 10), children: const <Widget>[Text('X')], ), ), ), ); expect(find.byType(DesktopTextSelectionToolbar), findsOneWidget); });

Style Guide References

Footnotes

Tests should follow the guidance in the 'writing effective tests guide'. Tests are generally expected to have assertions to verify behavior, not just the absence of crashes. (link) ↩

I think this is fine either way.

Actually, in all of these PRs, we should have an a expect that checks that the relevant widget (DesktopTextSelectionToolbar here) has a size of 0x0. Can you add that @ahmedsameha1?

Thanks @LongCatIsLooong for flagging this.

@justinmc We don't need any expect. The original issue only wants to make sure that it doesn't crash, not even needing the actual size to be zero.

justinmc

LGTM 👍

@dkwingsmt During triage you mentioned you were concerned about this test, were you wondering if the toolbar would end up in an Overlay or what was it?

justinmc · 2025-08-20T19:46:28Z

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

+  testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      MaterialApp(
+        home: SizedBox.shrink(
+          child: DesktopTextSelectionToolbar(
+            anchor: const Offset(10, 10),
+            children: const <Widget>[Text('X')],
+          ),
+        ),
+      ),
+    );
+  });


I think this is fine either way.

dkwingsmt · 2025-08-20T20:12:57Z

Yeah, I was saying I wanted someone familiar with this to verify its validity, such as making sure it's not laying out overlay.

justinmc · 2025-08-20T20:53:52Z

In that case we are good, the Overlay thing is done in text_selection.dart, not as a part of DesktopTextSelectionToolbar itself 👍 . Thanks for clarifying!

justinmc · 2025-08-21T19:57:31Z

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

+  testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      MaterialApp(
+        home: SizedBox.shrink(
+          child: DesktopTextSelectionToolbar(
+            anchor: const Offset(10, 10),
+            children: const <Widget>[Text('X')],
+          ),
+        ),
+      ),
+    );
+  });


Actually, in all of these PRs, we should have an a expect that checks that the relevant widget (DesktopTextSelectionToolbar here) has a size of 0x0. Can you add that @ahmedsameha1?

Thanks @LongCatIsLooong for flagging this.

ahmedsameha1 · 2025-08-21T20:59:44Z

@justinmc Could you resolve this in #6537 because, according to #6537 (comment), there is no need to add that expectation.
Please consider this: #172074 (comment)

ahmedsameha1 · 2025-08-25T09:52:41Z

I have updated this pull request according to #6537 (comment).

dkwingsmt

LGTM, thank you!

justinmc

LGTM 👍

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

dkwingsmt

Checklist:

The test is in the correct file
The test name goes “does not crash at zero area”
The target widget is wrapped by Center (or is fullscreen)
The target widget does not have an overlay, or the overlay is tested
The target widget is expected to have a size of exactly Size.zero

dkwingsmt · 2025-09-18T04:41:23Z

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

    expect(tester.getTopLeft(find.byType(DesktopTextSelectionToolbarButton)), anchor);
  });
+
+  testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {


Suggested change

testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {

testWidgets('DesktopTextSelectionToolbar does not crash at zero area', (WidgetTester tester) async {

dkwingsmt · 2025-09-18T04:41:32Z

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart

+        ),
+      ),
+    );
+    expect(tester.getSize(find.byType(DesktopTextSelectionToolbar)).isEmpty, isTrue);


Suggested change

expect(tester.getSize(find.byType(DesktopTextSelectionToolbar)).isEmpty, isTrue);

expect(tester.getSize(find.byType(DesktopTextSelectionToolbar)), Size.zero);

auto-submit · 2025-09-18T20:13:56Z

autosubmit label was removed for flutter/flutter/173928, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

auto-submit · 2025-09-23T20:45:46Z

autosubmit label was removed for flutter/flutter/173928, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

Piinks · 2025-09-26T22:18:13Z

Rebasing to retest

…ironment

auto-submit · 2025-09-26T22:21:18Z

autosubmit label was removed for flutter/flutter/173928, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

…in 0x0 env… (flutter/flutter#173928)

Roll Flutter from 6cc976ec26d3 to 96fe3b3df509 (32 revisions) flutter/flutter@6cc976e...96fe3b3 2025-09-29 engine-flutter-autoroll@skia.org Roll Packages from 389c678 to 34eec78 (6 revisions) (flutter/flutter#176205) 2025-09-29 engine-flutter-autoroll@skia.org Roll Skia from 9b2b942d1eb1 to bb3b6bd4be0d (4 revisions) (flutter/flutter#176201) 2025-09-29 bkonyi@google.com [ Widget Preview ] Improve IDE integration support (flutter/flutter#176114) 2025-09-29 robert.ancell@canonical.com Fix name of driver file (flutter/flutter#176186) 2025-09-29 engine-flutter-autoroll@skia.org Roll Skia from beb673968802 to 9b2b942d1eb1 (3 revisions) (flutter/flutter#176190) 2025-09-28 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 0Z45OXT_Wb8aWI3a0... to 8zjcJic_DtvB2Bo2x... (flutter/flutter#176158) 2025-09-28 flar@google.com Revert "[Impeller] Optimize scale translate rectangle transforms" (flutter/flutter#176161) 2025-09-27 engine-flutter-autoroll@skia.org Roll Skia from 2e5da5c0a9cd to beb673968802 (1 revision) (flutter/flutter#176145) 2025-09-27 flar@google.com [Impeller] Optimize scale translate rectangle transforms (flutter/flutter#176123) 2025-09-27 engine-flutter-autoroll@skia.org Roll Skia from d8422aaf8f89 to 2e5da5c0a9cd (2 revisions) (flutter/flutter#176141) 2025-09-27 mdebbar@google.com [web] Remove mention of non-existent `canvaskit_lock.yaml` (flutter/flutter#176108) 2025-09-27 engine-flutter-autoroll@skia.org Roll Skia from 96b73f61fe61 to d8422aaf8f89 (2 revisions) (flutter/flutter#176118) 2025-09-27 sokolovskyi.konstantin@gmail.com [a11y] Add `expanded` flag support to Android. (flutter/flutter#174981) 2025-09-26 ahmedsameha1@gmail.com Make sure that a DesktopTextSelectionToolbar doesn't crash in 0x0 env… (flutter/flutter#173928) 2025-09-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 899c7340cc4c to af31d2637b6b (11 revisions) (flutter/flutter#176056) 2025-09-26 1063596+reidbaker@users.noreply.github.com Update java version ranges with the top end limitation for java pre 17 (flutter/flutter#176049) 2025-09-26 1063596+reidbaker@users.noreply.github.com Add warn java evaluation to android_workflow (flutter/flutter#176097) 2025-09-26 katelovett@google.com Removes type annotations in templates (flutter/flutter#176106) 2025-09-26 fluttergithubbot@gmail.com Marks Linux_pixel_7pro static_path_stroke_tessellation_perf__timeline_summary to be unflaky (flutter/flutter#175917) 2025-09-26 1063596+reidbaker@users.noreply.github.com Add kotlin/kgp 2.2.* evaluation criteria. (flutter/flutter#176094) 2025-09-26 32538273+ValentinVignal@users.noreply.github.com Migrate to `WidgetStateMouseCursor` (flutter/flutter#175981) 2025-09-26 engine-flutter-autoroll@skia.org Roll Packages from 117bf63 to 389c678 (4 revisions) (flutter/flutter#176092) 2025-09-26 34871572+gmackall@users.noreply.github.com Fix link to .gclient setup instructions (flutter/flutter#176046) 2025-09-26 matt.kosarek@canonical.com Implement Regular Windows for the win32 framework + add an example application for regular windows (flutter/flutter#173715) 2025-09-26 engine-flutter-autoroll@skia.org Roll Skia from 5d99c3fc7c83 to 96b73f61fe61 (3 revisions) (flutter/flutter#176075) 2025-09-26 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from naeytagBIBEpKgZNZ... to 0Z45OXT_Wb8aWI3a0... (flutter/flutter#176068) 2025-09-26 100504385+AlsoShantanuBorkar@users.noreply.github.com Add itemClipBehavior property for CarouselView's children (flutter/flutter#175324) 2025-09-26 engine-flutter-autoroll@skia.org Roll Skia from 55436d87e414 to 5d99c3fc7c83 (4 revisions) (flutter/flutter#176060) 2025-09-26 flar@google.com Revert "[Impeller] Optimize scale translate rectangle transforms" (flutter/flutter#176061) 2025-09-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reapply "Update the AccessibilityPlugin::Announce method to account f… (#174365)" (flutter/flutter#176059) 2025-09-25 ahmedsameha1@gmail.com Make sure that a CupertinoDesktopTextSelectionToolbarButton doesn't c… (flutter/flutter#173894) 2025-09-25 mohellebiabdessalem@gmail.com Improve code quality in `SensitiveContentPluginTest.java` (flutter/flutter#175721) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...

…in 0x0 env… (flutter/flutter#173928)

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 17, 2025

gemini-code-assist bot reviewed Aug 17, 2025

View reviewed changes

dkwingsmt requested review from dkwingsmt and justinmc August 20, 2025 18:39

justinmc approved these changes Aug 20, 2025

View reviewed changes

justinmc requested changes Aug 21, 2025

View reviewed changes

ahmedsameha1 mentioned this pull request Aug 23, 2025

Some of our controls can't handle being built in 0x0 environments #6537

Open

ahmedsameha1 force-pushed the handle#6537-DesktopTextSelectionToolbar branch from ec9ebf3 to 2f5d274 Compare August 25, 2025 09:51

dkwingsmt approved these changes Aug 25, 2025

View reviewed changes

LongCatIsLooong mentioned this pull request Aug 26, 2025

Make sure that a Dialog doesn't crash in 0x0 environment #174023

Merged

dkwingsmt self-requested a review August 27, 2025 18:35

justinmc approved these changes Aug 28, 2025

View reviewed changes

packages/flutter/test/material/desktop_text_selection_toolbar_test.dart Show resolved Hide resolved

dkwingsmt reviewed Sep 18, 2025

View reviewed changes

dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2025

dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025

Make sure that a DesktopTextSelectionToolbar doesn't crash in 0x0 env…

a59b80a

…ironment

Piinks force-pushed the handle#6537-DesktopTextSelectionToolbar branch from 6cf806e to a59b80a Compare September 26, 2025 22:18

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

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 26, 2025

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

auto-submit bot added this pull request to the merge queue Sep 26, 2025

Merged via the queue into flutter:master with commit fc9a6fa Sep 27, 2025
79 of 80 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 27, 2025

engine-flutter-autoroll mentioned this pull request Sep 27, 2025

Roll Flutter from b1a28bc065b0 to 55dec6355d9d (26 revisions) flutter/packages#10105

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

60ad5df

…in 0x0 env… (flutter/flutter#173928)

engine-flutter-autoroll mentioned this pull request Sep 27, 2025

Roll Flutter from b1a28bc065b0 to 94cd6c7ea4ba (27 revisions) flutter/packages#10106

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

f278020

…in 0x0 env… (flutter/flutter#173928)

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

e3df65d

…in 0x0 env… (flutter/flutter#173928)

engine-flutter-autoroll mentioned this pull request Sep 28, 2025

Roll Flutter from b1a28bc065b0 to dc56bc26bb1c (30 revisions) flutter/packages#10107

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

8a8bfde

…in 0x0 env… (flutter/flutter#173928)

This was referenced Sep 28, 2025

Roll Flutter from b1a28bc065b0 to 0eb1c8f9c313 (31 revisions) flutter/packages#10108

Closed

Roll Flutter from b1a28bc065b0 to 0eb1c8f9c313 (31 revisions) flutter/packages#10109

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

6bf7c28

…in 0x0 env… (flutter/flutter#173928)

engine-flutter-autoroll mentioned this pull request Sep 29, 2025

Roll Flutter from b1a28bc065b0 to b78356cbac6f (33 revisions) flutter/packages#10118

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

45ff988

…in 0x0 env… (flutter/flutter#173928)

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

c450b03

…in 0x0 env… (flutter/flutter#173928)

This was referenced Sep 29, 2025

Roll Flutter from 6cc976ec26d3 to b78356cbac6f (29 revisions) flutter/packages#10121

Closed

Roll Flutter from 6cc976ec26d3 to 3d6b5994b86b (30 revisions) flutter/packages#10122

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

05275f2

…in 0x0 env… (flutter/flutter#173928)

engine-flutter-autoroll mentioned this pull request Sep 29, 2025

Roll Flutter from 6cc976ec26d3 to 1e2447c604e5 (31 revisions) flutter/packages#10123

Closed

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

3713606

…in 0x0 env… (flutter/flutter#173928)

engine-flutter-autoroll mentioned this pull request Sep 29, 2025

Roll Flutter from 6cc976ec26d3 to 96fe3b3df509 (32 revisions) flutter/packages#10125

Merged

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

f5bbc9d

…in 0x0 env… (flutter/flutter#173928)

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

fc9a6fa6d Make sure that a DesktopTextSelectionToolbar doesn't crash …

a0958bb

…in 0x0 env… (flutter/flutter#173928)

	testWidgets('DesktopTextSelectionToolbar renders at zero area', (WidgetTester tester) async {
	testWidgets('DesktopTextSelectionToolbar does not crash at zero area', (WidgetTester tester) async {

	expect(tester.getSize(find.byType(DesktopTextSelectionToolbar)).isEmpty, isTrue);
	expect(tester.getSize(find.byType(DesktopTextSelectionToolbar)), Size.zero);

Make sure that a DesktopTextSelectionToolbar doesn't crash in 0x0 env… #173928

Make sure that a DesktopTextSelectionToolbar doesn't crash in 0x0 env… #173928

Uh oh!

Conversation

ahmedsameha1 commented Aug 17, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 17, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

justinmc Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 21, 2025 • 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!

dkwingsmt Aug 22, 2025 • 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!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

dkwingsmt commented Aug 20, 2025

Uh oh!

justinmc commented Aug 20, 2025

Uh oh!

justinmc Aug 21, 2025 • 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!

ahmedsameha1 commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahmedsameha1 commented Aug 25, 2025

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Sep 18, 2025

Uh oh!

auto-submit bot commented Sep 23, 2025

Uh oh!

Piinks commented Sep 26, 2025

Uh oh!

auto-submit bot commented Sep 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

justinmc Aug 21, 2025 •

edited

Loading

dkwingsmt Aug 22, 2025 •

edited

Loading

justinmc Aug 21, 2025 •

edited

Loading

ahmedsameha1 commented Aug 21, 2025 •

edited

Loading