Unverified Commit 0bc0cc61 authored by chunhtai's avatar chunhtai Committed by GitHub

Fix crash when widgetspan does not produce a semantics node in render… (#69919)

* Fix crash when widgetspan does not produce a semantics node in render paragraph

* fix comments

* fixed corner case, added test, refactored semantics widget

* addressing comment
parent a8f9d4ce
...@@ -54,6 +54,32 @@ class TextParentData extends ContainerBoxParentData<RenderBox> { ...@@ -54,6 +54,32 @@ class TextParentData extends ContainerBoxParentData<RenderBox> {
} }
} }
/// Used by the [RenderParagraph] to map its rendering children to their
/// corresponding semantics nodes.
///
/// The [RichText] uses this to tag the relation between its placeholder spans
/// and their semantics nodes.
@immutable
class PlaceholderSpanIndexSemanticsTag extends SemanticsTag {
/// Creates a semantics tag with the input `index`.
///
/// Different [PlaceholderSpanIndexSemanticsTag]s with the same `index` are
/// consider the same.
const PlaceholderSpanIndexSemanticsTag(this.index) : super('PlaceholderSpanIndexSemanticsTag($index)');
/// The index of this tag.
final int index;
@override
bool operator ==(Object other) {
return other is PlaceholderSpanIndexSemanticsTag
&& other.index == index;
}
@override
int get hashCode => hashValues(PlaceholderSpanIndexSemanticsTag, index);
}
/// A render object that displays a paragraph of text. /// A render object that displays a paragraph of text.
class RenderParagraph extends RenderBox class RenderParagraph extends RenderBox
with ContainerRenderObjectMixin<RenderBox, TextParentData>, with ContainerRenderObjectMixin<RenderBox, TextParentData>,
...@@ -878,6 +904,7 @@ class RenderParagraph extends RenderBox ...@@ -878,6 +904,7 @@ class RenderParagraph extends RenderBox
double ordinal = 0.0; double ordinal = 0.0;
int start = 0; int start = 0;
int placeholderIndex = 0; int placeholderIndex = 0;
int childIndex = 0;
RenderBox? child = firstChild; RenderBox? child = firstChild;
final Queue<SemanticsNode> newChildCache = Queue<SemanticsNode>(); final Queue<SemanticsNode> newChildCache = Queue<SemanticsNode>();
for (final InlineSpanSemanticsInformation info in _combineSemanticsInfo()) { for (final InlineSpanSemanticsInformation info in _combineSemanticsInfo()) {
...@@ -915,8 +942,11 @@ class RenderParagraph extends RenderBox ...@@ -915,8 +942,11 @@ class RenderParagraph extends RenderBox
); );
if (info.isPlaceholder) { if (info.isPlaceholder) {
if (children.isNotEmpty) { // A placeholder span may have 0 to multple semantics nodes, we need
final SemanticsNode childNode = children.elementAt(placeholderIndex++); // to annotate all of the semantics nodes belong to this span.
while (children.length > childIndex &&
children.elementAt(childIndex).isTagged(PlaceholderSpanIndexSemanticsTag(placeholderIndex))) {
final SemanticsNode childNode = children.elementAt(childIndex);
final TextParentData parentData = child!.parentData! as TextParentData; final TextParentData parentData = child!.parentData! as TextParentData;
childNode.rect = Rect.fromLTWH( childNode.rect = Rect.fromLTWH(
childNode.rect.left, childNode.rect.left,
...@@ -925,8 +955,10 @@ class RenderParagraph extends RenderBox ...@@ -925,8 +955,10 @@ class RenderParagraph extends RenderBox
childNode.rect.height * parentData.scale!, childNode.rect.height * parentData.scale!,
); );
newChildren.add(childNode); newChildren.add(childNode);
child = childAfter(child); childIndex += 1;
} }
child = childAfter(child!);
placeholderIndex += 1;
} else { } else {
final SemanticsConfiguration configuration = SemanticsConfiguration() final SemanticsConfiguration configuration = SemanticsConfiguration()
..sortKey = OrdinalSortKey(ordinal++) ..sortKey = OrdinalSortKey(ordinal++)
...@@ -962,6 +994,10 @@ class RenderParagraph extends RenderBox ...@@ -962,6 +994,10 @@ class RenderParagraph extends RenderBox
newChildren.add(newChild); newChildren.add(newChild);
} }
} }
// Makes sure we annotated all of the semantics children.
assert(childIndex == children.length);
assert(child == null);
_cachedChildNodes = newChildCache; _cachedChildNodes = newChildCache;
node.updateWith(config: config, childrenInInversePaintOrder: newChildren); node.updateWith(config: config, childrenInInversePaintOrder: newChildren);
} }
......
...@@ -3600,6 +3600,7 @@ class RenderSemanticsAnnotations extends RenderProxyBox { ...@@ -3600,6 +3600,7 @@ class RenderSemanticsAnnotations extends RenderProxyBox {
SemanticsHintOverrides? hintOverrides, SemanticsHintOverrides? hintOverrides,
TextDirection? textDirection, TextDirection? textDirection,
SemanticsSortKey? sortKey, SemanticsSortKey? sortKey,
SemanticsTag? tagForChildren,
VoidCallback? onTap, VoidCallback? onTap,
VoidCallback? onDismiss, VoidCallback? onDismiss,
VoidCallback? onLongPress, VoidCallback? onLongPress,
...@@ -3655,6 +3656,7 @@ class RenderSemanticsAnnotations extends RenderProxyBox { ...@@ -3655,6 +3656,7 @@ class RenderSemanticsAnnotations extends RenderProxyBox {
_hintOverrides = hintOverrides, _hintOverrides = hintOverrides,
_textDirection = textDirection, _textDirection = textDirection,
_sortKey = sortKey, _sortKey = sortKey,
_tagForChildren = tagForChildren,
_onTap = onTap, _onTap = onTap,
_onLongPress = onLongPress, _onLongPress = onLongPress,
_onScrollLeft = onScrollLeft, _onScrollLeft = onScrollLeft,
...@@ -4072,6 +4074,16 @@ class RenderSemanticsAnnotations extends RenderProxyBox { ...@@ -4072,6 +4074,16 @@ class RenderSemanticsAnnotations extends RenderProxyBox {
markNeedsSemanticsUpdate(); markNeedsSemanticsUpdate();
} }
/// Adds a semenatics tag to the semantics subtree.
SemanticsTag? get tagForChildren => _tagForChildren;
SemanticsTag? _tagForChildren;
set tagForChildren(SemanticsTag? value) {
if (_tagForChildren == value)
return;
markNeedsSemanticsUpdate();
_tagForChildren = value;
}
/// The handler for [SemanticsAction.tap]. /// The handler for [SemanticsAction.tap].
/// ///
/// This is the semantic equivalent of a user briefly tapping the screen with /// This is the semantic equivalent of a user briefly tapping the screen with
...@@ -4549,6 +4561,8 @@ class RenderSemanticsAnnotations extends RenderProxyBox { ...@@ -4549,6 +4561,8 @@ class RenderSemanticsAnnotations extends RenderProxyBox {
config.textDirection = textDirection; config.textDirection = textDirection;
if (sortKey != null) if (sortKey != null)
config.sortKey = sortKey; config.sortKey = sortKey;
if (tagForChildren != null)
config.addTagForChildren(tagForChildren!);
// Registering _perform* as action handlers instead of the user provided // Registering _perform* as action handlers instead of the user provided
// ones to ensure that changing a user provided handler from a non-null to // ones to ensure that changing a user provided handler from a non-null to
// another non-null value doesn't require a semantics update. // another non-null value doesn't require a semantics update.
......
...@@ -610,6 +610,7 @@ class SemanticsProperties extends DiagnosticableTree { ...@@ -610,6 +610,7 @@ class SemanticsProperties extends DiagnosticableTree {
this.hintOverrides, this.hintOverrides,
this.textDirection, this.textDirection,
this.sortKey, this.sortKey,
this.tagForChildren,
this.onTap, this.onTap,
this.onLongPress, this.onLongPress,
this.onScrollLeft, this.onScrollLeft,
...@@ -913,6 +914,22 @@ class SemanticsProperties extends DiagnosticableTree { ...@@ -913,6 +914,22 @@ class SemanticsProperties extends DiagnosticableTree {
/// on iOS and TalkBack on Android). /// on iOS and TalkBack on Android).
final SemanticsSortKey? sortKey; final SemanticsSortKey? sortKey;
/// A tag to be applied to the child [SemanticsNode]s of this widget.
///
/// The tag is added to all child [SemanticsNode]s that pass through the
/// [RenderObject] corresponding to this widget while looking to be attached
/// to a parent SemanticsNode.
///
/// Tags are used to communicate to a parent SemanticsNode that a child
/// SemanticsNode was passed through a particular RenderObject. The parent can
/// use this information to determine the shape of the semantics tree.
///
/// See also:
///
/// * [SemanticsConfiguration.addTagForChildren], to which the tags provided
/// here will be passed.
final SemanticsTag? tagForChildren;
/// The handler for [SemanticsAction.tap]. /// The handler for [SemanticsAction.tap].
/// ///
/// This is the semantic equivalent of a user briefly tapping the screen with /// This is the semantic equivalent of a user briefly tapping the screen with
......
...@@ -5456,10 +5456,14 @@ class RichText extends MultiChildRenderObjectWidget { ...@@ -5456,10 +5456,14 @@ class RichText extends MultiChildRenderObjectWidget {
// Traverses the InlineSpan tree and depth-first collects the list of // Traverses the InlineSpan tree and depth-first collects the list of
// child widgets that are created in WidgetSpans. // child widgets that are created in WidgetSpans.
static List<Widget> _extractChildren(InlineSpan span) { static List<Widget> _extractChildren(InlineSpan span) {
int index = 0;
final List<Widget> result = <Widget>[]; final List<Widget> result = <Widget>[];
span.visitChildren((InlineSpan span) { span.visitChildren((InlineSpan span) {
if (span is WidgetSpan) { if (span is WidgetSpan) {
result.add(span.child); result.add(Semantics(
tagForChildren: PlaceholderSpanIndexSemanticsTag(index++),
child: span.child,
));
} }
return true; return true;
}); });
...@@ -6890,6 +6894,7 @@ class Semantics extends SingleChildRenderObjectWidget { ...@@ -6890,6 +6894,7 @@ class Semantics extends SingleChildRenderObjectWidget {
String? onLongPressHint, String? onLongPressHint,
TextDirection? textDirection, TextDirection? textDirection,
SemanticsSortKey? sortKey, SemanticsSortKey? sortKey,
SemanticsTag? tagForChildren,
VoidCallback? onTap, VoidCallback? onTap,
VoidCallback? onLongPress, VoidCallback? onLongPress,
VoidCallback? onScrollLeft, VoidCallback? onScrollLeft,
...@@ -6944,6 +6949,7 @@ class Semantics extends SingleChildRenderObjectWidget { ...@@ -6944,6 +6949,7 @@ class Semantics extends SingleChildRenderObjectWidget {
hint: hint, hint: hint,
textDirection: textDirection, textDirection: textDirection,
sortKey: sortKey, sortKey: sortKey,
tagForChildren: tagForChildren,
onTap: onTap, onTap: onTap,
onLongPress: onLongPress, onLongPress: onLongPress,
onScrollLeft: onScrollLeft, onScrollLeft: onScrollLeft,
...@@ -7060,6 +7066,7 @@ class Semantics extends SingleChildRenderObjectWidget { ...@@ -7060,6 +7066,7 @@ class Semantics extends SingleChildRenderObjectWidget {
hintOverrides: properties.hintOverrides, hintOverrides: properties.hintOverrides,
textDirection: _getTextDirection(context), textDirection: _getTextDirection(context),
sortKey: properties.sortKey, sortKey: properties.sortKey,
tagForChildren: properties.tagForChildren,
onTap: properties.onTap, onTap: properties.onTap,
onLongPress: properties.onLongPress, onLongPress: properties.onLongPress,
onScrollLeft: properties.onScrollLeft, onScrollLeft: properties.onScrollLeft,
...@@ -7131,6 +7138,7 @@ class Semantics extends SingleChildRenderObjectWidget { ...@@ -7131,6 +7138,7 @@ class Semantics extends SingleChildRenderObjectWidget {
..namesRoute = properties.namesRoute ..namesRoute = properties.namesRoute
..textDirection = _getTextDirection(context) ..textDirection = _getTextDirection(context)
..sortKey = properties.sortKey ..sortKey = properties.sortKey
..tagForChildren = properties.tagForChildren
..onTap = properties.onTap ..onTap = properties.onTap
..onLongPress = properties.onLongPress ..onLongPress = properties.onLongPress
..onScrollLeft = properties.onScrollLeft ..onScrollLeft = properties.onScrollLeft
......
...@@ -387,6 +387,54 @@ void main() { ...@@ -387,6 +387,54 @@ void main() {
semantics.dispose(); semantics.dispose();
}); });
testWidgets('Semantics tagForChildren works', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Semantics(
container: true,
tagForChildren: const SemanticsTag('custom tag'),
child: Column(
children: <Widget>[
Semantics(
container: true,
child: const Text('child 1'),
),
Semantics(
container: true,
child: const Text('child 2'),
),
],
),
),
),
);
final TestSemantics expectedSemantics = TestSemantics.root(
children: <TestSemantics>[
TestSemantics.rootChild(
children: <TestSemantics>[
TestSemantics(
label: 'child 1',
tags: <SemanticsTag>[const SemanticsTag('custom tag')],
textDirection: TextDirection.ltr,
),
TestSemantics(
label: 'child 2',
tags: <SemanticsTag>[const SemanticsTag('custom tag')],
textDirection: TextDirection.ltr,
),
]
),
],
);
expect(semantics, hasSemantics(expectedSemantics, ignoreTransform: true, ignoreRect: true, ignoreId: true));
semantics.dispose();
});
testWidgets('Semantics widget supports all actions', (WidgetTester tester) async { testWidgets('Semantics widget supports all actions', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester); final SemanticsTester semantics = SemanticsTester(tester);
......
...@@ -1039,6 +1039,176 @@ void main() { ...@@ -1039,6 +1039,176 @@ void main() {
], ],
))); )));
}, semanticsEnabled: true, skip: isBrowser); // Browser semantics have different sizes. }, semanticsEnabled: true, skip: isBrowser); // Browser semantics have different sizes.
// Regression test for https://github.com/flutter/flutter/issues/69787
testWidgets('WidgetSpans with no semantic information are elided from semantics - case 2', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: RichText(
text: TextSpan(children: <InlineSpan>[
const WidgetSpan(child: SizedBox.shrink()),
const WidgetSpan(child: Text('included')),
TextSpan(
text: 'HELLO',
style: const TextStyle(color: Colors.black),
recognizer: TapGestureRecognizer()..onTap = () {},
),
const WidgetSpan(child: Text('included2')),
]),
),
)
);
expect(semantics, hasSemantics(TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(label: 'included'),
TestSemantics(
label: 'HELLO',
actions: <SemanticsAction>[
SemanticsAction.tap,
],
flags: <SemanticsFlag>[
SemanticsFlag.isLink,
],
),
TestSemantics(label: 'included2'),
],
),
],
),
ignoreId: true,
ignoreRect: true,
ignoreTransform: true,
));
}, semanticsEnabled: true, skip: isBrowser); // Browser does not support widget span.
// Regression test for https://github.com/flutter/flutter/issues/69787
testWidgets('WidgetSpans with no semantic information are elided from semantics - case 3', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: RichText(
text: TextSpan(children: <InlineSpan>[
const WidgetSpan(child: SizedBox.shrink()),
WidgetSpan(
child: Row(
children: <Widget>[
Semantics(
container: true,
child: const Text('foo'),
),
Semantics(
container: true,
child: const Text('bar'),
),
],
),
),
TextSpan(
text: 'HELLO',
style: const TextStyle(color: Colors.black),
recognizer: TapGestureRecognizer()..onTap = () {},
),
]),
),
)
);
expect(semantics, hasSemantics(TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(label: 'foo'),
TestSemantics(label: 'bar'),
TestSemantics(
label: 'HELLO',
actions: <SemanticsAction>[
SemanticsAction.tap,
],
flags: <SemanticsFlag>[
SemanticsFlag.isLink,
],
),
],
),
],
),
ignoreId: true,
ignoreRect: true,
ignoreTransform: true,
));
}, semanticsEnabled: true, skip: isBrowser); // Browser does not support widget span.
// Regression test for https://github.com/flutter/flutter/issues/69787
testWidgets('WidgetSpans with no semantic information are elided from semantics - case 4', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: ClipRect(
child: Container(
color: Colors.green,
height: 100,
width: 100,
child: OverflowBox(
alignment: Alignment.topLeft,
maxWidth: double.infinity,
child: RichText(
text: TextSpan(
children: <InlineSpan>[
const WidgetSpan(
child: Icon(
Icons.edit,
size: 16,
semanticLabel: 'not clipped',
),
),
TextSpan(
text: 'next WS is clipped',
recognizer: TapGestureRecognizer()..onTap = () { },
),
const WidgetSpan(
child: Icon(
Icons.edit,
size: 16,
semanticLabel: 'clipped',
),
),
],
),
),
),
),
),
),
)
);
expect(semantics, hasSemantics(TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(label: 'not clipped'),
TestSemantics(
label: 'next WS is clipped',
flags: <SemanticsFlag>[SemanticsFlag.isLink],
actions: <SemanticsAction>[SemanticsAction.tap],
),
],
),
],
),
ignoreId: true,
ignoreRect: true,
ignoreTransform: true,
));
}, semanticsEnabled: true, skip: isBrowser); // Browser does not support widget span
} }
Future<void> _pumpTextWidget({ Future<void> _pumpTextWidget({
......
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