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

Announce alerts through SemanticsService on Windows #37173

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Announce alerts through SemanticsService on Windows #37173

merged 3 commits into from
Nov 3, 2022

Conversation

yaakovschectman
Copy link
Contributor

Fix for PR 36966, which caused a breaking change as-merged. By adding an implementation of IServiceProvider, the previous problem is resolved.

Addresses flutter/flutter#133059

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@yaakovschectman yaakovschectman self-assigned this Oct 31, 2022
loic-sharma
loic-sharma approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM

@yaakovschectman
Copy link
Contributor Author

Changes specifically to this PR compared to the previous begin with commit 5c358a8

a-wallen
a-wallen approved these changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@a-wallen a-wallen left a comment

Choose a reason for hiding this comment

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

LGTM. This is a good step in the right direction. The only thing that concerns me is the amount of functions that return E_NOTIMPL. The past 2 a11y bugs that I've worked on are crashes that are related to unimplemented boilerplate.

@yaakovschectman yaakovschectman merged commit 74f0219 into flutter:main Nov 3, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 3, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2022
@engine-flutter-autoroll
74f021922 Announce alerts through SemanticsService on Windows (flutte…
1d595c7 
…r/engine#37173)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 3, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2022
@engine-flutter-autoroll
74f021922 Announce alerts through SemanticsService on Windows (flutte…
915b0a9 
…r/engine#37173)
GaryQian pushed a commit to flutter/flutter that referenced this pull request Nov 3, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 840a7b346216 to 66b244d9fa0a (25 revisions) (#…
44ecbbc 
…114640)

* 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221)

* 491032cfc Update docs to mention felt build --host (flutter/engine#37224)

* 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392)

* 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226)

* 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239)

* 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162)

* 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266)

* c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267)

* d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270)

* edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269)

* c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249)

* 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273)

* 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275)

* 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276)

* fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277)

* 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198)

* 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278)

* 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279)

* 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282)

* 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173)

* a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284)

* 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285)

* d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288)

* 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290)

* 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)
@cbracken cbracken mentioned this pull request Nov 9, 2022
Closed
schwa423 pushed a commit to schwa423/engine that referenced this pull request Nov 16, 2022
@yaakovschectman @schwa423
Announce alerts through SemanticsService on Windows (flutter#37173)
cf3d94e 
* Corresponds to changes by reverted PR flutter#36966.

* Changes on top of original reverted PR that add IServiceProvider and COM_INTERFACE_ENTRY's to the new node classes.

* self_com_ unused
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
@engine-flutter-autoroll @shogohida
Roll Flutter Engine from 840a7b346216 to 66b244d9fa0a (25 revisions) (f…
1381740 
…lutter#114640)

* 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221)

* 491032cfc Update docs to mention felt build --host (flutter/engine#37224)

* 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392)

* 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226)

* 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239)

* 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162)

* 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266)

* c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267)

* d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270)

* edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269)

* c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249)

* 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273)

* 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275)

* 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276)

* fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277)

* 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198)

* 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278)

* 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279)

* 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282)

* 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173)

* a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284)

* 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285)

* d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288)

* 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290)

* 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)
@PoojaPatilCybage
Copy link

flutter/flutter#113059 is still not fixed

@yaakovschectman
Copy link
Contributor Author

@PoojaPatilCybage Can you elaborate on this? I, and anyone else who has tested this to my knowledge, hear announcements read aloud when fired.

@loic-sharma
Copy link
Member

loic-sharma commented Mar 15, 2023 •
edited
Loading

@PoojaPatilCybage flutter/flutter#113059 was to implement support for accessibility alerts on Windows to have parity with other Flutter platforms. However, there are some known gaps in the Flutter framework where features do not fire accessibility alerts as expected. For example, form validation errors are currently not announced by screen readers flutter/flutter#99715. Is this what you're referring to?

If you are referring to another issue, please create a new GitHub issue with:

  1. A minimal app that reproduces the bug
  2. Steps to reproduce the bug
  3. Any relevant additional information, like affected platforms, Flutter version, etc...

Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loic-sharma loic-sharma loic-sharma approved these changes

@a-wallen a-wallen a-wallen approved these changes

@cbracken cbracken Awaiting requested review from cbracken

@Hixie Hixie Awaiting requested review from Hixie

@gaaclarke gaaclarke Awaiting requested review from gaaclarke

Assignees

@yaakovschectman yaakovschectman

Labels
accessibility affects: desktop platform-windows
Projects
Flutter Desktop
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yaakovschectman @PoojaPatilCybage @loic-sharma @a-wallen