a11y: Basic accessibility tree implementation for web contents#42338
a11y: Basic accessibility tree implementation for web contents#42338alice wants to merge 7 commits intoservo:mainfrom
Conversation
alice
force-pushed
the
accessibility-tree
branch
from
d1ce519 to
2d92f7a
Compare
|
I would happily test accessibility if there is a binary or something I can easily spin up and test screen reader functionality. |
|
@alex-chap Thank you! This branch is still very much a work in progress for the time being, but as we start landing more of it we can ping back here for you to take a look. #42333 is the first step: adding a pref which can be set from the command line to enable the accessibility code to run at all. (Right now, it has no effect.) As we go, we'll land code behind that pref, so it'll be available in any nightly binary built after it lands. |
@alice please do! This is an amazing project and I'll be also happy to help with testing as much as I can. You servo devs and contributors are doing a fantastic job! |
alice
force-pushed
the
accessibility-tree
branch
2 times, most recently
from
d5b7784 to
5570a73
Compare
delan
force-pushed
the
accessibility-tree
branch
from
5dd453d to
00311a7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
delan
force-pushed
the
accessibility-tree
branch
7 times, most recently
from
9599978 to
9ccafbc
Compare
components/config/prefs.rs
Outdated
| dom_worklet_timeout_ms: 10, | ||
| dom_visual_viewport_enabled: false, | ||
| accessibility_enabled: false, | ||
| accessibility_enabled: true, |
| ConstellationWebView::new(webview_id, browsing_context_id, user_content_manager_id), | ||
| ); | ||
|
|
||
| // TODO: enable accessibility if necessary? |
There was a problem hiding this comment.
i think something like this will be necessary. embedders think in terms of webviews, not in terms of pipelines, since those change every time you navigate. for example, servoshell will want to have a subtree graft for each tab, but it shouldn’t have to know which pipeline is currently loaded in each tab.
There was a problem hiding this comment.
some thoughts from our pairing today…
does the webview get its own trivial subtree that the pipeline subtrees are grafted into, or does the webview lend its subtree to whatever pipeline is currently loaded in its top-level browsing context? i think we want to give each webview its own trivial subtree, because that’s conceptually simpler, and less likely to cause problems when we deal with bfcache issues (see below)?
each pipeline gets a subtree, and likely its own subtree id (see above). but what about navigating then clicking back? bfcache means that the previous page should generally be revived cheaply and instantly, but as soon as we disconnect the previous page’s subtree, we think the accesskit consumer would destroy the subtree contents. will we need some way of keeping the subtree contents around, but inactive? maybe we can use the AT-SPI showing state (≈ !accesskit::Node::is_hidden()), which firefox also uses for inactive tabs? this is not necessarily just a performance optimisation, because destroying and recreating content in the a11y tree may have AT side effects. i think to answer this we need to know two things: (a) what do ATs do? and (b) what do other browsers do?
delan
force-pushed
the
accessibility-tree
branch
3 times, most recently
from
2897a88 to
450e963
Compare
alice
force-pushed
the
accessibility-tree
branch
from
c52c94d to
4a980fa
Compare
delan
force-pushed
the
accessibility-tree
branch
4 times, most recently
from
8e5b6e1 to
50163f6
Compare
| self.embedder_proxy.send(EmbedderMsg::AccessibilityTreeId( | ||
| webview_id, | ||
| webview.active_top_level_pipeline_id.accesskit_tree_id(), | ||
| )); |
There was a problem hiding this comment.
notable change: one of three locations where we set a11y tree id. relies on deterministic PipelineId → TreeId conversion to avoid asking script thread for a stored TreeId. need to document downstream-activation-flag pattern (this is the “existing” case)
delan
force-pushed
the
accessibility-tree
branch
4 times, most recently
from
fee071d to
bda072a
Compare
|
we have a crash when deactivating then reactivating accessibility. this should repro with plasma on linux. one window:
two windows:
in both cases, we get the subtree-before-graft panic: panic details |
this patch plumbs the webview accessibility trees (#43029, #43556) into servoshell. we add a global flag in servoshell, which is set when the platform activates accessibility and cleared when the platform deactivates accessibility. the flag in turn [activates accessibility](https://doc.servo.org/servo/struct.WebView.html#method.set_accessibility_active) in existing and new webviews. Testing: none in this patch, but will be covered by end-to-end platform a11y tests in WPT Fixes: part of #4344, extracted from our work in #42338 Signed-off-by: delan azabani <[email protected]> Co-authored-by: Luke Warlow <[email protected]> Co-authored-by: Alice Boxhall <[email protected]>
alice
force-pushed
the
accessibility-tree
branch
2 times, most recently
from
e4b3021 to
3e015dc
Compare
|
I got to the bottom of the crash: we weren't sending a new TreeUpdate with the graft node for the web contents' subtree after re-enabling accessibility, so we were getting a TreeUpdate for the web contents' subtree that had nowhere to go. Fixed by clearing out |
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
alice
force-pushed
the
accessibility-tree
branch
from
3e015dc to
779b7f3
Compare
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
alice
force-pushed
the
accessibility-tree
branch
from
779b7f3 to
9066f3b
Compare
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
alice
force-pushed
the
accessibility-tree
branch
from
9066f3b to
1f4b668
Compare
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
alice
force-pushed
the
accessibility-tree
branch
from
1f4b668 to
039d488
Compare
alice
changed the title
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
|
Just noticed a crash when navigating back in history, investigating now! |
|
Ok, new crash also fixed: 1e6f091 Investigating it made me realise I'd inadvertently lied in the issue description, but the code now matches the accidental lie: we were previously rebuilding the accessibility tree and sending a tree update containing the full tree on every reflow, not just when accessibility was enabled and when navigating. The fix for the crash now resets the The spammy tree updates caused the crash because in servoshell we "only" send tree updates to accesskit once per tick, after we've ensured that graft nodes exist for every active webview's top level pipeline. The crash was caused by a tree update sent immediately before the navigation which was still pending at the point of the next gui tick. While avoiding spammy updates should generally prevent this in most cases, I've added code to check the TreeId of each update before we send it to accesskit as well. |
Co-authored-by: Luke Warlow <[email protected]> Co-authored-by: Alice Boxhall <[email protected]> Signed-off-by: delan azabani <[email protected]>
See servo#42338 (comment) Signed-off-by: Alice Boxhall <[email protected]>
- Clear needs_accessibility_update after accessibility update in LayoutThread - Check all TreeUpdates in servoshell Gui::update() are for a TreeId which has been grafted. Signed-off-by: Alice Boxhall <[email protected]>
This simply tests that after set_accessibility_active() is called, Servo: 1) Generates an accessibility tree for its (sole) webview 2) Whose root node is a `RootWebArea`. Signed-off-by: Alice Boxhall <[email protected]> Co-authored-by: Delan Azabani <[email protected]>
Signed-off-by: delan azabani <[email protected]>
…o be sent Signed-off-by: Alice Boxhall <[email protected]> Co-authored-by: Delan Azabani <[email protected]>
Signed-off-by: delan azabani <[email protected]>
|
The lint found some issues with Cargo.lock; I'll have to look into those next week. |
6 participants
This change introduces the
accessibility_treemodule, containing code to build an in-memory representation of a very basic accessibility tree for web contents. Currently, the tree for a given document contains:RootWebAreawhich has the document root node as its sole child,Unknownnode for the root DOM node,GenericContainernode for each DOM element, andTextRunnode for each text node.This allows us to make basic assertions about the tree contents in the
accessibilitytest by doing a tree walk to find text nodes and checking their contents.Right now, the tree is rebuilt from scratch when accessibility is enabled and when a navigation occurs (via
Constellation::set_frame_tree_for_webview()sendingScriptThreadMessage::SetAccessibilityActive); it's not responsive to changes in the page.Fixes: Part of #4344