Announce alerts through SemanticsService on Windows #36966
Conversation
yaakovschectman
requested review from
cbracken,
loic-sharma,
dkwingsmt,
jonahwilliams and
a-wallen
| return nullptr; | ||
| } | ||
| if (child_id == CHILDID_SELF || child_id == kWindowChildId) { | ||
| child_id = CHILDID_SELF; |
There was a problem hiding this comment.
What's the value of setting child_id on this line and line 38 below?
There was a problem hiding this comment.
child_id is a reference to var_id->lVal. Setting it updates the value in the VARIANT passed to the function, so for example, if a method is called in the root node with var_id.lVal == 1 pointing to the window node child, var_id.lVal is set to CHILDID_SELF = 0 and the method is called on the window node.
There was a problem hiding this comment.
Ah I missed the reference, thanks!
There was a problem hiding this comment.
I should make stronger coffee 😂
There was a problem hiding this comment.
I wonder if this method would be clearer if it returned the target and child ID instead of mutating the input var_id. Perhaps it could return a std::optional<std::pair<IAccessible*, VARIANT>>? This would be more verbose but also make its behavior more apparent. Thoughts?
|
@yaakovschectman Just curious, how did you make the decision between implementing this at the Windows native layer versus at the accessibility bridge layer? It seems we could make the |
When trying to research how to go about implementing this, I saw Chromium does this by attaching an optional alert node to whatever AX view node the alert is associated with. This would not be easily translated to our project as we do not use the AX view architecture they do, and our alerts from the framework are not associated with any particular node. Adding a new sibling to the existing root node via generating an update looks like it would require recreating the |
yaakovschectman
force-pushed
the
announce_alert
branch
from
d25eb52 to
74f6924
Compare
|
I chatted with Yaakov offline on #36966 (comment). It seems the concerns with the accessibility bridge approach are surmountable. Here are pro/cons on the accessibility bridge approach over the native MSAA approach for announcing alerts: Pros:
Cons:
|
yaakovschectman
force-pushed
the
announce_alert
branch
from
f34fdd3 to
da7a773
Compare
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
This reverts commit 9915434.
* Corresponds to changes by reverted PR #36966. * Changes on top of original reverted PR that add IServiceProvider and COM_INTERFACE_ENTRY's to the new node classes. * self_com_ unused
…36966)" (flutter#37120) This reverts commit 9915434.
* 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
The platform message used to trigger announcements/alerts by the Framework was previously ignored on Windows, as there was no channel to accept it, nor method to announce it. This PR receives the message to announce a message to the screen reader. The former root node of the Window is now wrapped by a parent node which may also create an alert node as a sibling, and can then notify a
EVENT_SYSTEM_ALERTevent pointing to this alert node to read out an alert.You may test this by running the code sample in the linked issue with either Windows Narrator or NVDA.
Addresses flutter/flutter#113059
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.