Unverified Commit c05f623c authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Remove FocusTraversalGroups from the final sorted list of descendants. (#74758)

This fixes a subtle bug in the focus traversal that would leave unfocusable FocusTraversalGroup nodes in the list of sorted nodes to be traversed, causing an assert to fire if you nested a FocusTraversalGroup in another one and made its children unfocusable.
parent 541188c7
......@@ -335,10 +335,23 @@ abstract class FocusTraversalPolicy with Diagnosticable {
}
}
// Visit the children of the scope.
visitGroups(groups[scopeGroupMarker?.focusNode]!);
// Remove the FocusTraversalGroup nodes themselves, which aren't focusable.
// They were left in above because they were needed to find their members
// during sorting.
sortedDescendants.removeWhere((FocusNode node) {
return !node.canRequestFocus || node.skipTraversal;
});
// Sanity check to make sure that the algorithm above doesn't diverge from
// the one in FocusScopeNode.traversalDescendants in terms of which nodes it
// finds.
assert(
sortedDescendants.length <= scope.traversalDescendants.length && sortedDescendants.toSet().difference(scope.traversalDescendants.toSet()).isEmpty,
'sorted descendants contains more nodes than it should: (${sortedDescendants.toSet().difference(scope.traversalDescendants.toSet())})'
'Sorted descendants contains different nodes than FocusScopeNode.traversalDescendants would. '
'These are the different nodes: ${sortedDescendants.toSet().difference(scope.traversalDescendants.toSet())}'
);
return sortedDescendants;
}
......
......@@ -2088,6 +2088,82 @@ void main() {
expect(containerNode.hasFocus, isFalse);
expect(unfocusableNode.hasFocus, isFalse);
});
testWidgets("Nested FocusTraversalGroup with unfocusable children doesn't assert.", (WidgetTester tester) async {
final GlobalKey key1 = GlobalKey(debugLabel: '1');
final GlobalKey key2 = GlobalKey(debugLabel: '2');
final FocusNode focusNode = FocusNode();
bool? gotFocus;
await tester.pumpWidget(
FocusTraversalGroup(
child: Column(
children: <Widget>[
Focus(
autofocus: true,
child: Container(),
),
FocusTraversalGroup(
descendantsAreFocusable: false,
child: Focus(
onFocusChange: (bool focused) => gotFocus = focused,
child: Focus(
key: key1,
focusNode: focusNode,
child: Container(key: key2),
),
),
),
],
),
),
);
final Element childWidget = tester.element(find.byKey(key1));
final FocusNode unfocusableNode = Focus.of(childWidget);
final Element containerWidget = tester.element(find.byKey(key2));
final FocusNode containerNode = Focus.of(containerWidget);
await tester.pump();
primaryFocus!.nextFocus();
expect(gotFocus, isNull);
expect(containerNode.hasFocus, isFalse);
expect(unfocusableNode.hasFocus, isFalse);
containerNode.requestFocus();
await tester.pump();
expect(gotFocus, isNull);
expect(containerNode.hasFocus, isFalse);
expect(unfocusableNode.hasFocus, isFalse);
});
testWidgets("Empty FocusTraversalGroup doesn't cause an exception.", (WidgetTester tester) async {
final GlobalKey key = GlobalKey(debugLabel: 'Test Key');
final FocusNode focusNode = FocusNode(debugLabel: 'Test Node');
await tester.pumpWidget(
FocusTraversalGroup(
child: Directionality(
textDirection: TextDirection.rtl,
child: Column(
children: <Widget>[
FocusTraversalGroup(
child: Container(key: key),
),
Focus(
focusNode: focusNode,
autofocus: true,
child: Container(),
),
],
),
),
),
);
await tester.pump();
primaryFocus!.nextFocus();
await tester.pump();
expect(primaryFocus, equals(focusNode));
});
});
group(RawKeyboardListener, () {
testWidgets('Raw keyboard listener introduces a Semantics node by default', (WidgetTester tester) async {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment