Unverified Commit 660992a1 authored by chunhtai's avatar chunhtai Committed by GitHub

Fix text.rich to merge widget span (#113461)

* Fix text.rich to merge widget span

* migrate to new API

* update

* addressing comment

* addressing comments

* update

* addressing comment

* Update
parent 56e1bddc
...@@ -15,6 +15,7 @@ import 'package:flutter/semantics.dart'; ...@@ -15,6 +15,7 @@ import 'package:flutter/semantics.dart';
import 'debug.dart'; import 'debug.dart';
import 'layer.dart'; import 'layer.dart';
import 'proxy_box.dart';
export 'package:flutter/foundation.dart' show export 'package:flutter/foundation.dart' show
DiagnosticPropertiesBuilder, DiagnosticPropertiesBuilder,
...@@ -3244,7 +3245,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im ...@@ -3244,7 +3245,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
final SemanticsConfiguration config = _semanticsConfiguration; final SemanticsConfiguration config = _semanticsConfiguration;
bool dropSemanticsOfPreviousSiblings = config.isBlockingSemanticsOfPreviouslyPaintedNodes; bool dropSemanticsOfPreviousSiblings = config.isBlockingSemanticsOfPreviouslyPaintedNodes;
final bool producesForkingFragment = !config.hasBeenAnnotated && !config.isSemanticBoundary; bool producesForkingFragment = !config.hasBeenAnnotated && !config.isSemanticBoundary;
final bool childrenMergeIntoParent = mergeIntoParent || config.isMergingSemanticsOfDescendants; final bool childrenMergeIntoParent = mergeIntoParent || config.isMergingSemanticsOfDescendants;
final List<SemanticsConfiguration> childConfigurations = <SemanticsConfiguration>[]; final List<SemanticsConfiguration> childConfigurations = <SemanticsConfiguration>[];
final bool explicitChildNode = config.explicitChildNodes || parent is! RenderObject; final bool explicitChildNode = config.explicitChildNodes || parent is! RenderObject;
...@@ -3252,6 +3253,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im ...@@ -3252,6 +3253,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
final Map<SemanticsConfiguration, _InterestingSemanticsFragment> configToFragment = <SemanticsConfiguration, _InterestingSemanticsFragment>{}; final Map<SemanticsConfiguration, _InterestingSemanticsFragment> configToFragment = <SemanticsConfiguration, _InterestingSemanticsFragment>{};
final List<_InterestingSemanticsFragment> mergeUpFragments = <_InterestingSemanticsFragment>[]; final List<_InterestingSemanticsFragment> mergeUpFragments = <_InterestingSemanticsFragment>[];
final List<List<_InterestingSemanticsFragment>> siblingMergeFragmentGroups = <List<_InterestingSemanticsFragment>>[]; final List<List<_InterestingSemanticsFragment>> siblingMergeFragmentGroups = <List<_InterestingSemanticsFragment>>[];
final bool hasTags = config.tagsForChildren?.isNotEmpty ?? false;
visitChildrenForSemantics((RenderObject renderChild) { visitChildrenForSemantics((RenderObject renderChild) {
assert(!_needsLayout); assert(!_needsLayout);
final _SemanticsFragment parentFragment = renderChild._getSemanticsForParent( final _SemanticsFragment parentFragment = renderChild._getSemanticsForParent(
...@@ -3267,7 +3269,9 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im ...@@ -3267,7 +3269,9 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
} }
for (final _InterestingSemanticsFragment fragment in parentFragment.mergeUpFragments) { for (final _InterestingSemanticsFragment fragment in parentFragment.mergeUpFragments) {
fragment.addAncestor(this); fragment.addAncestor(this);
fragment.addTags(config.tagsForChildren); if (hasTags) {
fragment.addTags(config.tagsForChildren!);
}
if (hasChildConfigurationsDelegate && fragment.config != null) { if (hasChildConfigurationsDelegate && fragment.config != null) {
// This fragment need to go through delegate to determine whether it // This fragment need to go through delegate to determine whether it
// merge up or not. // merge up or not.
...@@ -3283,7 +3287,9 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im ...@@ -3283,7 +3287,9 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
for (final List<_InterestingSemanticsFragment> siblingMergeGroup in parentFragment.siblingMergeGroups) { for (final List<_InterestingSemanticsFragment> siblingMergeGroup in parentFragment.siblingMergeGroups) {
for (final _InterestingSemanticsFragment siblingMergingFragment in siblingMergeGroup) { for (final _InterestingSemanticsFragment siblingMergingFragment in siblingMergeGroup) {
siblingMergingFragment.addAncestor(this); siblingMergingFragment.addAncestor(this);
siblingMergingFragment.addTags(config.tagsForChildren); if (hasTags) {
siblingMergingFragment.addTags(config.tagsForChildren!);
}
} }
siblingMergeFragmentGroups.add(siblingMergeGroup); siblingMergeFragmentGroups.add(siblingMergeGroup);
} }
...@@ -3296,14 +3302,25 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im ...@@ -3296,14 +3302,25 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
for (final _InterestingSemanticsFragment fragment in mergeUpFragments) { for (final _InterestingSemanticsFragment fragment in mergeUpFragments) {
fragment.markAsExplicit(); fragment.markAsExplicit();
} }
} else if (hasChildConfigurationsDelegate && childConfigurations.isNotEmpty) { } else if (hasChildConfigurationsDelegate) {
final ChildSemanticsConfigurationsResult result = config.childConfigurationsDelegate!(childConfigurations); final ChildSemanticsConfigurationsResult result = config.childConfigurationsDelegate!(childConfigurations);
mergeUpFragments.addAll( mergeUpFragments.addAll(
result.mergeUp.map<_InterestingSemanticsFragment>((SemanticsConfiguration config) => configToFragment[config]!), result.mergeUp.map<_InterestingSemanticsFragment>((SemanticsConfiguration config) {
final _InterestingSemanticsFragment? fragment = configToFragment[config];
if (fragment == null) {
// Parent fragment of Incomplete fragments can't be a forking
// fragment since they need to be merged.
producesForkingFragment = false;
return _IncompleteSemanticsFragment(config: config, owner: this);
}
return fragment;
}),
); );
for (final Iterable<SemanticsConfiguration> group in result.siblingMergeGroups) { for (final Iterable<SemanticsConfiguration> group in result.siblingMergeGroups) {
siblingMergeFragmentGroups.add( siblingMergeFragmentGroups.add(
group.map<_InterestingSemanticsFragment>((SemanticsConfiguration config) => configToFragment[config]!).toList() group.map<_InterestingSemanticsFragment>((SemanticsConfiguration config) {
return configToFragment[config] ?? _IncompleteSemanticsFragment(config: config, owner: this);
}).toList(),
); );
} }
} }
...@@ -4184,10 +4201,10 @@ abstract class _InterestingSemanticsFragment extends _SemanticsFragment { ...@@ -4184,10 +4201,10 @@ abstract class _InterestingSemanticsFragment extends _SemanticsFragment {
Set<SemanticsTag>? _tagsForChildren; Set<SemanticsTag>? _tagsForChildren;
/// Tag all children produced by [compileChildren] with `tags`. /// Tag all children produced by [compileChildren] with `tags`.
void addTags(Iterable<SemanticsTag>? tags) { ///
if (tags == null || tags.isEmpty) { /// `tags` must not be empty.
return; void addTags(Iterable<SemanticsTag> tags) {
} assert(tags.isNotEmpty);
_tagsForChildren ??= <SemanticsTag>{}; _tagsForChildren ??= <SemanticsTag>{};
_tagsForChildren!.addAll(tags); _tagsForChildren!.addAll(tags);
} }
...@@ -4281,6 +4298,48 @@ class _RootSemanticsFragment extends _InterestingSemanticsFragment { ...@@ -4281,6 +4298,48 @@ class _RootSemanticsFragment extends _InterestingSemanticsFragment {
} }
} }
/// A fragment with partial information that must not form an explicit
/// semantics node without merging into another _SwitchableSemanticsFragment.
///
/// This fragment is generated from synthetic SemanticsConfiguration returned from
/// [SemanticsConfiguration.childConfigurationsDelegate].
class _IncompleteSemanticsFragment extends _InterestingSemanticsFragment {
_IncompleteSemanticsFragment({
required this.config,
required super.owner,
}) : super(dropsSemanticsOfPreviousSiblings: false);
@override
void addAll(Iterable<_InterestingSemanticsFragment> fragments) {
assert(false, 'This fragment must be a leaf node');
}
@override
void compileChildren({
required Rect? parentSemanticsClipRect,
required Rect? parentPaintClipRect,
required double elevationAdjustment,
required List<SemanticsNode> result,
required List<SemanticsNode> siblingNodes,
}) {
// There is nothing to do because this fragment must be a leaf node and
// must not be explicit.
}
@override
final SemanticsConfiguration config;
@override
void markAsExplicit() {
assert(
false,
'SemanticsConfiguration created in '
'SemanticsConfiguration.childConfigurationsDelegate must not produce '
'its own semantics node'
);
}
}
/// An [_InterestingSemanticsFragment] that can be told to only add explicit /// An [_InterestingSemanticsFragment] that can be told to only add explicit
/// [SemanticsNode]s to the parent. /// [SemanticsNode]s to the parent.
/// ///
...@@ -4559,6 +4618,14 @@ class _SwitchableSemanticsFragment extends _InterestingSemanticsFragment { ...@@ -4559,6 +4618,14 @@ class _SwitchableSemanticsFragment extends _InterestingSemanticsFragment {
} }
} }
@override
void addTags(Iterable<SemanticsTag> tags) {
super.addTags(tags);
// _ContainerSemanticsFragments add their tags to child fragments through
// this method. This fragment must make sure its _config is in sync.
tags.forEach(_config.addTagForChildren);
}
void _ensureConfigIsWritable() { void _ensureConfigIsWritable() {
if (!_isConfigWritable) { if (!_isConfigWritable) {
_config = _config.copy(); _config = _config.copy();
......
...@@ -119,7 +119,9 @@ class RenderParagraph extends RenderBox ...@@ -119,7 +119,9 @@ class RenderParagraph extends RenderBox
static final String _placeholderCharacter = String.fromCharCode(PlaceholderSpan.placeholderCodeUnit); static final String _placeholderCharacter = String.fromCharCode(PlaceholderSpan.placeholderCodeUnit);
final TextPainter _textPainter; final TextPainter _textPainter;
AttributedString? _cachedAttributedLabel;
List<AttributedString>? _cachedAttributedLabels;
List<InlineSpanSemanticsInformation>? _cachedCombinedSemanticsInfos; List<InlineSpanSemanticsInformation>? _cachedCombinedSemanticsInfos;
/// The text to display. /// The text to display.
...@@ -135,7 +137,7 @@ class RenderParagraph extends RenderBox ...@@ -135,7 +137,7 @@ class RenderParagraph extends RenderBox
break; break;
case RenderComparison.paint: case RenderComparison.paint:
_textPainter.text = value; _textPainter.text = value;
_cachedAttributedLabel = null; _cachedAttributedLabels = null;
_cachedCombinedSemanticsInfos = null; _cachedCombinedSemanticsInfos = null;
_extractPlaceholderSpans(value); _extractPlaceholderSpans(value);
markNeedsPaint(); markNeedsPaint();
...@@ -144,7 +146,7 @@ class RenderParagraph extends RenderBox ...@@ -144,7 +146,7 @@ class RenderParagraph extends RenderBox
case RenderComparison.layout: case RenderComparison.layout:
_textPainter.text = value; _textPainter.text = value;
_overflowShader = null; _overflowShader = null;
_cachedAttributedLabel = null; _cachedAttributedLabels = null;
_cachedCombinedSemanticsInfos = null; _cachedCombinedSemanticsInfos = null;
_extractPlaceholderSpans(value); _extractPlaceholderSpans(value);
markNeedsLayout(); markNeedsLayout();
...@@ -1035,12 +1037,23 @@ class RenderParagraph extends RenderBox ...@@ -1035,12 +1037,23 @@ class RenderParagraph extends RenderBox
void describeSemanticsConfiguration(SemanticsConfiguration config) { void describeSemanticsConfiguration(SemanticsConfiguration config) {
super.describeSemanticsConfiguration(config); super.describeSemanticsConfiguration(config);
_semanticsInfo = text.getSemanticsInformation(); _semanticsInfo = text.getSemanticsInformation();
bool needsAssembleSemanticsNode = false;
bool needsChildConfigrationsDelegate = false;
for (final InlineSpanSemanticsInformation info in _semanticsInfo!) {
if (info.recognizer != null) {
needsAssembleSemanticsNode = true;
break;
}
needsChildConfigrationsDelegate = needsChildConfigrationsDelegate || info.isPlaceholder;
}
if (_semanticsInfo!.any((InlineSpanSemanticsInformation info) => info.recognizer != null)) { if (needsAssembleSemanticsNode) {
config.explicitChildNodes = true; config.explicitChildNodes = true;
config.isSemanticBoundary = true; config.isSemanticBoundary = true;
} else if (needsChildConfigrationsDelegate) {
config.childConfigurationsDelegate = _childSemanticsConfigurationsDelegate;
} else { } else {
if (_cachedAttributedLabel == null) { if (_cachedAttributedLabels == null) {
final StringBuffer buffer = StringBuffer(); final StringBuffer buffer = StringBuffer();
int offset = 0; int offset = 0;
final List<StringAttribute> attributes = <StringAttribute>[]; final List<StringAttribute> attributes = <StringAttribute>[];
...@@ -1050,21 +1063,77 @@ class RenderParagraph extends RenderBox ...@@ -1050,21 +1063,77 @@ class RenderParagraph extends RenderBox
final TextRange originalRange = infoAttribute.range; final TextRange originalRange = infoAttribute.range;
attributes.add( attributes.add(
infoAttribute.copy( infoAttribute.copy(
range: TextRange(start: offset + originalRange.start, range: TextRange(
end: offset + originalRange.end) start: offset + originalRange.start,
end: offset + originalRange.end,
),
), ),
); );
} }
buffer.write(label); buffer.write(label);
offset += label.length; offset += label.length;
} }
_cachedAttributedLabel = AttributedString(buffer.toString(), attributes: attributes); _cachedAttributedLabels = <AttributedString>[AttributedString(buffer.toString(), attributes: attributes)];
} }
config.attributedLabel = _cachedAttributedLabel!; config.attributedLabel = _cachedAttributedLabels![0];
config.textDirection = textDirection; config.textDirection = textDirection;
} }
} }
ChildSemanticsConfigurationsResult _childSemanticsConfigurationsDelegate(List<SemanticsConfiguration> childConfigs) {
final ChildSemanticsConfigurationsResultBuilder builder = ChildSemanticsConfigurationsResultBuilder();
int placeholderIndex = 0;
int childConfigsIndex = 0;
int attributedLabelCacheIndex = 0;
InlineSpanSemanticsInformation? seenTextInfo;
_cachedCombinedSemanticsInfos ??= combineSemanticsInfo(_semanticsInfo!);
for (final InlineSpanSemanticsInformation info in _cachedCombinedSemanticsInfos!) {
if (info.isPlaceholder) {
if (seenTextInfo != null) {
builder.markAsMergeUp(_createSemanticsConfigForTextInfo(seenTextInfo, attributedLabelCacheIndex));
attributedLabelCacheIndex += 1;
}
// Mark every childConfig belongs to this placeholder to merge up group.
while (childConfigsIndex < childConfigs.length &&
childConfigs[childConfigsIndex].tagsChildrenWith(PlaceholderSpanIndexSemanticsTag(placeholderIndex))) {
builder.markAsMergeUp(childConfigs[childConfigsIndex]);
childConfigsIndex += 1;
}
placeholderIndex += 1;
} else {
seenTextInfo = info;
}
}
// Handle plain text info at the end.
if (seenTextInfo != null) {
builder.markAsMergeUp(_createSemanticsConfigForTextInfo(seenTextInfo, attributedLabelCacheIndex));
}
return builder.build();
}
SemanticsConfiguration _createSemanticsConfigForTextInfo(InlineSpanSemanticsInformation textInfo, int cacheIndex) {
assert(!textInfo.requiresOwnNode);
final List<AttributedString> cachedStrings = _cachedAttributedLabels ??= <AttributedString>[];
assert(cacheIndex <= cachedStrings.length);
final bool hasCache = cacheIndex < cachedStrings.length;
late AttributedString attributedLabel;
if (hasCache) {
attributedLabel = cachedStrings[cacheIndex];
} else {
assert(cachedStrings.length == cacheIndex);
attributedLabel = AttributedString(
textInfo.semanticsLabel ?? textInfo.text,
attributes: textInfo.stringAttributes,
);
cachedStrings.add(attributedLabel);
}
return SemanticsConfiguration()
..textDirection = textDirection
..attributedLabel = attributedLabel;
}
// Caches [SemanticsNode]s created during [assembleSemanticsNode] so they // Caches [SemanticsNode]s created during [assembleSemanticsNode] so they
// can be re-used when [assembleSemanticsNode] is called again. This ensures // can be re-used when [assembleSemanticsNode] is called again. This ensures
// stable ids for the [SemanticsNode]s of [TextSpan]s across // stable ids for the [SemanticsNode]s of [TextSpan]s across
......
...@@ -346,7 +346,7 @@ class AttributedString { ...@@ -346,7 +346,7 @@ class AttributedString {
} }
@override @override
int get hashCode => Object.hash(string, attributes,); int get hashCode => Object.hash(string, attributes);
@override @override
String toString() { String toString() {
...@@ -3805,7 +3805,8 @@ class SemanticsConfiguration { ...@@ -3805,7 +3805,8 @@ class SemanticsConfiguration {
/// which of them should be merged upwards into the parent SemanticsNode. /// which of them should be merged upwards into the parent SemanticsNode.
/// ///
/// The input list of [SemanticsConfiguration]s can be empty if the rendering /// The input list of [SemanticsConfiguration]s can be empty if the rendering
/// object of this semantics configuration is a leaf node. /// object of this semantics configuration is a leaf node or child rendering
/// objects do not contribute to the semantics.
ChildSemanticsConfigurationsDelegate? get childConfigurationsDelegate => _childConfigurationsDelegate; ChildSemanticsConfigurationsDelegate? get childConfigurationsDelegate => _childConfigurationsDelegate;
ChildSemanticsConfigurationsDelegate? _childConfigurationsDelegate; ChildSemanticsConfigurationsDelegate? _childConfigurationsDelegate;
set childConfigurationsDelegate(ChildSemanticsConfigurationsDelegate? value) { set childConfigurationsDelegate(ChildSemanticsConfigurationsDelegate? value) {
......
...@@ -310,6 +310,160 @@ void main() { ...@@ -310,6 +310,160 @@ void main() {
semantics.dispose(); semantics.dispose();
}); });
testWidgets('semantics label is in order when uses widget span', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Text.rich(
TextSpan(
children: <InlineSpan>[
const TextSpan(text: 'before '),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: Semantics(label: 'foo'),
),
const TextSpan(text: ' after'),
],
),
),
),
);
expect(
tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \nfoo\n after'),
);
// If the Paragraph is not dirty it should use the cache correctly.
final RenderObject parent = tester.renderObject<RenderObject>(find.byType(Directionality));
parent.markNeedsSemanticsUpdate();
await tester.pumpAndSettle();
expect(
tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \nfoo\n after'),
);
});
testWidgets('semantics can handle some widget spans without semantics', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Text.rich(
TextSpan(
children: <InlineSpan>[
const TextSpan(text: 'before '),
const WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: SizedBox(width: 10.0),
),
const TextSpan(text: ' mid'),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: Semantics(label: 'foo'),
),
const TextSpan(text: ' after'),
const WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: SizedBox(width: 10.0),
),
],
),
),
),
);
expect(tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n mid\nfoo\n after'));
// If the Paragraph is not dirty it should use the cache correctly.
final RenderObject parent = tester.renderObject<RenderObject>(find.byType(Directionality));
parent.markNeedsSemanticsUpdate();
await tester.pumpAndSettle();
expect(tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n mid\nfoo\n after'));
});
testWidgets('semantics can handle all widget spans without semantics', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Text.rich(
TextSpan(
children: <InlineSpan>[
TextSpan(text: 'before '),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: SizedBox(width: 10.0),
),
TextSpan(text: ' mid'),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: SizedBox(width: 10.0),
),
TextSpan(text: ' after'),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: SizedBox(width: 10.0),
),
],
),
),
),
);
expect(tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n mid\n after'));
// If the Paragraph is not dirty it should use the cache correctly.
final RenderObject parent = tester.renderObject<RenderObject>(find.byType(Directionality));
parent.markNeedsSemanticsUpdate();
await tester.pumpAndSettle();
expect(tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n mid\n after'));
});
testWidgets('semantics can handle widget spans with explicit semantics node', (WidgetTester tester) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Text.rich(
TextSpan(
children: <InlineSpan>[
const TextSpan(text: 'before '),
WidgetSpan(
alignment: PlaceholderAlignment.baseline,
baseline: TextBaseline.alphabetic,
child: Semantics(label: 'inner', container: true),
),
const TextSpan(text: ' after'),
],
),
),
),
);
expect(
tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n after', children: <Matcher>[matchesSemantics(label: 'inner')]),
);
// If the Paragraph is not dirty it should use the cache correctly.
final RenderObject parent = tester.renderObject<RenderObject>(find.byType(Directionality));
parent.markNeedsSemanticsUpdate();
await tester.pumpAndSettle();
expect(
tester.getSemantics(find.byType(Text)),
matchesSemantics(label: 'before \n after', children: <Matcher>[matchesSemantics(label: 'inner')]),
);
});
testWidgets('semanticsLabel can be shorter than text', (WidgetTester tester) async { testWidgets('semanticsLabel can be shorter than text', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester); final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(Directionality( await tester.pumpWidget(Directionality(
......
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