Added Semantic header support on Android.#13262
Conversation
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
Change looks good.
/cc @jonahwilliams for if this can be tested with our existing infrastructure.
shell/platform/android/io/flutter/view/AccessibilityBridge.java
Outdated
Show resolved
Hide resolved
…ion to avoid erroneous lint warnings.
|
Thank you for the contribution! This can be unit tested with JUnit, but it is unfortunately painful right now because of our mix of unique build system and no dependency injection in the Java code. See the README for instructions on adding a test. It would probably be preferable to test this with a flutter driver test if possible, but I'm not totally sure if that's possible (as is this wouldn't create any changes that Flutter driver could test). Change itself also LG, but all patches need to be tested unless there's a personal exemption from Hixie (#4). So this is blocked until we have one for it. |
|
Yeah, testing this has been something I have been looking into. Jonah suggested that I look at the integration tests that are in the main flutter tree: https://github.com/flutter/flutter/tree/master/dev/integration_tests/android_semantics_testing Would adding something to these tests for this feature be sufficient, or do we require unit tests in the engine itself? Thx. |
|
Oh, neat.
I think integration tests are generally better since they actually verify that it works in a way that unit tests don't. If it's possible to test it that way I'd prefer it. |
|
Given that (AFAIK) we don't have devicelab devices with accessibility enabled, is the plan here to add some, then take that approach? |
|
We do actually, see flutter/flutter#44031 |
|
Ok, I have landed the integration test for this (flutter/flutter#44031), can I get an approval to push this to master? |
[email protected]:flutter/engine.git/compare/05ab04dbe8cf...6c763bb git log 05ab04d..6c763bb --no-merges --oneline 2019-11-06 [email protected] [fuchsia] Temporarily disable intl provider (flutter/engine#13696) 2019-11-05 [email protected] Fix plugin registrant reflection path. (#44161) (flutter/engine#13698) 2019-11-05 [email protected] Added Semantic header support on Android. (flutter/engine#13262) 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 [email protected] on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md
6 participants
Simple change to add support for the
Semantics.headerfield on Android. Android P added aAccessibilityNodeInfo.setHeadingfunction, so this just hooks that up to our Semantic tree.Fixes: flutter/flutter#41494