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

Swap nextNodeId for previousNodeId, since setTraversalBefore is broken. (#14607)

It seems that setTraversalBefore doesn't work as well as setTraversalAfter for some reason, although I'm using them the same way. Some apps would lock up TalkBack when traversing if setTraversalBefore was set, but not with the equivalent setTraversalAfter.

It's not entirely clear why this is, but I'm going with this to at least get it fixed for apps we know about.

Fixes #14600
parent 09aa673b
ab3b3a63becc2371ac44e0ef31341ec9aea4d4f6
f5a4a9378740c3d5996583a9ed1f7e28ff08ee85
......@@ -92,7 +92,7 @@ class SemanticsData extends Diagnosticable {
@required this.decreasedValue,
@required this.hint,
@required this.textDirection,
@required this.nextNodeId,
@required this.previousNodeId,
@required this.rect,
@required this.textSelection,
@required this.scrollPosition,
......@@ -151,9 +151,9 @@ class SemanticsData extends Diagnosticable {
/// [increasedValue], and [decreasedValue].
final TextDirection textDirection;
/// The index indicating the ID of the next node in the traversal order after
/// The index indicating the ID of the previous node in the traversal order before
/// this node for the platform's accessibility services.
final int nextNodeId;
final int previousNodeId;
/// The currently selected text (or the position of the cursor) within [value]
/// if this node represents a text field.
......@@ -237,7 +237,7 @@ class SemanticsData extends Diagnosticable {
properties.add(new StringProperty('decreasedValue', decreasedValue, defaultValue: ''));
properties.add(new StringProperty('hint', hint, defaultValue: ''));
properties.add(new EnumProperty<TextDirection>('textDirection', textDirection, defaultValue: null));
properties.add(new IntProperty('nextNodeId', nextNodeId, defaultValue: null));
properties.add(new IntProperty('previousNodeId', previousNodeId, defaultValue: null));
if (textSelection?.isValid == true)
properties.add(new MessageProperty('textSelection', '[${textSelection.start}, ${textSelection.end}]'));
properties.add(new DoubleProperty('scrollExtentMin', scrollExtentMin, defaultValue: null));
......@@ -258,7 +258,7 @@ class SemanticsData extends Diagnosticable {
&& typedOther.decreasedValue == decreasedValue
&& typedOther.hint == hint
&& typedOther.textDirection == textDirection
&& typedOther.nextNodeId == nextNodeId
&& typedOther.previousNodeId == previousNodeId
&& typedOther.rect == rect
&& setEquals(typedOther.tags, tags)
&& typedOther.textSelection == textSelection
......@@ -269,7 +269,7 @@ class SemanticsData extends Diagnosticable {
}
@override
int get hashCode => ui.hashValues(flags, actions, label, value, increasedValue, decreasedValue, hint, textDirection, nextNodeId, rect, tags, textSelection, scrollPosition, scrollExtentMax, scrollExtentMin, transform);
int get hashCode => ui.hashValues(flags, actions, label, value, increasedValue, decreasedValue, hint, textDirection, previousNodeId, rect, tags, textSelection, scrollPosition, scrollExtentMax, scrollExtentMin, transform);
}
class _SemanticsDiagnosticableNode extends DiagnosticableNode<SemanticsNode> {
......@@ -1067,28 +1067,28 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
/// The sort order for ordering the traversal of [SemanticsNode]s by the
/// platform's accessibility services (e.g. VoiceOver on iOS and TalkBack on
/// Android). This is used to determine the [nextNodeId] during a semantics update.
/// Android). This is used to determine the [previousNodeId] during a semantics update.
SemanticsSortOrder _sortOrder;
SemanticsSortOrder get sortOrder => _sortOrder;
/// The ID of the next node in the traversal order after this node.
/// The ID of the previous node in the traversal order before this node.
///
/// Only valid after at least one semantics update has been built.
///
/// This is the value passed to the engine to tell it what the order
/// should be for traversing semantics nodes.
///
/// If this is set to -1, it will indicate that there is no next node to
/// the engine (i.e. this is the last node in the sort order). When it is
/// If this is set to -1, it will indicate that there is no previous node to
/// the engine (i.e. this is the first node in the sort order). When it is
/// null, it means that no semantics update has been built yet.
int _nextNodeId;
void _updateNextNodeId(int value) {
if (value == _nextNodeId)
int _previousNodeId;
void _updatePreviousNodeId(int value) {
if (value == _previousNodeId)
return;
_nextNodeId = value;
_previousNodeId = value;
_markDirty();
}
int get nextNodeId => _nextNodeId;
int get previousNodeId => _previousNodeId;
/// The currently selected text (or the position of the cursor) within [value]
/// if this node represents a text field.
......@@ -1194,7 +1194,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
String increasedValue = _increasedValue;
String decreasedValue = _decreasedValue;
TextDirection textDirection = _textDirection;
int nextNodeId = _nextNodeId;
int previousNodeId = _previousNodeId;
Set<SemanticsTag> mergedTags = tags == null ? null : new Set<SemanticsTag>.from(tags);
TextSelection textSelection = _textSelection;
double scrollPosition = _scrollPosition;
......@@ -1207,7 +1207,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
flags |= node._flags;
actions |= node._actionsAsBits;
textDirection ??= node._textDirection;
nextNodeId ??= node._nextNodeId;
previousNodeId ??= node._previousNodeId;
textSelection ??= node._textSelection;
scrollPosition ??= node._scrollPosition;
scrollExtentMax ??= node._scrollExtentMax;
......@@ -1247,7 +1247,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
decreasedValue: decreasedValue,
hint: hint,
textDirection: textDirection,
nextNodeId: nextNodeId,
previousNodeId: previousNodeId,
rect: rect,
transform: transform,
tags: mergedTags,
......@@ -1289,7 +1289,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
increasedValue: data.increasedValue,
hint: data.hint,
textDirection: data.textDirection,
nextNodeId: data.nextNodeId,
previousNodeId: data.previousNodeId,
textSelectionBase: data.textSelection != null ? data.textSelection.baseOffset : -1,
textSelectionExtent: data.textSelection != null ? data.textSelection.extentOffset : -1,
scrollPosition: data.scrollPosition != null ? data.scrollPosition : double.nan,
......@@ -1363,7 +1363,7 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
properties.add(new StringProperty('decreasedValue', _decreasedValue, defaultValue: ''));
properties.add(new StringProperty('hint', _hint, defaultValue: ''));
properties.add(new EnumProperty<TextDirection>('textDirection', _textDirection, defaultValue: null));
properties.add(new IntProperty('nextNodeId', _nextNodeId, defaultValue: null));
properties.add(new IntProperty('previousNodeId', _previousNodeId, defaultValue: null));
properties.add(new DiagnosticsProperty<SemanticsSortOrder>('sortOrder', sortOrder, defaultValue: null));
if (_textSelection?.isValid == true)
properties.add(new MessageProperty('text selection', '[${_textSelection.start}, ${_textSelection.end}]'));
......@@ -1483,9 +1483,9 @@ class SemanticsOwner extends ChangeNotifier {
super.dispose();
}
// Updates the nextNodeId IDs on the semantics nodes. These IDs are used
// Updates the previousNodeId IDs on the semantics nodes. These IDs are used
// on the platform side to order the nodes for traversal by the accessibility
// services. If the nextNodeId for a node changes, the node will be marked as
// services. If the previousNodeId for a node changes, the node will be marked as
// dirty.
void _updateTraversalOrder() {
final List<_TraversalSortNode> nodesInSemanticsTraversalOrder = <_TraversalSortNode>[];
......@@ -1505,10 +1505,10 @@ class SemanticsOwner extends ChangeNotifier {
}
rootSemanticsNode.visitChildren(visitor);
nodesInSemanticsTraversalOrder.sort();
int nextNodeId = -1;
for (_TraversalSortNode node in nodesInSemanticsTraversalOrder.reversed) {
node.node._updateNextNodeId(nextNodeId);
nextNodeId = node.node.id;
int previousNodeId = -1;
for (_TraversalSortNode node in nodesInSemanticsTraversalOrder) {
node.node._updatePreviousNodeId(previousNodeId);
previousNodeId = node.node.id;
}
}
......@@ -1516,7 +1516,7 @@ class SemanticsOwner extends ChangeNotifier {
void sendSemanticsUpdate() {
if (_dirtyNodes.isEmpty)
return;
// Nodes that change their nextNodeId will be marked as dirty.
// Nodes that change their previousNodeId will be marked as dirty.
_updateTraversalOrder();
final List<SemanticsNode> visitedNodes = <SemanticsNode>[];
while (_dirtyNodes.isNotEmpty) {
......
......@@ -348,7 +348,7 @@ void main() {
expect(
minimalProperties.toStringDeep(minLevel: DiagnosticLevel.hidden),
'SemanticsNode#1(owner: null, isMergedIntoParent: false, mergeAllDescendantsIntoThisNode: false, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), actions: [], isInMutuallyExcusiveGroup: false, isSelected: false, isFocused: false, isButton: false, isTextField: false, invisible, label: "", value: "", increasedValue: "", decreasedValue: "", hint: "", textDirection: null, nextNodeId: null, sortOrder: null, scrollExtentMin: null, scrollPosition: null, scrollExtentMax: null)\n'
'SemanticsNode#1(owner: null, isMergedIntoParent: false, mergeAllDescendantsIntoThisNode: false, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0), actions: [], isInMutuallyExcusiveGroup: false, isSelected: false, isFocused: false, isButton: false, isTextField: false, invisible, label: "", value: "", increasedValue: "", decreasedValue: "", hint: "", textDirection: null, previousNodeId: null, sortOrder: null, scrollExtentMin: null, scrollPosition: null, scrollExtentMax: null)\n'
);
final SemanticsConfiguration config = new SemanticsConfiguration()
......
......@@ -413,7 +413,7 @@ void main() {
id: expectedId,
rect: TestSemantics.fullScreen,
actions: allActions.fold(0, (int previous, SemanticsAction action) => previous | action.index),
nextNodeId: -1,
previousNodeId: -1,
),
],
);
......@@ -612,35 +612,35 @@ void main() {
new TestSemantics(
children: <TestSemantics>[
new TestSemantics(
nextNodeId: 5,
previousNodeId: -1,
children: <TestSemantics>[
new TestSemantics(
label: r'Label 1',
textDirection: TextDirection.ltr,
nextNodeId: -1,
previousNodeId: 4,
),
new TestSemantics(
label: r'Label 2',
textDirection: TextDirection.ltr,
nextNodeId: 3,
previousNodeId: 6,
),
new TestSemantics(
nextNodeId: 8,
previousNodeId: 2,
children: <TestSemantics>[
new TestSemantics(
label: r'Label 3',
textDirection: TextDirection.ltr,
nextNodeId: 4,
previousNodeId: 7,
),
new TestSemantics(
label: r'Label 4',
textDirection: TextDirection.ltr,
nextNodeId: 6,
previousNodeId: 8,
),
new TestSemantics(
label: r'Label 5',
textDirection: TextDirection.ltr,
nextNodeId: 7,
previousNodeId: 5,
),
],
),
......@@ -710,31 +710,30 @@ void main() {
new TestSemantics(
label: r'Label 1',
textDirection: TextDirection.ltr,
nextNodeId: -1,
previousNodeId: 3,
),
new TestSemantics(
label: r'Label 2',
textDirection: TextDirection.ltr,
nextNodeId: 2,
previousNodeId: 4,
),
new TestSemantics(
label: r'Label 3',
textDirection: TextDirection.ltr,
nextNodeId: 3,
previousNodeId: 5,
),
new TestSemantics(
label: r'Label 4',
textDirection: TextDirection.ltr,
nextNodeId: 4,
previousNodeId: 6,
),
new TestSemantics(
label: r'Label 5',
textDirection: TextDirection.ltr,
nextNodeId: 5,
previousNodeId: -1,
),
],
)
, ignoreTransform: true, ignoreRect: true, ignoreId: true));
), ignoreTransform: true, ignoreRect: true, ignoreId: true));
semantics.dispose();
});
......@@ -800,32 +799,32 @@ void main() {
new TestSemantics(
children: <TestSemantics>[
new TestSemantics(
nextNodeId: 5,
previousNodeId: -1,
children: <TestSemantics>[
new TestSemantics(
label: r'Label 1',
textDirection: TextDirection.ltr,
nextNodeId: 7,
previousNodeId: 5,
),
new TestSemantics(
label: r'Label 2',
textDirection: TextDirection.ltr,
nextNodeId: -1,
previousNodeId: 6,
),
new TestSemantics(
label: r'Label 3',
textDirection: TextDirection.ltr,
nextNodeId: 3,
previousNodeId: 2,
),
new TestSemantics(
label: r'Label 4',
textDirection: TextDirection.ltr,
nextNodeId: 4,
previousNodeId: 7,
),
new TestSemantics(
label: r'Label 5',
textDirection: TextDirection.ltr,
nextNodeId: 6,
previousNodeId: 3,
),
],
),
......
......@@ -43,7 +43,7 @@ class TestSemantics {
this.decreasedValue: '',
this.hint: '',
this.textDirection,
this.nextNodeId,
this.previousNodeId,
this.rect,
this.transform,
this.textSelection,
......@@ -70,7 +70,7 @@ class TestSemantics {
this.decreasedValue: '',
this.hint: '',
this.textDirection,
this.nextNodeId,
this.previousNodeId,
this.transform,
this.textSelection,
this.children: const <TestSemantics>[],
......@@ -106,7 +106,7 @@ class TestSemantics {
this.increasedValue: '',
this.decreasedValue: '',
this.textDirection,
this.nextNodeId,
this.previousNodeId,
this.rect,
Matrix4 transform,
this.textSelection,
......@@ -173,9 +173,9 @@ class TestSemantics {
/// is also set.
final TextDirection textDirection;
/// The ID of the node that is next in the semantics traversal order after
/// The ID of the node that is previous in the semantics traversal order before
/// this node.
final int nextNodeId;
final int previousNodeId;
/// The bounding box for this node in its coordinate system.
///
......@@ -258,8 +258,8 @@ class TestSemantics {
return fail('expected node id $id to have hint "$hint" but found hint "${nodeData.hint}".');
if (textDirection != null && textDirection != nodeData.textDirection)
return fail('expected node id $id to have textDirection "$textDirection" but found "${nodeData.textDirection}".');
if (nextNodeId != null && nextNodeId != nodeData.nextNodeId)
return fail('expected node id $id to have nextNodeId "$nextNodeId" but found "${nodeData.nextNodeId}".');
if (previousNodeId != null && previousNodeId != nodeData.previousNodeId)
return fail('expected node id $id to have previousNodeId "$previousNodeId" but found "${nodeData.previousNodeId}".');
if ((nodeData.label != '' || nodeData.value != '' || nodeData.hint != '' || node.increasedValue != '' || node.decreasedValue != '') && nodeData.textDirection == null)
return fail('expected node id $id, which has a label, value, or hint, to have a textDirection, but it did not.');
if (!ignoreRect && rect != nodeData.rect)
......@@ -311,8 +311,8 @@ class TestSemantics {
buf.writeln('$indent hint: \'$hint\',');
if (textDirection != null)
buf.writeln('$indent textDirection: $textDirection,');
if (nextNodeId != null)
buf.writeln('$indent nextNodeId: $nextNodeId,');
if (previousNodeId != null)
buf.writeln('$indent previousNodeId: $previousNodeId,');
if (textSelection?.isValid == true)
buf.writeln('$indent textSelection:\n[${textSelection.start}, ${textSelection.end}],');
if (rect != null)
......@@ -522,8 +522,8 @@ class SemanticsTester {
buf.writeln(' hint: r\'${node.hint}\',');
if (node.textDirection != null)
buf.writeln(' textDirection: ${node.textDirection},');
if (node.nextNodeId != null)
buf.writeln(' nextNodeId: ${node.nextNodeId},');
if (node.previousNodeId != null)
buf.writeln(' previousNodeId: ${node.previousNodeId},');
if (node.hasChildren) {
buf.writeln(' children: <TestSemantics>[');
......
......@@ -105,15 +105,15 @@ void _tests() {
new TestSemantics(
children: <TestSemantics>[
new TestSemantics(
nextNodeId: 4,
previousNodeId: -1,
children: <TestSemantics>[
new TestSemantics(
nextNodeId: 2,
previousNodeId: 1,
children: <TestSemantics>[
new TestSemantics(
label: r'Plain text',
textDirection: TextDirection.ltr,
nextNodeId: 3,
previousNodeId: 4,
),
new TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.hasCheckedState, SemanticsFlag.isChecked, SemanticsFlag.isSelected],
......@@ -124,7 +124,7 @@ void _tests() {
decreasedValue: r'test-decreasedValue',
hint: r'test-hint',
textDirection: TextDirection.rtl,
nextNodeId: -1,
previousNodeId: 2,
),
],
),
......
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