-
Notifications
You must be signed in to change notification settings - Fork 6k
Adds fuchsia node roles to accessibility bridge updates. #20385
Conversation
it looks like there are tests in framework side that grab all the possible flag from ui.SemanticsFlags and compare it with semantics property api. This create a circular dependencies. cc @gaaclarke Do you know how should we migrate in this case? |
@abrush21 Heads up, this PR is failing because you need to run the code through the formatter. |
@abrush21 We will probably need to do a three phase transition regarding the test breakage fortunately, there are only two tests that are failing
|
I see that issue, and I also see a separate issue in the build_and_test_linux_unopt_debug check that Chun-Heng mentioned. A couple of framework tests (in packages/flutter/test/widgets/semantics_test.dart and flutter/packages/flutter/test/widgets/custom_painter_test.dart) are failing because of the newly introduced semantics flag. https://api.cirrus-ci.com/v1/task/5696073467428864/logs/test_framework.log. Correct me if I'm wrong, but it looks like there's a circular dependency between this change and those tests -- in order to fix the framework tests, the engine change introducing the new flag needs to land, but in order to land the engine change, the framework tests need to pass. |
@chunhtai Sounds good. When you say "disable" those tests, can I just comment the failing tests out in the test files? Or is there a more formal process for enabling/disabling flutter tests? |
@abrush21 Since this should only exist for a short amount of time I'd say commenting them out is fine. Add a link to the issue and some context at the beginning of the comment. |
5e62d2c
to
e38fc62
Compare
@chunhtai @gaaclarke PR 63595 (disabling failing tests) has merged, so this PR is ready for review again. |
@@ -300,6 +300,7 @@ class SemanticsFlag { | |||
static const int _kIsReadOnlyIndex = 1 << 20; | |||
static const int _kIsFocusableIndex = 1 << 21; | |||
static const int _kIsLinkIndex = 1 << 22; | |||
static const int _kIsSliderIndex = 1 << 23; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we forgot to mention, we also need to update flutter/lib/web_ui/lib/src/ui/semantics.dart
Can you also modify the comment here to also mention this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as well as flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java line 1748
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done.
e38fc62
to
5110981
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This test is causing failures in the engine roll (tested by running the test multiple times on this and the previous engine commit):
|
I didn't see (or don't remember seeing) any other failures that I thought would come from this PR. |
You can check the test results from any of the engine PRs that "mentioned this PR" before my revert... |
@flar Thanks! @abrush21 , it seems like there is one more test you need to disable in the framework in order to merge this PR https://github.com/flutter/flutter/blob/2f96d1f0263dfdaf9f1c8d4f0fa054839342e350/packages/flutter_test/test/matchers_test.dart#L529 |
* 5e54c70 Reland: Enable hybrid composition by default on Android (#20722) (flutter/engine#20864) * 9398717 Roll Skia from db5582b71116 to 44e96bee4b6a (4 revisions) (flutter/engine#20908) * 5f49a95 Add auto plugin registration to FlutterFragmentActivity as well (flutter/engine#20865) * c4c4f34 Wait for first frame before checking layer tree (flutter/engine#20910) * 0773bf0 Roll Skia from 44e96bee4b6a to 3913d3e137ed (2 revisions) (flutter/engine#20909) * 8ec8af7 [windows] Add horizontal scroll support (flutter/engine#20668) * 1bd9b8e Reset Shell::weak_factory_gpu_ on the raster thread (flutter/engine#20869) * d67923f Pass text input key events to the EventResponder if they do not yield characters (flutter/engine#20912) * abe10d1 Roll Dart SDK from 84c3eacc7ba0 to 6eab35f49cbb (2 revisions) (flutter/engine#20913) * 101316b [web] migrate from e2e to integration_test (flutter/engine#20914) * 1f52ec3 Roll Dart SDK from 6eab35f49cbb to 2a5f37d25453 (1 revision) (flutter/engine#20917) * 8019058 Default C++ wrapper templates to EncodableValue (flutter/engine#20760) * 5f3ec38 Roll Fuchsia Mac SDK from sI2DAAmSI... to waj2pOhDh... (flutter/engine#20919) * a651020 Roll Fuchsia Linux SDK from _SVZn8uN2... to 9tLNFbjA1... (flutter/engine#20920) * 696c2aa Roll Skia from 3913d3e137ed to 7b46300fe4ff (4 revisions) (flutter/engine#20924) * 95f2b72 Create root isolate asynchronously (flutter/engine#20142) * 58a6207 Adds fuchsia node roles to accessibility bridge updates. (flutter/engine#20385) * a762143 Roll Dart SDK from 2a5f37d25453 to e8e0d5a539fb (3 revisions) (flutter/engine#20928) * 49d6805 Ensure all images are closed in FlutterImageView (flutter/engine#20842) * d67bda7 Image.toByteData and Picture.toImage implementations (#3) (flutter/engine#20750) * 96efe39 Revert "Adds fuchsia node roles to accessibility bridge updates. (#20385)" (flutter/engine#20936) * 5585ed9 Revert "Create root isolate asynchronously (#20142)" (flutter/engine#20937) * f6270c0 Roll Dart SDK from e8e0d5a539fb to b29f228f62e2 (3 revisions) (flutter/engine#20939) * 15bf1bb [Android R] Integrate DisplayCutouts into viewportMetrics (flutter/engine#20921) * 615e668 Clear the GL context only after submitting the frame (flutter/engine#20931) * ca989b8 Roll Skia from 7b46300fe4ff to 1338a37a1add (16 revisions) (flutter/engine#20943) * 8f3f711 Roll Fuchsia Linux SDK from 9tLNFbjA1... to knpSoAoZq... (flutter/engine#20944) * 873c007 Log exception in addition to the stack trace for unhandled exceptions. (flutter/engine#20935) * d761629 Roll Skia from 1338a37a1add to 8fa3b4e8cde5 (6 revisions) (flutter/engine#20949) * f6920da Roll Skia from 8fa3b4e8cde5 to e9a9ad908226 (5 revisions) (flutter/engine#20952) * 634e499 Use hint freed specifically for image disposal (flutter/engine#20754) * c700479 Revert external size changes to Picture (flutter/engine#20950) * 4353797 Roll Skia from e9a9ad908226 to 3d1d636cd839 (6 revisions) (flutter/engine#20955) * 80f4481 renaming e2e tests to integration (flutter/engine#20954) * 61e057a Clear GL context before Gr context (flutter/engine#20957) * f43c3d7 Roll Fuchsia Mac SDK from waj2pOhDh... to 0r88gDzUP... (flutter/engine#20958) * 5a2db33 Roll Skia from 3d1d636cd839 to 683beccf6776 (13 revisions) (flutter/engine#20961) * efb339f Only clear GL context after changing the thread configuration (flutter/engine#20965) * 58d5132 Roll Fuchsia Linux SDK from knpSoAoZq... to odFvFQV9Z... (flutter/engine#20968) * 3729fdb Roll Skia from 683beccf6776 to a66a9c31a318 (4 revisions) (flutter/engine#20969) * 40fe7f3 Roll Fuchsia Mac SDK from 0r88gDzUP... to gOI3W1UNU... (flutter/engine#20970) * e979c29 Roll Skia from a66a9c31a318 to be72801f29f9 (1 revision) (flutter/engine#20971)
Sounds good, created pull request flutter/flutter#65194. |
* 5e54c70 Reland: Enable hybrid composition by default on Android (#20722) (flutter/engine#20864) * 9398717 Roll Skia from db5582b71116 to 44e96bee4b6a (4 revisions) (flutter/engine#20908) * 5f49a95 Add auto plugin registration to FlutterFragmentActivity as well (flutter/engine#20865) * c4c4f34 Wait for first frame before checking layer tree (flutter/engine#20910) * 0773bf0 Roll Skia from 44e96bee4b6a to 3913d3e137ed (2 revisions) (flutter/engine#20909) * 8ec8af7 [windows] Add horizontal scroll support (flutter/engine#20668) * 1bd9b8e Reset Shell::weak_factory_gpu_ on the raster thread (flutter/engine#20869) * d67923f Pass text input key events to the EventResponder if they do not yield characters (flutter/engine#20912) * abe10d1 Roll Dart SDK from 84c3eacc7ba0 to 6eab35f49cbb (2 revisions) (flutter/engine#20913) * 101316b [web] migrate from e2e to integration_test (flutter/engine#20914) * 1f52ec3 Roll Dart SDK from 6eab35f49cbb to 2a5f37d25453 (1 revision) (flutter/engine#20917) * 8019058 Default C++ wrapper templates to EncodableValue (flutter/engine#20760) * 5f3ec38 Roll Fuchsia Mac SDK from sI2DAAmSI... to waj2pOhDh... (flutter/engine#20919) * a651020 Roll Fuchsia Linux SDK from _SVZn8uN2... to 9tLNFbjA1... (flutter/engine#20920) * 696c2aa Roll Skia from 3913d3e137ed to 7b46300fe4ff (4 revisions) (flutter/engine#20924) * 95f2b72 Create root isolate asynchronously (flutter/engine#20142) * 58a6207 Adds fuchsia node roles to accessibility bridge updates. (flutter/engine#20385) * a762143 Roll Dart SDK from 2a5f37d25453 to e8e0d5a539fb (3 revisions) (flutter/engine#20928) * 49d6805 Ensure all images are closed in FlutterImageView (flutter/engine#20842) * d67bda7 Image.toByteData and Picture.toImage implementations (#3) (flutter/engine#20750) * 96efe39 Revert "Adds fuchsia node roles to accessibility bridge updates. (#20385)" (flutter/engine#20936) * 5585ed9 Revert "Create root isolate asynchronously (#20142)" (flutter/engine#20937) * f6270c0 Roll Dart SDK from e8e0d5a539fb to b29f228f62e2 (3 revisions) (flutter/engine#20939) * 15bf1bb [Android R] Integrate DisplayCutouts into viewportMetrics (flutter/engine#20921) * 615e668 Clear the GL context only after submitting the frame (flutter/engine#20931) * ca989b8 Roll Skia from 7b46300fe4ff to 1338a37a1add (16 revisions) (flutter/engine#20943) * 8f3f711 Roll Fuchsia Linux SDK from 9tLNFbjA1... to knpSoAoZq... (flutter/engine#20944) * 873c007 Log exception in addition to the stack trace for unhandled exceptions. (flutter/engine#20935) * d761629 Roll Skia from 1338a37a1add to 8fa3b4e8cde5 (6 revisions) (flutter/engine#20949) * f6920da Roll Skia from 8fa3b4e8cde5 to e9a9ad908226 (5 revisions) (flutter/engine#20952) * 634e499 Use hint freed specifically for image disposal (flutter/engine#20754) * c700479 Revert external size changes to Picture (flutter/engine#20950) * 4353797 Roll Skia from e9a9ad908226 to 3d1d636cd839 (6 revisions) (flutter/engine#20955) * 80f4481 renaming e2e tests to integration (flutter/engine#20954) * 61e057a Clear GL context before Gr context (flutter/engine#20957) * f43c3d7 Roll Fuchsia Mac SDK from waj2pOhDh... to 0r88gDzUP... (flutter/engine#20958) * 5a2db33 Roll Skia from 3d1d636cd839 to 683beccf6776 (13 revisions) (flutter/engine#20961) * efb339f Only clear GL context after changing the thread configuration (flutter/engine#20965) * 58d5132 Roll Fuchsia Linux SDK from knpSoAoZq... to odFvFQV9Z... (flutter/engine#20968) * 3729fdb Roll Skia from 683beccf6776 to a66a9c31a318 (4 revisions) (flutter/engine#20969) * 40fe7f3 Roll Fuchsia Mac SDK from 0r88gDzUP... to gOI3W1UNU... (flutter/engine#20970) * e979c29 Roll Skia from a66a9c31a318 to be72801f29f9 (1 revision) (flutter/engine#20971) * 6e8930b Roll Skia from be72801f29f9 to f8823b572600 (1 revision) (flutter/engine#20973) * 68b7b84 [fuchsia] Send trace events to system tracing on all configurations (flutter/engine#20974) * 3f05b52 Always set the callback during Rasterizer setup (flutter/engine#20976)
* 5e54c70 Reland: Enable hybrid composition by default on Android (flutter#20722) (flutter/engine#20864) * 9398717 Roll Skia from db5582b71116 to 44e96bee4b6a (4 revisions) (flutter/engine#20908) * 5f49a95 Add auto plugin registration to FlutterFragmentActivity as well (flutter/engine#20865) * c4c4f34 Wait for first frame before checking layer tree (flutter/engine#20910) * 0773bf0 Roll Skia from 44e96bee4b6a to 3913d3e137ed (2 revisions) (flutter/engine#20909) * 8ec8af7 [windows] Add horizontal scroll support (flutter/engine#20668) * 1bd9b8e Reset Shell::weak_factory_gpu_ on the raster thread (flutter/engine#20869) * d67923f Pass text input key events to the EventResponder if they do not yield characters (flutter/engine#20912) * abe10d1 Roll Dart SDK from 84c3eacc7ba0 to 6eab35f49cbb (2 revisions) (flutter/engine#20913) * 101316b [web] migrate from e2e to integration_test (flutter/engine#20914) * 1f52ec3 Roll Dart SDK from 6eab35f49cbb to 2a5f37d25453 (1 revision) (flutter/engine#20917) * 8019058 Default C++ wrapper templates to EncodableValue (flutter/engine#20760) * 5f3ec38 Roll Fuchsia Mac SDK from sI2DAAmSI... to waj2pOhDh... (flutter/engine#20919) * a651020 Roll Fuchsia Linux SDK from _SVZn8uN2... to 9tLNFbjA1... (flutter/engine#20920) * 696c2aa Roll Skia from 3913d3e137ed to 7b46300fe4ff (4 revisions) (flutter/engine#20924) * 95f2b72 Create root isolate asynchronously (flutter/engine#20142) * 58a6207 Adds fuchsia node roles to accessibility bridge updates. (flutter/engine#20385) * a762143 Roll Dart SDK from 2a5f37d25453 to e8e0d5a539fb (3 revisions) (flutter/engine#20928) * 49d6805 Ensure all images are closed in FlutterImageView (flutter/engine#20842) * d67bda7 Image.toByteData and Picture.toImage implementations (flutter#3) (flutter/engine#20750) * 96efe39 Revert "Adds fuchsia node roles to accessibility bridge updates. (flutter#20385)" (flutter/engine#20936) * 5585ed9 Revert "Create root isolate asynchronously (flutter#20142)" (flutter/engine#20937) * f6270c0 Roll Dart SDK from e8e0d5a539fb to b29f228f62e2 (3 revisions) (flutter/engine#20939) * 15bf1bb [Android R] Integrate DisplayCutouts into viewportMetrics (flutter/engine#20921) * 615e668 Clear the GL context only after submitting the frame (flutter/engine#20931) * ca989b8 Roll Skia from 7b46300fe4ff to 1338a37a1add (16 revisions) (flutter/engine#20943) * 8f3f711 Roll Fuchsia Linux SDK from 9tLNFbjA1... to knpSoAoZq... (flutter/engine#20944) * 873c007 Log exception in addition to the stack trace for unhandled exceptions. (flutter/engine#20935) * d761629 Roll Skia from 1338a37a1add to 8fa3b4e8cde5 (6 revisions) (flutter/engine#20949) * f6920da Roll Skia from 8fa3b4e8cde5 to e9a9ad908226 (5 revisions) (flutter/engine#20952) * 634e499 Use hint freed specifically for image disposal (flutter/engine#20754) * c700479 Revert external size changes to Picture (flutter/engine#20950) * 4353797 Roll Skia from e9a9ad908226 to 3d1d636cd839 (6 revisions) (flutter/engine#20955) * 80f4481 renaming e2e tests to integration (flutter/engine#20954) * 61e057a Clear GL context before Gr context (flutter/engine#20957) * f43c3d7 Roll Fuchsia Mac SDK from waj2pOhDh... to 0r88gDzUP... (flutter/engine#20958) * 5a2db33 Roll Skia from 3d1d636cd839 to 683beccf6776 (13 revisions) (flutter/engine#20961) * efb339f Only clear GL context after changing the thread configuration (flutter/engine#20965) * 58d5132 Roll Fuchsia Linux SDK from knpSoAoZq... to odFvFQV9Z... (flutter/engine#20968) * 3729fdb Roll Skia from 683beccf6776 to a66a9c31a318 (4 revisions) (flutter/engine#20969) * 40fe7f3 Roll Fuchsia Mac SDK from 0r88gDzUP... to gOI3W1UNU... (flutter/engine#20970) * e979c29 Roll Skia from a66a9c31a318 to be72801f29f9 (1 revision) (flutter/engine#20971)
* 5e54c70 Reland: Enable hybrid composition by default on Android (flutter#20722) (flutter/engine#20864) * 9398717 Roll Skia from db5582b71116 to 44e96bee4b6a (4 revisions) (flutter/engine#20908) * 5f49a95 Add auto plugin registration to FlutterFragmentActivity as well (flutter/engine#20865) * c4c4f34 Wait for first frame before checking layer tree (flutter/engine#20910) * 0773bf0 Roll Skia from 44e96bee4b6a to 3913d3e137ed (2 revisions) (flutter/engine#20909) * 8ec8af7 [windows] Add horizontal scroll support (flutter/engine#20668) * 1bd9b8e Reset Shell::weak_factory_gpu_ on the raster thread (flutter/engine#20869) * d67923f Pass text input key events to the EventResponder if they do not yield characters (flutter/engine#20912) * abe10d1 Roll Dart SDK from 84c3eacc7ba0 to 6eab35f49cbb (2 revisions) (flutter/engine#20913) * 101316b [web] migrate from e2e to integration_test (flutter/engine#20914) * 1f52ec3 Roll Dart SDK from 6eab35f49cbb to 2a5f37d25453 (1 revision) (flutter/engine#20917) * 8019058 Default C++ wrapper templates to EncodableValue (flutter/engine#20760) * 5f3ec38 Roll Fuchsia Mac SDK from sI2DAAmSI... to waj2pOhDh... (flutter/engine#20919) * a651020 Roll Fuchsia Linux SDK from _SVZn8uN2... to 9tLNFbjA1... (flutter/engine#20920) * 696c2aa Roll Skia from 3913d3e137ed to 7b46300fe4ff (4 revisions) (flutter/engine#20924) * 95f2b72 Create root isolate asynchronously (flutter/engine#20142) * 58a6207 Adds fuchsia node roles to accessibility bridge updates. (flutter/engine#20385) * a762143 Roll Dart SDK from 2a5f37d25453 to e8e0d5a539fb (3 revisions) (flutter/engine#20928) * 49d6805 Ensure all images are closed in FlutterImageView (flutter/engine#20842) * d67bda7 Image.toByteData and Picture.toImage implementations (flutter#3) (flutter/engine#20750) * 96efe39 Revert "Adds fuchsia node roles to accessibility bridge updates. (flutter#20385)" (flutter/engine#20936) * 5585ed9 Revert "Create root isolate asynchronously (flutter#20142)" (flutter/engine#20937) * f6270c0 Roll Dart SDK from e8e0d5a539fb to b29f228f62e2 (3 revisions) (flutter/engine#20939) * 15bf1bb [Android R] Integrate DisplayCutouts into viewportMetrics (flutter/engine#20921) * 615e668 Clear the GL context only after submitting the frame (flutter/engine#20931) * ca989b8 Roll Skia from 7b46300fe4ff to 1338a37a1add (16 revisions) (flutter/engine#20943) * 8f3f711 Roll Fuchsia Linux SDK from 9tLNFbjA1... to knpSoAoZq... (flutter/engine#20944) * 873c007 Log exception in addition to the stack trace for unhandled exceptions. (flutter/engine#20935) * d761629 Roll Skia from 1338a37a1add to 8fa3b4e8cde5 (6 revisions) (flutter/engine#20949) * f6920da Roll Skia from 8fa3b4e8cde5 to e9a9ad908226 (5 revisions) (flutter/engine#20952) * 634e499 Use hint freed specifically for image disposal (flutter/engine#20754) * c700479 Revert external size changes to Picture (flutter/engine#20950) * 4353797 Roll Skia from e9a9ad908226 to 3d1d636cd839 (6 revisions) (flutter/engine#20955) * 80f4481 renaming e2e tests to integration (flutter/engine#20954) * 61e057a Clear GL context before Gr context (flutter/engine#20957) * f43c3d7 Roll Fuchsia Mac SDK from waj2pOhDh... to 0r88gDzUP... (flutter/engine#20958) * 5a2db33 Roll Skia from 3d1d636cd839 to 683beccf6776 (13 revisions) (flutter/engine#20961) * efb339f Only clear GL context after changing the thread configuration (flutter/engine#20965) * 58d5132 Roll Fuchsia Linux SDK from knpSoAoZq... to odFvFQV9Z... (flutter/engine#20968) * 3729fdb Roll Skia from 683beccf6776 to a66a9c31a318 (4 revisions) (flutter/engine#20969) * 40fe7f3 Roll Fuchsia Mac SDK from 0r88gDzUP... to gOI3W1UNU... (flutter/engine#20970) * e979c29 Roll Skia from a66a9c31a318 to be72801f29f9 (1 revision) (flutter/engine#20971) * 6e8930b Roll Skia from be72801f29f9 to f8823b572600 (1 revision) (flutter/engine#20973) * 68b7b84 [fuchsia] Send trace events to system tracing on all configurations (flutter/engine#20974) * 3f05b52 Always set the callback during Rasterizer setup (flutter/engine#20976)
…es. (flutter#20385)" (flutter#20936)" This reverts commit 96efe39.
Description
This change adds a semantics flag to mark slider UI elements, and modifies the fuchsia accessibility bridge to populate the fuchsia node role.
Related Issues
N/A
Tests
I added the following tests:
Accessibility bridge unit test
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.