Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[web:a11y] add platform view role #44188

Merged
merged 3 commits into from
Aug 1, 2023
Merged

[web:a11y] add platform view role #44188

merged 3 commits into from
Aug 1, 2023

Conversation

yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Jul 31, 2023

Add PlatformViewRoleManager, the primary role manager for platform views. Currently, all it does is manager the aria-owns attribute that determines the screen reader traversal order of the platform view w.r.t. surrounding content.

This is a partial fix for flutter/flutter#124765. While it does not address literally using the TAB key as a means for traversing widgets, it does address traversal via screen reader shortcuts.

@yjbanov yjbanov requested a review from ditman July 31, 2023 21:31
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Jul 31, 2023
@yjbanov yjbanov force-pushed the a11y-platform-views branch from 50b5eed to a5c42bd Compare July 31, 2023 21:47
@yjbanov yjbanov force-pushed the a11y-platform-views branch from a5c42bd to 9e88f5f Compare July 31, 2023 22:16
ditman
ditman approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Looks good! I hope this improves the navigation in most screen readers! (except for Safari)

lib/web_ui/lib/src/engine/platform_views/content_manager.dart Outdated
@@ -108,7 +108,7 @@ class PlatformViewManager {
/// The resulting DOM for the `contents` of a Platform View looks like this:
///
/// ```html
/// <flt-platform-view slot="...">
/// <flt-platform-view id="flt-platform-view-VIEW_ID" slot="...">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// <flt-platform-view id="flt-platform-view-VIEW_ID" slot="...">
/// <flt-platform-view id="flt-pv-VIEW_ID" slot="...">

(According to https://github.com/flutter/engine/pull/44188/files#diff-0ee07db46a6272991cd49c1bace8240dc5efaa8a0efabbbfc3f80722589653f1R19)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I initially named it "platform-view", but then found the slot name and decided to be consistent with that, but forgot to update the docs.

lib/web_ui/lib/src/engine/semantics/platform_view.dart
void update() {
super.update();

if (semanticsObject.isPlatformView) {
Copy link
Member

Choose a reason for hiding this comment

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

So we already had isPlatformView but it was never used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used elsewhere to set proper pointer event handling, but that aspect is not handled by this role manager since the logic is applied to all roles.

lib/web_ui/test/engine/platform_views/slots_test.dart
Comment on lines +39 to +45
test('getPlatformViewSlotName', () {
expect(getPlatformViewSlotName(42), 'flt-pv-slot-42');
});

test('getPlatformViewDomId', () {
expect(getPlatformViewDomId(42), 'flt-pv-42');
});
Copy link
Member

Choose a reason for hiding this comment

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

Good Boy Scout! Thanks!

@yjbanov yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2023
@auto-submit auto-submit bot merged commit afe200d into flutter:main Aug 1, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
c30d396
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
5249412
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
95fa8c1
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
73a2df7
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
86638b8
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
fbcfcd0
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
513bcf1
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
3027c11
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
d519d8c
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
9130a59
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
101e2d0
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
b5218ee
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
e52f15a
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from 9dae7b708bda to d6b962d0e36d (25 revi…
f9e4567 
…sions) (#131785)

Manual roll requested by jacksongardner@google.com

flutter/engine@9dae7b7...d6b962d

2023-08-02 zanderso@users.noreply.github.com Revert "[Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture." (flutter/engine#44262)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from c6577d328585 to 6009cc6d7d80 (2 revisions) (flutter/engine#44264)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 11cef17b53ac to d61a50c155bd (1 revision) (flutter/engine#44261)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 1c0bba7c1053 to c6577d328585 (2 revisions) (flutter/engine#44260)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 8ff03ebf7eaa to afbaf4216fc8 (1 revision) (flutter/engine#44259)
2023-08-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 3JSF6hXLRdAK1wUN1... to Hx7ap5qcoqRIknnnG... (flutter/engine#44258)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from a21631c02e45 to 11cef17b53ac (1 revision) (flutter/engine#44256)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6807d8b8a9d3 to 1c0bba7c1053 (1 revision) (flutter/engine#44255)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 5d4b3645d0dc to a21631c02e45 (1 revision) (flutter/engine#44252)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6087a5224c6f to 6807d8b8a9d3 (2 revisions) (flutter/engine#44250)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6cb888889ad8 to 6087a5224c6f (1 revision) (flutter/engine#44249)
2023-08-02 jonahwilliams@google.com [Impeller] Give a fixed timeout for acquireNextImageKHR. (flutter/engine#44241)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 7c03426705da to 8ff03ebf7eaa (2 revisions) (flutter/engine#44243)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 4deae93198f9 to 6cb888889ad8 (1 revision) (flutter/engine#44240)
2023-08-01 hterkelsen@users.noreply.github.com [canvaskit] Enable CanvasKit to compute tight SkPicture bounds (flutter/engine#43361)
2023-08-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 197fc0d7fea2 to 7c03426705da (2 revisions) (flutter/engine#44237)
2023-08-01 john@johnmccutchan.com [Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture. (flutter/engine#44087)
2023-08-01 yjbanov@google.com [web:a11y] add platform view role (flutter/engine#44188)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 18cf818e044f to 4deae93198f9 (1 revision) (flutter/engine#44236)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 58c031441cbb to 18cf818e044f (6 revisions) (flutter/engine#44234)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from g0dgOL2IlZQJCK4El... to 3JSF6hXLRdAK1wUN1... (flutter/engine#44233)
2023-08-01 skia-flutter-autoroll@skia.org Roll ANGLE from b53d99d87e6a to 5d4b3645d0dc (1 revision) (flutter/engine#44231)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from lwCo6le6r0X-Srvx3... to KPSWBhOvG6piddBQG... (flutter/engine#44230)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from d53f7b880651 to 58c031441cbb (2 revisions) (flutter/engine#44229)
2023-08-01 58529443+srujzs@users.noreply.github.com Remove extends JSTypedArray from JSUint8Array1 (flutter/engine#44175)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from lwCo6le6r0X- to KPSWBhOvG6pi
  fuchsia/sdk/core/mac-amd64 from g0dgOL2IlZQJ to Hx7ap5qcoqRI

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
afe200dee [web:a11y] add platform view role (flutter/engine#44188)
5b79cea
vashworth pushed a commit to vashworth/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll @vashworth
Manual roll Flutter Engine from 9dae7b708bda to d6b962d0e36d (25 revi…
908ed76 
…sions) (flutter#131785)

Manual roll requested by jacksongardner@google.com

flutter/engine@9dae7b7...d6b962d

2023-08-02 zanderso@users.noreply.github.com Revert "[Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture." (flutter/engine#44262)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from c6577d328585 to 6009cc6d7d80 (2 revisions) (flutter/engine#44264)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 11cef17b53ac to d61a50c155bd (1 revision) (flutter/engine#44261)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 1c0bba7c1053 to c6577d328585 (2 revisions) (flutter/engine#44260)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 8ff03ebf7eaa to afbaf4216fc8 (1 revision) (flutter/engine#44259)
2023-08-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 3JSF6hXLRdAK1wUN1... to Hx7ap5qcoqRIknnnG... (flutter/engine#44258)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from a21631c02e45 to 11cef17b53ac (1 revision) (flutter/engine#44256)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6807d8b8a9d3 to 1c0bba7c1053 (1 revision) (flutter/engine#44255)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 5d4b3645d0dc to a21631c02e45 (1 revision) (flutter/engine#44252)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6087a5224c6f to 6807d8b8a9d3 (2 revisions) (flutter/engine#44250)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6cb888889ad8 to 6087a5224c6f (1 revision) (flutter/engine#44249)
2023-08-02 jonahwilliams@google.com [Impeller] Give a fixed timeout for acquireNextImageKHR. (flutter/engine#44241)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 7c03426705da to 8ff03ebf7eaa (2 revisions) (flutter/engine#44243)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 4deae93198f9 to 6cb888889ad8 (1 revision) (flutter/engine#44240)
2023-08-01 hterkelsen@users.noreply.github.com [canvaskit] Enable CanvasKit to compute tight SkPicture bounds (flutter/engine#43361)
2023-08-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 197fc0d7fea2 to 7c03426705da (2 revisions) (flutter/engine#44237)
2023-08-01 john@johnmccutchan.com [Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture. (flutter/engine#44087)
2023-08-01 yjbanov@google.com [web:a11y] add platform view role (flutter/engine#44188)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 18cf818e044f to 4deae93198f9 (1 revision) (flutter/engine#44236)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 58c031441cbb to 18cf818e044f (6 revisions) (flutter/engine#44234)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from g0dgOL2IlZQJCK4El... to 3JSF6hXLRdAK1wUN1... (flutter/engine#44233)
2023-08-01 skia-flutter-autoroll@skia.org Roll ANGLE from b53d99d87e6a to 5d4b3645d0dc (1 revision) (flutter/engine#44231)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from lwCo6le6r0X-Srvx3... to KPSWBhOvG6piddBQG... (flutter/engine#44230)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from d53f7b880651 to 58c031441cbb (2 revisions) (flutter/engine#44229)
2023-08-01 58529443+srujzs@users.noreply.github.com Remove extends JSTypedArray from JSUint8Array1 (flutter/engine#44175)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from lwCo6le6r0X- to KPSWBhOvG6pi
  fuchsia/sdk/core/mac-amd64 from g0dgOL2IlZQJ to Hx7ap5qcoqRI

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
@yjbanov @gaaclarke
[web:a11y] add platform view role (flutter#44188)
96b8256 
Add `PlatformViewRoleManager`, the primary role manager for platform views. Currently, all it does is manager the `aria-owns` attribute that determines the screen reader traversal order of the platform view w.r.t. surrounding content.

This is a partial fix for flutter/flutter#124765. While it does not address literally using the TAB key as a means for traversing widgets, it does address traversal via screen reader shortcuts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ditman ditman ditman approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yjbanov @ditman