Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add Semantics Property linkUrl #53507

Merged
merged 6 commits into from
Jul 3, 2024
Merged

Add Semantics Property linkUrl #53507

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73dded3
Add Semantics Property `linkUri`
mdebbar Jun 20, 2024
bbbb01f
update href
mdebbar Jun 25, 2024
9b4ef86
Merge branch 'main' into link_uri
mdebbar Jun 25, 2024
6262539
linkUri -> linkUrl plus responding to other review comments
harryterkelsen Jul 3, 2024
d2be434
Merge branch 'main' into link_uri
harryterkelsen Jul 3, 2024
c2f954c
Merge branch 'main' into link_uri
harryterkelsen Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/ui/fixtures/ui_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ void sendSemanticsUpdate() {
childrenInHitTestOrder: childrenInHitTestOrder,
additionalActions: additionalActions,
headingLevel: 0,
linkUri: '',
Copy link
Contributor

@yjbanov yjbanov Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "URL" is more accurate here than "URI". The <a> tag and the Link widget only work with URIs that are also fetchable over the network via the same URI. Using arbitrary URIs as abstract identifiers without being able to fetch them is not something that's supported by this property, AFAICT. Unless there's something on the mobile side that extends beyond that?

Copy link
Contributor

@harryterkelsen harryterkelsen Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

);
_semanticsUpdate(builder.build());
}
Expand Down
12 changes: 10 additions & 2 deletions lib/ui/semantics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,9 @@ abstract class SemanticsUpdateBuilder {
///
/// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role
/// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-level
///
/// The `linkUri` describes the URI that this node links to. If the node is
/// not a link, this should be an empty string.
void updateNode({
required int id,
required int flags,
Expand Down Expand Up @@ -888,6 +891,7 @@ abstract class SemanticsUpdateBuilder {
required Int32List childrenInHitTestOrder,
required Int32List additionalActions,
int headingLevel = 0,
String linkUri = '',
});

/// Update the custom semantics action associated with the given `id`.
Expand Down Expand Up @@ -959,6 +963,7 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem
required Int32List childrenInHitTestOrder,
required Int32List additionalActions,
int headingLevel = 0,
String linkUri = '',
}) {
assert(_matrix4IsValid(transform));
assert (
Expand Down Expand Up @@ -1003,6 +1008,7 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem
childrenInHitTestOrder,
additionalActions,
headingLevel,
linkUri,
);
}
@Native<
Expand Down Expand Up @@ -1044,7 +1050,8 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem
Handle,
Handle,
Handle,
Int32)>(symbol: 'SemanticsUpdateBuilder::updateNode')
Int32,
Handle)>(symbol: 'SemanticsUpdateBuilder::updateNode')
external void _updateNode(
int id,
int flags,
Expand Down Expand Up @@ -1082,7 +1089,8 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem
Int32List childrenInTraversalOrder,
Int32List childrenInHitTestOrder,
Int32List additionalActions,
int headingLevel);
int headingLevel,
String linkUri);

@override
void updateCustomAction({required int id, String? label, String? hint, int overrideId = -1}) {
Expand Down
2 changes: 2 additions & 0 deletions lib/ui/semantics/semantics_node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ struct SemanticsNode {
std::vector<int32_t> childrenInHitTestOrder;
std::vector<int32_t> customAccessibilityActions;
int32_t headingLevel = 0;

std::string linkUri;
};

// Contains semantic nodes that need to be updated.
Expand Down
4 changes: 3 additions & 1 deletion lib/ui/semantics/semantics_update_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ void SemanticsUpdateBuilder::updateNode(
const tonic::Int32List& childrenInTraversalOrder,
const tonic::Int32List& childrenInHitTestOrder,
const tonic::Int32List& localContextActions,
int headingLevel) {
int headingLevel,
std::string linkUri) {
FML_CHECK(scrollChildren == 0 ||
(scrollChildren > 0 && childrenInHitTestOrder.data()))
<< "Semantics update contained scrollChildren but did not have "
Expand Down Expand Up @@ -121,6 +122,7 @@ void SemanticsUpdateBuilder::updateNode(
nodes_[id] = node;

node.headingLevel = headingLevel;
node.linkUri = std::move(linkUri);
}

void SemanticsUpdateBuilder::updateCustomAction(int id,
Expand Down
3 changes: 2 additions & 1 deletion lib/ui/semantics/semantics_update_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ class SemanticsUpdateBuilder
const tonic::Int32List& childrenInTraversalOrder,
const tonic::Int32List& childrenInHitTestOrder,
const tonic::Int32List& customAccessibilityActions,
int headingLevel);
int headingLevel,
std::string linkUri);

void updateCustomAction(int id,
std::string label,
Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/lib/semantics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ class SemanticsUpdateBuilder {
required Int32List childrenInHitTestOrder,
required Int32List additionalActions,
int headingLevel = 0,
String? linkUri,
}) {
if (transform.length != 16) {
throw ArgumentError('transform argument must have 16 entries.');
Expand Down Expand Up @@ -326,6 +327,7 @@ class SemanticsUpdateBuilder {
additionalActions: additionalActions,
platformViewId: platformViewId,
headingLevel: headingLevel,
linkUri: linkUri,
));
}

Expand Down
5 changes: 3 additions & 2 deletions lib/web_ui/lib/src/engine/semantics/link.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ class Link extends PrimaryRoleManager {
@override
DomElement createElement() {
final DomElement element = domDocument.createElement('a');
// TODO(mdebbar): Fill in the real link once the framework sends entire uri.
// https://github.com/flutter/flutter/issues/150263.
element.style.display = 'block';
if (semanticsObject.hasLinkUri) {
Copy link
Contributor

@yjbanov yjbanov Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the link change? If yes, this code should probably go into an override of the update method and it should also check for isLinkUriDirty.

Copy link
Contributor Author

@mdebbar mdebbar Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link
Contributor

@harryterkelsen harryterkelsen Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

element.setAttribute('href', semanticsObject.linkUri!);
}
return element;
}

Expand Down
25 changes: 25 additions & 0 deletions lib/web_ui/lib/src/engine/semantics/semantics.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ class SemanticsNodeUpdate {
required this.childrenInHitTestOrder,
required this.additionalActions,
required this.headingLevel,
this.linkUri,
});

/// See [ui.SemanticsUpdateBuilder.updateNode].
Expand Down Expand Up @@ -337,6 +338,9 @@ class SemanticsNodeUpdate {

/// See [ui.SemanticsUpdateBuilder.updateNode].
final int headingLevel;

/// See [ui.SemanticsUpdateBuilder.updateNode].
final String? linkUri;
Copy link
Contributor

@yjbanov yjbanov Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linkUri is non-nullable in the updateNode API. Does it need to be nullable here?

Copy link
Contributor

@harryterkelsen harryterkelsen Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mirrors the code for the nullable tooltip field. I think it is non-nullable in the updateNode API to make it easier for the C++ side to handle.

}

/// Identifies [PrimaryRoleManager] implementations.
Expand Down Expand Up @@ -1146,6 +1150,22 @@ class SemanticsObject {
_dirtyFields |= _identifierIndex;
}

/// See [ui.SemanticsUpdateBuilder.updateNode].
String? get linkUri => _linkUri;
String? _linkUri;

/// Whether this object contains a non-empty link URI.
bool get hasLinkUri => _linkUri != null && _linkUri!.isNotEmpty;

static const int _linkUriIndex = 1 << 26;

/// Whether the [linkUri] field has been updated but has not been
/// applied to the DOM yet.
bool get isLinkUriDirty => _isDirty(_linkUriIndex);
void _markLinkUriDirty() {
_dirtyFields |= _linkUriIndex;
}

/// A unique permanent identifier of the semantics node in the tree.
final int id;

Expand Down Expand Up @@ -1445,6 +1465,11 @@ class SemanticsObject {
_markPlatformViewIdDirty();
}

if (_linkUri != update.linkUri) {
_linkUri = update.linkUri;
_markLinkUriDirty();
}

// Apply updates to the DOM.
_updateRoles();

Expand Down
24 changes: 24 additions & 0 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3603,6 +3603,28 @@ void _testLink() {
expect(object.element.tagName.toLowerCase(), 'a');
expect(object.element.hasAttribute('href'), isFalse);
});

test('link nodes with linkUri set the href attribute', () {
semantics()
..debugOverrideTimestampFunction(() => _testTime)
..semanticsEnabled = true;

SemanticsObject pumpSemantics() {
final SemanticsTester tester = SemanticsTester(owner());
tester.updateNode(
id: 0,
isLink: true,
linkUri: 'https://flutter.dev',
rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
);
tester.apply();
return tester.getSemanticsObject(0);
}

final SemanticsObject object = pumpSemantics();
expect(object.element.tagName.toLowerCase(), 'a');
expect(object.element.getAttribute('href'), 'https://flutter.dev');
});
}

/// A facade in front of [ui.SemanticsUpdateBuilder.updateNode] that
Expand Down Expand Up @@ -3645,6 +3667,7 @@ void updateNode(
Int32List? childrenInHitTestOrder,
Int32List? additionalActions,
int headingLevel = 0,
String? linkUri,
}) {
transform ??= Float64List.fromList(Matrix4.identity().storage);
childrenInTraversalOrder ??= Int32List(0);
Expand Down Expand Up @@ -3685,6 +3708,7 @@ void updateNode(
childrenInHitTestOrder: childrenInHitTestOrder,
additionalActions: additionalActions,
headingLevel: headingLevel,
linkUri: linkUri,
);
}

Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/test/engine/semantics/semantics_tester.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class SemanticsTester {
Int32List? additionalActions,
List<SemanticsNodeUpdate>? children,
int? headingLevel,
String? linkUri,
}) {
// Flags
if (hasCheckedState ?? false) {
Expand Down Expand Up @@ -313,6 +314,7 @@ class SemanticsTester {
childrenInHitTestOrder: childIds,
additionalActions: additionalActions ?? Int32List(0),
headingLevel: headingLevel ?? 0,
linkUri: linkUri,
);
_nodeUpdates.add(update);
return update;
Expand Down
13 changes: 9 additions & 4 deletions shell/platform/embedder/fixtures/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ Future<void> a11y_main() async {
tooltip: 'tooltip',
textDirection: TextDirection.ltr,
additionalActions: Int32List(0),
headingLevel: 0
headingLevel: 0,
linkUri: '',
)
..updateNode(
id: 84,
Expand Down Expand Up @@ -214,7 +215,8 @@ Future<void> a11y_main() async {
additionalActions: Int32List(0),
childrenInHitTestOrder: Int32List(0),
childrenInTraversalOrder: Int32List(0),
headingLevel: 0
headingLevel: 0,
linkUri: '',
)
..updateNode(
id: 96,
Expand Down Expand Up @@ -250,7 +252,8 @@ Future<void> a11y_main() async {
tooltip: 'tooltip',
textDirection: TextDirection.ltr,
additionalActions: Int32List(0),
headingLevel: 0
headingLevel: 0,
linkUri: '',
)
..updateNode(
id: 128,
Expand Down Expand Up @@ -286,7 +289,8 @@ Future<void> a11y_main() async {
textDirection: TextDirection.ltr,
childrenInHitTestOrder: Int32List(0),
childrenInTraversalOrder: Int32List(0),
headingLevel: 0
headingLevel: 0,
linkUri: '',
)
..updateCustomAction(
id: 21,
Expand Down Expand Up @@ -384,6 +388,7 @@ Future<void> a11y_string_attributes() async {
textDirection: TextDirection.ltr,
additionalActions: Int32List(0),
headingLevel: 0,
linkUri: '',
);

PlatformDispatcher.instance.views.first.updateSemantics(builder.build());
Expand Down