It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@loic-sharma could you review this updated PR ?

@loic-peron-inetum-public Thanks for the pull request!

I'm a little concerned that this change might regress Flutter Windows's accessibility support in unexpected ways as a similar change to add UI Automation had caused unexpected problems.

Could you include details on how you verified this change? What scenarios did you test?

Regarding how we use and tests this proposal:

We are building an application targeting iOS and Windows. We automate black-box tests using Appium on both platform. We use a custom build of the flutter engine for Windows that includes the required PR to get Semantics::identifer to be exposed as AutomationId. The application is not evaluated on its accessibility for actual users.

Regarding UI Automation-related regressions:

I understand that switching to UI Automation required a full switch in the accessibility interface exposed by Flutter. It seems that IAccessibleEx is a way to extend IAccessible-based exposition with the additional capabilities offered by UI Automation, without jeopardizing existing baheviours.

What are the unexpected ways that triggered unexpected problems and regressed Flutter accessibility support during a previous attempt to switch to UI Automation ? How can I check them ?

What are the unexpected ways that triggered unexpected problems and regressed Flutter accessibility support during a previous attempt to switch to UI Automation ? How can I check them ?

I would manually check that a sample Flutter Windows app still works well with JAWS, NVDA, and Windows Narrator. I'd make sure that as you tab around, the screen reader properly announces content, without any differences compared to today's implementation.

Could we de-risk this by making the IAccessibleEx implementation opt-in at runtime?

Flutter Windows's API has a DartProject that lets you configure your Flutter app. For example, you can configure which threads the apps uses (see this and this).

Could we update QueryService to only return an IAccessibleEx interface if the Flutter app turned on the IAccessibleEx feature?

Here's what this API might look like:

// Configures the accessibility implementation used by Flutter.
enum class AccessibilityMode {
  // Default value. Flutter will automatically select the best available
  // implementation.
  Default,
  // Use the IAccessible implementation.
  IAccessible,
  // Use the experimental IAccessibleEx implementation.
  IAccessibleEx,
};

class DartProject {
  // Sets the accessibility implementation used by Flutter.
  void set_accessibility_mode(AccessibilityMode accessibility_mode) {
    accessibility_mode_ = accessibility_mode;
  }
}

To opt-in to the IAccessibilityEx implementation, the app developer would need to modify their windows/runner/main.cpp file:

flutter::DartProject project(L"data");
project.set_accessibility_mode(flutter::AccessibilityMode::IAccessibleEx);

Then, QueryService would only return an IAccessibleEx interface if the app opted-in to AccessibilityMode::IAccessibleEx. What do you think of this approach?

Note to self: these docs are an excellent resource to review this PR: https://learn.microsoft.com/en-us/windows/win32/winauto/implementing-iaccessibleex-for-providers

Could we de-risk this by making the IAccessibleEx implementation opt-in at runtime?

I will try and implement an engine-runtime configuration option in november.

Could this be enabled using an application-buildtime --dart-define= property ?

Could this be enabled using an application-buildtime --dart-define= property ?

Sadly, no. The Windows embedder doesn't have an easy way to read Dart defines. The suggestion I left above means that the app developer will need to update the C++ code in their Flutter Windows entry point (example).

Is progress still being made on this?

Is progress still being made on this?

Hi, yes I still work on this, but I have little work-time currently allocated for this tack. I still work to map the way from DartProject to AXPlatformNodeWin at runtime.

Would it be acceptable to use a technical attribute in AXNodeData to represent IAccessibleEx activation on Windows ?

Would it be acceptable to use a technical attribute in AXNodeData to represent IAccessibleEx activation on Windows ?

Could you say more about why you need this?

I think it's OK to add more data to AXNodeData, but I'll defer to @chunhtai on that.

Would it be acceptable to use a technical attribute in AXNodeData to represent IAccessibleEx activation on Windows ?

may I know what property you want to add? also is IAccessibleEx activation per node? if this is on/off for the entire app, I don't think we should use AXNodeData for this case.

@mattkae Could you do the second review for this PR?

There also doesn't seem to be any test added, should we get a test exemption? cc @loic-sharma since I am not too sure about the windows embedding code

Whoops, good catch. @loic-peron-inetum-public could you add a test for this? I'd consider a test that sends a WM_GETOBJECT message that gets the IAccessibleEx node. These tests might be useful prior art:

Your test can use FlutterDesktopViewGetHWND to get the right HWND and send the WM_GETOBJECT message.

I do not see how I can link a mocked Window (as demonstrated in the first linked test) to a real engine configured for IAccessibleEx (as demonstrated in the second linked test).

I tried and failed to send WM_GETOBJECT to the HWND returned by FlutterDesktopViewGetHWND and convert the returned LRESULT with ObjectFromLresult to an IAccessible as described in Accessibility-on-Windows.md.

Is there any additional element I should be aware of while continuing to try and implement this test ?

I cannot manage to SendMessage :

TEST_F(WindowsTest, EnableIAccessibleEx) {
  auto& context = GetContext();
  WindowsConfigBuilder builder(context);
  builder.SetAccessibilityMode(FlutterDesktopAccessibilityMode::IAccessibleExMode);
  ViewControllerPtr controller{builder.Run()};
  ASSERT_NE(controller, nullptr);
  auto view = FlutterDesktopViewControllerGetView(controller.get());
  ASSERT_NE(view, nullptr);
  HWND hwnd = FlutterDesktopViewGetHWND(view);
  ASSERT_NE(hwnd, nullptr);
  LRESULT lres = SendMessage(hwnd, WM_GETOBJECT, 0, OBJID_CLIENT);
  ASSERT_NE(lres, 0);
}

fails on ASSERT_NE(lres, 0)

@loic-sharma can you help me find what am I obviously missing ?

@loic-peron-inetum-public FYI, you can use Visual Studio to debug the unit test.

WM_GETOBJECT returns an LRESULT of 0 because the app does not have an accessibility tree yet. I'd recommend updating the test such that it:

1. In flutter\engine\src\flutter\shell\platform\windows\fixtures\main.dart, add a new Dart test app that:
   1. Waits until semantics is enabled
   2. Sends a semantics tree
   3. Sends a signal to the test
2. In flutter\engine\src\flutter\shell\platform\windows\flutter_windows_unittests.cc:
   1. Use WindowsConfigBuilder.SetDartEntrypoint to launch your new Dart test app
   2. Use WindowsTestContext::AddNativeFunction to listen from signals from the Dart app
   3. Pumps win32 messages until the app sends the signal
   4. Calls WM_GETOBJECT and checks the result.

Here's a Dart test app that sends a semantic tree:

Here's how you can pump win32 messages until the app sends a semantics tree:

Please let me know if you have questions!