-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Make sure that a DatePickerDialog doesn't crash in 0x0 environment #173677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sure that a DatePickerDialog doesn't crash in 0x0 environment #173677
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a regression test to ensure that DatePickerDialog does not crash when rendered in a zero-sized area, addressing issue #6537. The new test case is a good addition. I've suggested making the test's expectations more explicit to improve its robustness and align with testing best practices.
dkwingsmt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
victorsanni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the PR.
| testWidgets('DatePickerDialog renders at zero area', (WidgetTester tester) async { | ||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| home: SizedBox.shrink( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this test isn't actually testing an 0x0 dialog, if MaterialApp gives a tight BoxConstraints to the SizedBox. Could you double check the size of the DatePickerDialog after it's laid out and see if it's indeed 0x0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print(tester.getSize(find.byType(DatePickerDialog))); prints Size(800.0, 600.0)
Please check this: #6537 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's supposed to be 0x0 right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a Scaffold or a Center to the DatePickerDialog, and verifies in the test that the final size of the DatePickerDialog is indeed Size.zero?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is no need to do this, according to: #6537 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure that a DatePickerDialog doesn't crash in 0x0 environment
The test you added is to verify that DatePickerDialog doesn't crash in 0x0 environment, I think you want to make sure its size is Size.zero right? Otherwise you're just verifying the wrong thing in the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sir, did you ever read that #6537 (comment)? Currently, I am working on this issue according to that comment. If you see that my pull request doesn't follow that comment, your objections are welcome; otherwise, you could show your objections in the issue itself and not here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No offense but I agree that @LongCatIsLooong has a point. It's not about verifying the final size of DatePickerDialog to be zero. It's that the current way the tests are written does not even apply a zero size constraint to the target at all, therefore nothing is being tested. The outer widget MaterialApp uses a tight constraint Size(800, 600) to its child, meaning that the child will be of size 800x600 whatsoever even with SizedBox.shrink. In other words, the SizedBox.shrink here is a noop.
It's not technically necessary to add an expect that verifies the widget size to be zero, although adding one would make it extra safer. But we really need to add something between MaterialApp and the target widget, such as:
testWidgets('DatePickerDialog renders at zero area', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Center(
child: SizedBox.shrink(
child: DatePickerDialog(firstDate: firstDate, lastDate: lastDate),
),
),
),
);
// print(tester.getSize(find.byType(DatePickerDialog)));
});Only by adding the extra Center can we really create a 0x0 environment, which you can manually verify with that print - or automatically verify with an extra expect.
flutter/flutter@f4334d2...52af7a5 2025-08-15 engine-flutter-autoroll@skia.org Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833) 2025-08-15 engine-flutter-autoroll@skia.org Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822) 2025-08-15 jason-simmons@users.noreply.github.com Update the RBE configuration for the recent Clang update (flutter/flutter#173803) 2025-08-15 matanlurey@users.noreply.github.com Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793) 2025-08-15 matanlurey@users.noreply.github.com Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799) 2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798) 2025-08-14 mdebbar@google.com [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782) 2025-08-14 ahmedsameha1@gmail.com Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515) 2025-08-14 ahmedsameha1@gmail.com Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627) 2025-08-14 ahmedsameha1@gmail.com Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677) 2025-08-14 robert.ancell@canonical.com Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757) 2025-08-14 matanlurey@users.noreply.github.com Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797) 2025-08-14 jmccandless@google.com Predictive back route transitions by default (flutter/flutter#165832) 2025-08-14 houssemeddinefadhli81@gmail.com feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221) 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792) 2025-08-14 31859944+LongCatIsLooong@users.noreply.github.com Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148) 2025-08-14 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784) 2025-08-14 jhy03261997@gmail.com [Range slider] Tap on active range, the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725) 2025-08-14 engine-flutter-autoroll@skia.org Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789) 2025-08-14 ttankkeo112@gmail.com Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293) 2025-08-14 matanlurey@users.noreply.github.com Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790) 2025-08-14 victorsanniay@gmail.com [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480) 2025-08-14 bruno.leroux@gmail.com Fix InputDecorator label padding (flutter/flutter#173344) 2025-08-14 edwinzn9@gmail.com Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438) 2025-08-14 matanlurey@users.noreply.github.com Thread sub-builders for every engine-uploading builder (flutter/flutter#173742) 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 louisehsu@google.com,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
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
flutter/flutter@f4334d2...52af7a5 2025-08-15 engine-flutter-autoroll@skia.org Roll Packages from 09533b7 to 5c52c55 (6 revisions) (flutter/flutter#173854) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 46ec77ae3954 to 5654ac32ede0 (1 revision) (flutter/flutter#173848) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from 162f47d6b6bd to 46ec77ae3954 (2 revisions) (flutter/flutter#173833) 2025-08-15 engine-flutter-autoroll@skia.org Roll Dart SDK from c7faab270f27 to cc008dc8e7aa (2 revisions) (flutter/flutter#173826) 2025-08-15 engine-flutter-autoroll@skia.org Roll Skia from ad5d04000101 to 162f47d6b6bd (5 revisions) (flutter/flutter#173822) 2025-08-15 jason-simmons@users.noreply.github.com Update the RBE configuration for the recent Clang update (flutter/flutter#173803) 2025-08-15 matanlurey@users.noreply.github.com Stop writing legacy `FLUTTER_ROOT/version` file (by default?) (flutter/flutter#172793) 2025-08-15 matanlurey@users.noreply.github.com Remove `luci_flags.parallel_download_builds` and friends (flutter/flutter#173799) 2025-08-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Predictive back route transitions by default (#165832)" (flutter/flutter#173809) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from dca5f05fee87 to ad5d04000101 (8 revisions) (flutter/flutter#173798) 2025-08-14 mdebbar@google.com [web] Cleanup usages of deprecated `routeUpdated` message (flutter/flutter#173782) 2025-08-14 ahmedsameha1@gmail.com Make sure that DataTable, DataColumn, DataRow, and DataCell don't crash in 0x0 environment (flutter/flutter#173515) 2025-08-14 ahmedsameha1@gmail.com Make sure that a TableRowInkWell doesn't crash in 0x0 environment (flutter/flutter#173627) 2025-08-14 ahmedsameha1@gmail.com Make sure that a DatePickerDialog doesn't crash in 0x0 environment (flutter/flutter#173677) 2025-08-14 robert.ancell@canonical.com Return result of setting OpenGL contexts back to Flutter (flutter/flutter#173757) 2025-08-14 matanlurey@users.noreply.github.com Read `bin/cache/flutter.version.json` instead of `version` for `flutter_gallery` (flutter/flutter#173797) 2025-08-14 jmccandless@google.com Predictive back route transitions by default (flutter/flutter#165832) 2025-08-14 houssemeddinefadhli81@gmail.com feat: add onLongPressUp callback to InkWell widget (flutter/flutter#173221) 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 214a7f829913 to c7faab270f27 (1 revision) (flutter/flutter#173792) 2025-08-14 31859944+LongCatIsLooong@users.noreply.github.com Add error handling for `Element` lifecycle user callbacks (flutter/flutter#173148) 2025-08-14 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I1TfNmsqTp7t3rO8e... to zWRpLglb48zC1vZLv... (flutter/flutter#173784) 2025-08-14 jhy03261997@gmail.com [Range slider] Tap on active range, the thumb closest to the mouse cursor should move to the cursor position. (flutter/flutter#173725) 2025-08-14 engine-flutter-autoroll@skia.org Roll Packages from 6cb9113 to 09533b7 (4 revisions) (flutter/flutter#173789) 2025-08-14 ttankkeo112@gmail.com Implements the Android native stretch effect as a fragment shader (Impeller-only). (flutter/flutter#169293) 2025-08-14 matanlurey@users.noreply.github.com Sync `CHANGELOG.md` (3.35 -> `master`) (flutter/flutter#173790) 2025-08-14 victorsanniay@gmail.com [VPAT][A11y] Announce Autocomplete search results status (flutter/flutter#173480) 2025-08-14 bruno.leroux@gmail.com Fix InputDecorator label padding (flutter/flutter#173344) 2025-08-14 edwinzn9@gmail.com Fix default minimumSize in dropdownMenu when maximumSize is null (flutter/flutter#169438) 2025-08-14 matanlurey@users.noreply.github.com Thread sub-builders for every engine-uploading builder (flutter/flutter#173742) 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 louisehsu@google.com,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
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
…lutter#173677) This is my attempt to handle flutter#6537 for the DatePickerDialog UI control.
This is my attempt to handle #6537 for the DatePickerDialog UI control.