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

[web:a11y] fix traversal and hit-test orders #32712

Merged
yjbanov merged 5 commits into from
Apr 18, 2022
Merged

[web:a11y] fix traversal and hit-test orders #32712

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3b7f799
[web:a11y] implement traversal and hit-test orders
yjbanov Apr 15, 2022
a63bdd7
remove "we" in a bunch of places
yjbanov Apr 15, 2022
2052773
remove unused intersectionIndicesNew
yjbanov Apr 15, 2022
52076d3
add hit test test; fix platform view offset and scene host init
yjbanov Apr 18, 2022
0ff84fc
browser-specific fixes
yjbanov Apr 18, 2022
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
18 changes: 12 additions & 6 deletions lib/web_ui/lib/src/engine/embedder.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,20 @@ class FlutterViewEmbedder {
.prepareAccessibilityPlaceholder();

glassPaneElementHostNode.nodes.addAll(<html.Node>[
semanticsHostElement,
_accessibilityPlaceholder,
_sceneHostElement!,

// The semantic host goes last because hit-test order-wise we want it to
Copy link
Contributor

@chunhtai chunhtai Apr 15, 2022

Choose a reason for hiding this comment

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

nit: avoid we

Copy link
Contributor

@chunhtai chunhtai Apr 15, 2022

Choose a reason for hiding this comment

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

here and everywhere, we may refer to different entity based on the reader's perspective.

Copy link
Contributor Author

@yjbanov yjbanov Apr 15, 2022

Choose a reason for hiding this comment

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

Done.

// be first. If semantics goes under the scene host, platform views will
// obscure semantic elements.
//
// You may be wondering: wouldn't semantics obscure platform views and
// make then not accessible? At least with some careful planning, that
// should not be the case. The semantics tree makes all of its non-leaf
// elements transparent. This way, if a platform view appears among other
// interactive Flutter widgets, as long as those widgets do not intersect
// with the platform view, the platform view will be reachable.
semanticsHostElement,
Copy link
Contributor

@chunhtai chunhtai Apr 15, 2022

Choose a reason for hiding this comment

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

Is there a test coverage for this?

Copy link
Contributor Author

@yjbanov yjbanov Apr 15, 2022

Choose a reason for hiding this comment

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

In the past I could find a way to write a test that would emulate browser's hit test. But I just found this: https://developer.mozilla.org/en-US/docs/web/api/document/elementsfrompoint. I'm going to try.

Copy link
Contributor Author

@yjbanov yjbanov Apr 18, 2022

Choose a reason for hiding this comment

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

elementsFromPoint worked! I added a test. Also added CanvasKit mode to that test (since platform views are composited differently in CanvasKit), and while doing that discovered that offset was ignored in addPlatformView, so fixed that along the way.

]);

// When debugging semantics, make the scene semi-transparent so that the
Copy link
Member

@ditman ditman Apr 16, 2022

Choose a reason for hiding this comment

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

Now that the accessibility tree renders on top of the scene host, this is probably not required anymore :P

Copy link
Contributor Author

@yjbanov yjbanov Apr 16, 2022

Choose a reason for hiding this comment

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

Good point. Strictly speaking, no. But reducing the transparency of the scene makes the semantics overlay more visible, making it easier to work with. I'll keep the transparency, but I will update the comment.

Expand All @@ -304,11 +315,6 @@ class FlutterViewEmbedder {
PointerBinding.initInstance(glassPaneElement);
KeyboardBinding.initInstance(glassPaneElement);

// Hide the DOM nodes used to render the scene from accessibility, because
// the accessibility tree is built from the SemanticsNode tree as a parallel
// DOM tree.
_sceneHostElement!.setAttribute('aria-hidden', 'true');

if (html.window.visualViewport == null && isWebKit) {
// Older Safari versions sometimes give us bogus innerWidth/innerHeight
// values when the page loads. When it changes the values to correct ones
Expand Down
15 changes: 14 additions & 1 deletion lib/web_ui/lib/src/engine/html/picture.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,20 @@ class PersistedPicture extends PersistedLeafSurface {

@override
html.Element createElement() {
return defaultCreateElement('flt-picture');
final html.Element element = defaultCreateElement('flt-picture');

// The DOM elements used to render pictures are used purely to put pixels on
// the screen. They have no semantic information. If an assistive technology
// attempts to scan picture content it will look like garbage and confuse
// users. UI semantics are exported as a separate DOM tree rendered parallel
// to pictures.
//
// Why do we not hide layer and scene elements from ARIA? Because those
// elements may contain platform views, and we do want platform views to be
// accessible.
element.setAttribute('aria-hidden', 'true');

return element;
}

@override
Expand Down
Loading