mirror of
https://github.com/servo/servo
synced 2026-04-25 17:15:48 +02:00
a11y: Miscellaneous cleanups to our accessibility code (#43772)
this patch cleans up a few minor issues in our accessibility code: - we remove the initial synchronous TreeUpdate in [WebView::set_accessibility_active()](https://doc.servo.org/servo/struct.WebView.html#method.set_accessibility_active). it’s not actually necessary, because AccessKit only requires that subtree updates happen after the graft node is created in the parent, but those subtree updates can be delayed indefinitely. removing it simplifies our API a bit. - we rename notify_accessibility_tree_id() to notify_document_accessibility_tree_id(), and do the same to the underlying ConstellationToEmbedderMsg variant. this helps clarify that we’re referring to the (active top-level) document’s accessibility tree. - we make that method pub(crate) too. it doesn’t need to be pub. - we remove the label property from webview-to-pipeline graft nodes. properties set on graft nodes are only visible in the accesskit_consumer API, not in the platform accessibility API, and we don’t need it in our tests, so there’s no longer any reason to keep setting it. Testing: updated the relevant libservo accessibility tests Fixes: part of #4344 Signed-off-by: delan azabani <dazabani@igalia.com> Co-authored-by: Alice Boxhall <alice@igalia.com>
This commit is contained in:
@@ -3151,7 +3151,7 @@ where
|
||||
|
||||
webview.accessibility_active = active;
|
||||
self.constellation_to_embedder_proxy.send(
|
||||
ConstellationToEmbedderMsg::AccessibilityTreeIdChanged(
|
||||
ConstellationToEmbedderMsg::DocumentAccessibilityTreeIdChanged(
|
||||
webview_id,
|
||||
webview.active_top_level_pipeline_id.into(),
|
||||
),
|
||||
@@ -5799,7 +5799,7 @@ where
|
||||
if frame_tree.pipeline.id != webview.active_top_level_pipeline_id {
|
||||
webview.active_top_level_pipeline_id = frame_tree.pipeline.id;
|
||||
self.constellation_to_embedder_proxy.send(
|
||||
ConstellationToEmbedderMsg::AccessibilityTreeIdChanged(
|
||||
ConstellationToEmbedderMsg::DocumentAccessibilityTreeIdChanged(
|
||||
webview_id,
|
||||
webview.active_top_level_pipeline_id.into(),
|
||||
),
|
||||
|
||||
@@ -67,10 +67,12 @@ impl ConstellationWebView {
|
||||
focused_browsing_context_id: BrowsingContextId,
|
||||
user_content_manager_id: Option<UserContentManagerId>,
|
||||
) -> Self {
|
||||
embedder_proxy.send(ConstellationToEmbedderMsg::AccessibilityTreeIdChanged(
|
||||
webview_id,
|
||||
active_top_level_pipeline_id.into(),
|
||||
));
|
||||
embedder_proxy.send(
|
||||
ConstellationToEmbedderMsg::DocumentAccessibilityTreeIdChanged(
|
||||
webview_id,
|
||||
active_top_level_pipeline_id.into(),
|
||||
),
|
||||
);
|
||||
|
||||
Self {
|
||||
webview_id,
|
||||
|
||||
@@ -50,5 +50,5 @@ pub enum ConstellationToEmbedderMsg {
|
||||
HistoryChanged(WebViewId, Vec<ServoUrl>, usize),
|
||||
/// Notifies the embedder that the AccessKit [`TreeId`] for the top-level document in this
|
||||
/// WebView has been changed (or initially set).
|
||||
AccessibilityTreeIdChanged(WebViewId, TreeId),
|
||||
DocumentAccessibilityTreeIdChanged(WebViewId, TreeId),
|
||||
}
|
||||
|
||||
@@ -771,9 +771,9 @@ impl ServoInner {
|
||||
.notify_media_session_event(webview, media_session_event);
|
||||
}
|
||||
},
|
||||
ConstellationToEmbedderMsg::AccessibilityTreeIdChanged(webview_id, tree_id) => {
|
||||
ConstellationToEmbedderMsg::DocumentAccessibilityTreeIdChanged(webview_id, tree_id) => {
|
||||
if let Some(webview) = self.get_webview_handle(webview_id) {
|
||||
webview.notify_accessibility_tree_id(tree_id);
|
||||
webview.notify_document_accessibility_tree_id(tree_id);
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
@@ -48,7 +48,7 @@ fn test_basic_accessibility_update() {
|
||||
let load_webview = webview.clone();
|
||||
servo_test.spin(move || load_webview.load_status() != LoadStatus::Complete);
|
||||
|
||||
let updates = wait_for_min_updates(&servo_test, delegate.clone(), 2);
|
||||
let updates = wait_for_min_updates(&servo_test, delegate.clone(), 1);
|
||||
let tree = build_tree(updates);
|
||||
|
||||
let root_node = tree.state().root();
|
||||
|
||||
@@ -787,10 +787,9 @@ impl WebView {
|
||||
/// sending any tree updates from the webview to your AccessKit adapter. Otherwise you may
|
||||
/// violate AccessKit’s subtree invariants and **panic**.
|
||||
///
|
||||
/// This method may call [`WebViewDelegate::notify_accessibility_tree_update()`] synchronously
|
||||
/// with an initial tree update, so if your impl for that method can’t create the graft node
|
||||
/// (and send *that* update to AccessKit) before sending this update to AccessKit, then it must
|
||||
/// queue the update for later.
|
||||
/// If your impl for [`WebViewDelegate::notify_accessibility_tree_update()`] can’t create the
|
||||
/// graft node (and send *that* update to AccessKit) before sending any updates from this
|
||||
/// webview to AccessKit, then it must queue those updates until it can guarantee that.
|
||||
///
|
||||
/// [graft]: https://docs.rs/accesskit/0.24.0/accesskit/struct.Node.html#method.tree_id
|
||||
/// [`set_tree_id()`]: https://docs.rs/accesskit/0.24.0/accesskit/struct.Node.html#method.set_tree_id
|
||||
@@ -806,23 +805,6 @@ impl WebView {
|
||||
if active {
|
||||
let accesskit_tree_id = TreeId(AccesskitUuid::new_v4());
|
||||
self.inner_mut().accesskit_tree_id = Some(accesskit_tree_id);
|
||||
|
||||
// Synchronously emit a TreeUpdate containing just a ScrollView, but no graft node yet.
|
||||
let root_node_id = NodeId(0);
|
||||
let root_node = AccesskitNode::new(Role::ScrollView);
|
||||
self.delegate().notify_accessibility_tree_update(
|
||||
self.clone(),
|
||||
TreeUpdate {
|
||||
nodes: vec![(root_node_id, root_node)],
|
||||
tree: Some(Tree {
|
||||
root: root_node_id,
|
||||
toolkit_name: None,
|
||||
toolkit_version: None,
|
||||
}),
|
||||
tree_id: accesskit_tree_id,
|
||||
focus: root_node_id,
|
||||
},
|
||||
);
|
||||
} else {
|
||||
self.inner_mut().accesskit_tree_id = None;
|
||||
}
|
||||
@@ -834,7 +816,7 @@ impl WebView {
|
||||
self.accesskit_tree_id()
|
||||
}
|
||||
|
||||
pub fn notify_accessibility_tree_id(&self, grafted_tree_id: TreeId) {
|
||||
pub(crate) fn notify_document_accessibility_tree_id(&self, grafted_tree_id: TreeId) {
|
||||
let Some(webview_accesskit_tree_id) = self.inner().accesskit_tree_id else {
|
||||
return;
|
||||
};
|
||||
@@ -850,7 +832,6 @@ impl WebView {
|
||||
let mut root_node = AccesskitNode::new(Role::ScrollView);
|
||||
let graft_node_id = NodeId(1);
|
||||
let mut graft_node = AccesskitNode::new(Role::GenericContainer);
|
||||
graft_node.set_label("graft");
|
||||
graft_node.set_tree_id(grafted_tree_id);
|
||||
root_node.set_children(vec![graft_node_id]);
|
||||
self.delegate().notify_accessibility_tree_update(
|
||||
|
||||
Reference in New Issue
Block a user