Make sure that a Focus doesn't crash in 0x0 environment#180674
Conversation
github-actions
Bot
added
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a test case to verify that the Focus widget does not crash in a zero-sized environment. The test is well-written and correctly reproduces the scenario from issue #6537. However, the pull request is missing the actual implementation of the fix. Without the fix, this new test will fail. Please include the necessary changes to packages/flutter/lib/src/widgets/focus_scope.dart to resolve the issue.
| semantics.dispose(); | ||
| }); | ||
|
|
||
| testWidgets('Focus does not crash at zero area', (WidgetTester tester) async { |
There was a problem hiding this comment.
This test is a good regression test for the crash in a zero-sized environment. However, the pull request is incomplete as it's missing the corresponding fix in packages/flutter/lib/src/widgets/focus_scope.dart.
To fix the issue, you'll need to prevent the onFocus callback from being set when the widget has a zero size. A possible implementation in _FocusState.build could look like this:
// in packages/flutter/lib/src/widgets/focus_scope.dart, _FocusState.build method
final RenderBox? renderBox = context.findRenderObject() as RenderBox?;
final bool hasNonZeroSize = renderBox?.hasSize == true && renderBox!.size != Size.zero;
if (widget.includeSemantics) {
child = Semantics(
onFocus: defaultTargetPlatform != TargetPlatform.iOS && _couldRequestFocus && hasNonZeroSize
? focusNode.requestFocus
: null,
// ... other properties
);
}Please add the fix to this pull request so the new test can pass and the issue can be closed.
victorsanni
left a comment
victorsanni
left a comment
There was a problem hiding this comment.
Does Focus have its own renderbox? I took a look at the source code for _FocusState, in its build method it just wraps the child in an InheritedNotifier + Semantics.
Should I close this PR? |
dkwingsmt
left a comment
dkwingsmt
left a comment
There was a problem hiding this comment.
LGTM. I think it doesn't hurt to add one.
justinmc
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/180674, 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. |
victorsanni
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
This is my attempt to handle flutter#6537 for the Focus widget. Co-authored-by: Victor Sanni <victorsanniay@gmail.com>
4 participants
This is my attempt to handle #6537 for the Focus widget.