Add support for setting the heading level for web semantics (#97894) #41435

victorgalo · 2023-04-23T16:22:09Z

This change adds a new property in Semantics widget that would take an integer value corresponding to the heading levels defined by the ARIA heading role. This is necessary in order to get proper accessibility and usability in a website for users who rely on screen readers and other assistive technologies.

Issue fixed by this PR:
flutter/flutter#97894

Framework part:
flutter/flutter#125771

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

victorgalo · 2023-04-30T18:44:27Z

The build is failing because some integration tests are failing in the framework side, I need to integrate both changes together (framework and engine part) for integration tests to pass. Please, someone can tell me how I should deal with this?.

Maybe @yjbanov can shed some light?

flutter-dashboard · 2023-05-20T18:47:25Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

yjbanov · 2023-06-26T23:13:30Z

It's a tough one. We might have to temporarily disable the test, wait for the engine change to roll, then enable it again.

yjbanov · 2023-06-26T23:14:55Z

Would you be willing to wait for #43159 to land before landing this PR? Currently LabelAndValue is broken in that it steals the ARIA role attribute from more specific roles. After I land that, we can introduce a secondary RoleManager that would manage aria-level without touching the role attribute.

yjbanov · 2023-06-27T20:57:51Z

Update: #43159 has landed.

yjbanov · 2023-06-27T21:02:31Z

Here's what I'd recommend doing given the framework established in #43159.

We create a new class Heading extends RoleManager. This role manager is responsible for the aria-level attribute.
PrimaryRoleManager.withBasics constructor adds the Heading role manager if applicable. The Heading role is applicable if headingLevel != -1.

Alternatively, we can give the task of setting aria-level to LabelAndValue. I'm not sure if the two are orthogonal to each other 🤔

Let's keep LabelAndValue as is. In a separate PR we can fix the "header" vs "heading" vs "banner" situation.

odrya12 · 2023-07-26T10:49:21Z

hi there is any progress with the last checks?

victorgalo · 2023-09-03T23:30:57Z

Here's what I'd recommend doing given the framework established in #43159.

We create a new class Heading extends RoleManager. This role manager is responsible for the aria-level attribute.

PrimaryRoleManager.withBasics constructor adds the Heading role manager if applicable. The Heading role is applicable if headingLevel != -1.

Alternatively, we can give the task of setting aria-level to LabelAndValue. I'm not sure if the two are orthogonal to each other 🤔

Let's keep LabelAndValue as is. In a separate PR we can fix the "header" vs "heading" vs "banner" situation.

Nice ideas!, thank you very much. I will create a Heading class extending RoleManager as you proposed. Sorry for my late response, I will try to implement it during this week.

# Conflicts: # lib/ui/semantics.dart # lib/web_ui/lib/src/engine/semantics/label_and_value.dart # lib/web_ui/lib/src/engine/semantics/semantics.dart

lib/ui/semantics.dart

lib/web_ui/lib/semantics.dart

lib/web_ui/lib/src/engine/semantics/heading.dart

lib/web_ui/lib/src/engine/semantics/semantics.dart

yjbanov · 2023-09-18T21:56:19Z

lib/web_ui/test/engine/semantics/semantics_test.dart

+      label: 'Header of the page',
+      transform: Matrix4.identity().toFloat64(),
+      rect: const ui.Rect.fromLTRB(0, 0, 100, 50),
+    );


Let's also test that we can update it and clear aria-level when it's gone (assuming having the heading role without aria-level is a meaningful state).

Hey @yjbanov, I'd say i's not a meaningful state, aria-level is required for "heading" role. Do you think I still should add it?.

Nope, if it's not an expected state, we don't need to test it.

auto-submit · 2024-05-28T17:09:47Z

auto label is removed for flutter/engine/41435, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-05-28T22:39:18Z

auto label is removed for flutter/engine/41435, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-05-29T17:10:20Z

auto label is removed for flutter/engine/41435, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

chunhtai · 2024-05-30T16:37:30Z

Hi @victorgalo
looks like the parameter can't be required until we migrate framework

le:///b/s/w/ir/x/w/recipe_cleanup/builder/src/out/wasm_release/flutter_web_sdk/kernel/ddc_outline_sound.dill --verbosity=error --sound-null-safety
[  +13 ms] <- compile org-dartlang-app:///main.dart
[+7088 ms] lib/src/semantics/semantics.dart:2782:23: Error: Required named parameter 'headingLevel' must be provided.
[   +1 ms]     builder.updateNode(
[        ]                       ^
[+20417 ms] "flutter test" took 34,678ms.
[   +4 ms] Failed to compile
[        ] 
           #0      throwToolExit (package:flutter_tools/src/base/common.dart:10:3)
           #1      WebTestCompiler._compileJS (package:flutter_tools/src/test/web_test_compiler.dart:194:7)
           <asynchronous suspension>
           #2      _FlutterTestRunnerImpl.runTests (package:flutter_tools/src/test/runner.dart:176:34)
           <asynchronous suspension>
           #3      TestCommand.runCommand (package:flutter_tools/src/commands/test.dart:585:16)
           <asynchronous suspension>
           #4      FlutterCommand.run.<anonymous closure> (package:flutter_tools/src/runner/flutter_command.dart:1396:27)
           <asynchronous suspension>
           #5      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #6      CommandRunner.runCommand (package:args/command_runner.dart:212:13)
           <asynchronous suspension>
           #7      FlutterCommandRunner.runCommand.<anonymous closure> (package:flutter_tools/src/runner/flutter_command_runner.dart:372:9)
           <asynchronous suspension>
           #8      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #9      FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:308:5)
           <asynchronous suspension>
           #10     run.<anonymous closure>.<anonymous closure> (package:flutter_tools/runner.dart:130:9)
           <asynchronous suspension>
           #11     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #12     main (package:flutter_tools/executable.dart:93:3)
           <asynchronous suspension>

can you update the code?

chunhtai

The ci failure is bury deep in the code, It took me a while to understand it is not a flaky infra. Sorry for the delay

chunhtai · 2024-05-30T16:38:11Z

lib/ui/semantics.dart

@@ -823,6 +835,7 @@ abstract class SemanticsUpdateBuilder {
    required Int32List childrenInTraversalOrder,
    required Int32List childrenInHitTestOrder,
    required Int32List additionalActions,
+    required int headingLevel,


let's make this not required and give it a default value of 0

chunhtai · 2024-05-30T16:38:26Z

lib/ui/semantics.dart

@@ -893,8 +906,13 @@ base class _NativeSemanticsUpdateBuilder extends NativeFieldWrapperClass1 implem
    required Int32List childrenInTraversalOrder,
    required Int32List childrenInHitTestOrder,
    required Int32List additionalActions,
+    required int headingLevel,


chunhtai · 2024-05-30T16:38:55Z

lib/web_ui/lib/semantics.dart

@@ -283,6 +283,7 @@ class SemanticsUpdateBuilder {
    required Int32List childrenInTraversalOrder,
    required Int32List childrenInHitTestOrder,
    required Int32List additionalActions,
+    required int headingLevel,


…ework part is landed

chunhtai · 2024-06-03T16:43:59Z

Not entirely sure why this only show up now, maybe due to merge?

Looks like you need to also modify the test class

[  +14 ms] /b/s/w/ir/x/w/recipe_cleanup/builder/src/flutter/prebuilts/linux-x64/dart-sdk/bin/dartaotruntime --disable-dart-dev /b/s/w/ir/x/w/recipe_cleanup/builder/src/flutter/prebuilts/linux-x64/dart-sdk/bin/snapshots/frontend_server_aot.dart.snapshot --sdk-root /b/s/w/ir/x/w/recipe_cleanup/builder/src/out/wasm_release/flutter_web_sdk/ --incremental --target=dartdevc --experimental-emit-debug-metadata -DDART_HHH_BOT=false -DFLUTTER_WEB_AUTO_DETECT=false -DFLUTTER_WEB_USE_SKIA=false --output-dill /b/s/w/ir/x/t/flutter_tools.PRIJQD/flutter_test.JSZLXR/out --packages /b/s/w/ir/x/w/flutter/packages/flutter/.dart_tool/package_config.json -Ddart.vm.profile=false -Ddart.vm.product=false --enable-asserts --track-widget-creation --filesystem-root /b/s/w/ir/x/w/flutter/packages/flutter/test --filesystem-root /b/s/w/ir/x/t/flutter_tools.PRIJQD/flutter_test.JSZLXR/ --filesystem-scheme org-dartlang-app --initialize-from-dill build/45d48c4c9128ad4b2557fab909a9764a.cache.dill.track.dill --platform file:///b/s/w/ir/x/w/recipe_cleanup/builder/src/out/wasm_release/flutter_web_sdk/kernel/ddc_outline_sound.dill --verbosity=error --sound-null-safety
[  +13 ms] <- compile org-dartlang-app:///main.dart
[+2475 ms] org-dartlang-app:///semantics/semantics_update_test.dart:181:8: Error: The method 'SemanticsUpdateBuilderSpy.updateNode' has fewer named arguments than those of overridden method 'SemanticsUpdateBuilder.updateNode'.
[   +1 ms]   void updateNode({
[        ]        ^
[        ] org-dartlang-sdk:///lib/ui/semantics.dart: Context: This is the overridden method ('updateNode').
[        ] org-dartlang-app:///semantics/semantics_update_test.dart:181:8: Error: The method 'SemanticsUpdateBuilderSpy.updateNode' doesn't have the named parameter 'headingLevel' of overridden method 'SemanticsUpdateBuilder.updateNode'.
[        ]   void updateNode({
[        ]        ^
[        ] org-dartlang-sdk:///lib/ui/semantics.dart: Context: This is the overridden method ('updateNode').
[+24591 ms] "flutter test" took 33,553ms.
[   +3 ms] Failed to compile
[        ] 
           #0      throwToolExit (package:flutter_tools/src/base/common.dart:10:3)
           #1      WebTestCompiler._compileJS (package:flutter_tools/src/test/web_test_compiler.dart:194:7)
           <asynchronous suspension>
           #2      _FlutterTestRunnerImpl.runTests (package:flutter_tools/src/test/runner.dart:176:34)
           <asynchronous suspension>
           #3      TestCommand.runCommand (package:flutter_tools/src/commands/test.dart:585:16)
           <asynchronous suspension>
           #4      FlutterCommand.run.<anonymous closure> (package:flutter_tools/src/runner/flutter_command.dart:1396:27)
           <asynchronous suspension>
           #5      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #6      CommandRunner.runCommand (package:args/command_runner.dart:212:13)
           <asynchronous suspension>
           #7      FlutterCommandRunner.runCommand.<anonymous closure> (package:flutter_tools/src/runner/flutter_command_runner.dart:372:9)
           <asynchronous suspension>
           #8      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #9      FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:308:5)
           <asynchronous suspension>
           #10     run.<anonymous closure>.<anonymous closure> (package:flutter_tools/runner.dart:130:9)
           <asynchronous suspension>
           #11     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:153:19)
           <asynchronous suspension>
           #12     main (package:flutter_tools/executable.dart:93:3)
           <asynchronous suspension>

chunhtai · 2024-06-03T16:45:28Z

and here as well

 info - scenario_app/lib/src/locale_initialization.dart:81:23 - The value of the argument is redundant because it matches the default value. Try removing the argument. - avoid_redundant_argument_values
   info - scenario_app/lib/src/locale_initialization.dart:141:23 - The value of the argument is redundant because it matches the default value. Try removing the argument. - avoid_redundant_argument_values

chunhtai · 2024-06-04T16:29:51Z

no worry I can fix this from my side

…49671) The pr flutter/engine#41435 adds a new property into updateNode. The pr migrate the method in framework first to unblock the engine pr

…flutter#97894) (flutter/engine#41435)

…149699) flutter/engine@e884690...11a32d4 2024-06-04 vtrgalo@gmail.com Add support for setting the heading level for web semantics (#97894) (flutter/engine#41435) 2024-06-04 skia-flutter-autoroll@skia.org Roll Dart SDK from 2d7d6facc2b4 to 343c20614708 (2 revisions) (flutter/engine#53203) 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 matanl@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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…125771) This change adds a new property in Semantics widget that would take an integer corresponding to the heading levels defined by the ARIA heading role. This is necessary in order to get proper accessibility and usability in a website for users who rely on screen readers and other assistive technologies. Issue fixed by this PR: fixes #97894 Engine part: flutter/engine#41435

victorgalo · 2024-06-07T21:15:20Z

no worry I can fix this from my side
Thank you @chunhtai !!

Add support for setting the heading level for web semantics (#97894)

a341b26

flutter-dashboard bot added embedder Related to the embedder API platform-web Code specifically for the web engine labels Apr 23, 2023

victorgalo added 4 commits April 24, 2023 19:25

Add support for setting the heading level for web semantics (#97894)

eff0206

Merge branch 'main' into add-semantics-heading-level

7f8ef2c

Add support for setting the heading level for web semantics (#97894)

9029d63

Add support for setting the heading level for web semantics (#97894)

260d4bd

victorgalo mentioned this pull request Apr 30, 2023

Add support for setting the heading level for web semantics (#97894) flutter/flutter#125771

Merged

8 tasks

Merge branch 'main' into add-semantics-heading-level

baea4c1

victorgalo marked this pull request as ready for review April 30, 2023 18:44

victorgalo added 3 commits September 9, 2023 16:25

Merge branch 'main' into add-semantics-heading-level

bf0fe7f

# Conflicts: # lib/ui/semantics.dart # lib/web_ui/lib/src/engine/semantics/label_and_value.dart # lib/web_ui/lib/src/engine/semantics/semantics.dart

Merge branch 'main' into add-semantics-heading-level

b773f81

Remove trailing spaces

90de9fc

github-actions bot removed the embedder Related to the embedder API label Sep 9, 2023

victorgalo added 4 commits September 10, 2023 00:38

Merge branch 'main' into add-semantics-heading-level

c9ac350

Add export for new Heading role

c8ccd31

Fix references for new Heading role

0241f07

Remove unused import

6298a5f

yjbanov reviewed Sep 18, 2023

View reviewed changes

victorgalo added 2 commits September 20, 2023 01:15

Fix PR comments (41435)

36c5fd6

Remove trailing whitespace

30153e0

victorgalo mentioned this pull request Sep 19, 2023

Skip test temporarily until headingLevel is added in engine (issue 41… flutter/flutter#135077

Merged

8 tasks

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2024

goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2024

Merge branch 'main' into add-semantics-heading-level

1b0e241

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2024

chunhtai reviewed May 30, 2024

View reviewed changes

victorgalo added 2 commits June 1, 2024 03:13

Merge branch 'main' into add-semantics-heading-level

e918e20

Change semantics headingLevel attribute to be not required until fram…

4383464

…ework part is landed

fix lint

5186055

chunhtai mentioned this pull request Jun 4, 2024

Prepares semantics_update_test for upcoming heading level changes flutter/flutter#149671

Merged

9 tasks

Merge branch 'main' into add-semantics-heading-level

af5e70a

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024

auto-submit bot merged commit 11a32d4 into flutter:main Jun 4, 2024
31 checks passed

engine-flutter-autoroll mentioned this pull request Jun 5, 2024

Roll Flutter Engine from e88469090fed to 11a32d43e3f6 (2 revisions) flutter/flutter#149699

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 5, 2024

11a32d43e Add support for setting the heading level for web semantics (…

e5fc6da

…flutter#97894) (flutter/engine#41435)

localpcguy mentioned this pull request Jun 5, 2024

[Semantics] Add support for setting the heading level flutter/flutter#97894

Closed

koral-- mentioned this pull request Sep 30, 2024

[Semantics] Add support for setting the heading level on iOS and macOS flutter/flutter#155928

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for setting the heading level for web semantics (#97894) #41435

Add support for setting the heading level for web semantics (#97894) #41435

victorgalo commented Apr 23, 2023 •

edited

Loading

victorgalo commented Apr 30, 2023

flutter-dashboard bot commented May 20, 2023

yjbanov commented Jun 26, 2023

yjbanov commented Jun 26, 2023 •

edited

Loading

yjbanov commented Jun 27, 2023

yjbanov commented Jun 27, 2023

odrya12 commented Jul 26, 2023

victorgalo commented Sep 3, 2023

yjbanov Sep 18, 2023

victorgalo Sep 22, 2023

yjbanov Nov 27, 2023

auto-submit bot commented May 28, 2024

auto-submit bot commented May 28, 2024

auto-submit bot commented May 29, 2024

chunhtai commented May 30, 2024 •

edited

Loading

chunhtai left a comment

chunhtai May 30, 2024

chunhtai May 30, 2024

chunhtai May 30, 2024

chunhtai commented Jun 3, 2024 •

edited

Loading

chunhtai commented Jun 3, 2024

chunhtai commented Jun 4, 2024

victorgalo commented Jun 7, 2024

Add support for setting the heading level for web semantics (#97894) #41435

Add support for setting the heading level for web semantics (#97894) #41435

Conversation

victorgalo commented Apr 23, 2023 • edited Loading

Pre-launch Checklist

victorgalo commented Apr 30, 2023

flutter-dashboard bot commented May 20, 2023

yjbanov commented Jun 26, 2023

yjbanov commented Jun 26, 2023 • edited Loading

yjbanov commented Jun 27, 2023

yjbanov commented Jun 27, 2023

odrya12 commented Jul 26, 2023

victorgalo commented Sep 3, 2023

yjbanov Sep 18, 2023

Choose a reason for hiding this comment

victorgalo Sep 22, 2023

Choose a reason for hiding this comment

yjbanov Nov 27, 2023

Choose a reason for hiding this comment

auto-submit bot commented May 28, 2024

auto-submit bot commented May 28, 2024

auto-submit bot commented May 29, 2024

chunhtai commented May 30, 2024 • edited Loading

chunhtai left a comment

Choose a reason for hiding this comment

chunhtai May 30, 2024

Choose a reason for hiding this comment

chunhtai May 30, 2024

Choose a reason for hiding this comment

chunhtai May 30, 2024

Choose a reason for hiding this comment

chunhtai commented Jun 3, 2024 • edited Loading

chunhtai commented Jun 3, 2024

chunhtai commented Jun 4, 2024

victorgalo commented Jun 7, 2024

victorgalo commented Apr 23, 2023 •

edited

Loading

yjbanov commented Jun 26, 2023 •

edited

Loading

chunhtai commented May 30, 2024 •

edited

Loading

chunhtai commented Jun 3, 2024 •

edited

Loading