[Windows] Introduce an accessibility plugin #50898
Conversation
loic-sharma
force-pushed
the
windows_accessibility_plugin
branch
from
9187af5 to
9487a06
Compare
| const auto& type_itr = map->find(EncodableValue{kTypeKey}); | ||
| const auto& data_itr = map->find(EncodableValue{kDataKey}); | ||
| if (type_itr == map->end() || data_itr == map->end()) { | ||
| return; |
There was a problem hiding this comment.
Should this produce an error if type or data is missing?
There was a problem hiding this comment.
I'm not sure why, but the flutter/accessibility channel does not follow the normal method call protocol. AFAICT it does not support error handling. Ignoring malformed messages appears to be the "correct" implementation:
There was a problem hiding this comment.
Would we want to log an error message? Or just ignore it if the other platforms do so?
There was a problem hiding this comment.
Excellent idea, added!
There was a problem hiding this comment.
Hmm, GitHub seems to be broken... it isn't showing the latest commit... I tried force pushing my changes too 🙃
You can find the latest implementation here: https://github.com/loic-sharma/flutter-engine/blob/c5be9d79e3c55835ece522bd8e3b8261ee19f529/shell/platform/windows/accessibility_plugin.cc#L27-L69
loic-sharma
force-pushed
the
windows_accessibility_plugin
branch
from
61a9d2d to
afbf2e7
Compare
|
Something broke on GitHub... New commits aren't showing up. I'll close this pull request and open a new one: #50975 |
loic-sharma
force-pushed
the
windows_accessibility_plugin
branch
from
de41bb5 to
c5be9d7
Compare
_This is the same pull request as #50898. GitHub broke on the previous pull request so I re-created it_ This moves the logic to handle `flutter/accessibility` messages to a new type, `AccessibilityPlugin`. Notable changes: 1. Windows app no longer crashes if it receives accessibility events it does not support 2. Windows app no longer crashes if it receives accessibility events while in headless mode @yaakovschectman After playing around with this, I ended up using a different pattern than what what I suggested on #50598 (comment). This message handler is simple enough that splitting into a child/base types felt like unnecessary boilerplate. The key thing is separating messaging and implementation logic, which was achieved through the `SetUp` method. Let me know what you think, and sorry for all my flip-flopping on this topic! � This is preparation for: flutter/flutter#143765 Sample app for manual testing: flutter/flutter#113059 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This moves the logic to handle
flutter/accessibilitymessages to a new type,AccessibilityPlugin.Notable changes:
@yaakovschectman After playing around with this, I ended up using a different pattern than what what I suggested on #50598 (comment). This message handler is simple enough that splitting into a child/base types felt like unnecessary boilerplate. The key thing is separating messaging and implementation logic, which was achieved through the
SetUpmethod. Let me know what you think, and sorry for all my flip-flopping on this topic! 😅This is preparation for: flutter/flutter#143765
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.