Fix for a11y crash on iOS

[dnfield]

Without this change, we're seeing logs like:

[AXRuntimeCommon] This class 'FlutterSemanticsObject' is not a known serializable element and returning it as an accessibility element may lead to crashes
[AXRuntimeCommon] This class 'SemanticsObject' is not a known serializable element and returning it as an accessibility element may lead to crashes
[AXRuntimeCommon] This class 'SemanticsObjectContainer' is not a known serializable element and returning it as an accessibility element may lead to crashes

which is actually leading to the crash reported in flutter/flutter#42018

The documentation around these isn't so great, but it looks like right now we're implicitly implementing these interfaces and iOS wants us to be more explicit. The container object seems like it's ok as well because we're setting isAccessibilityElement to NO, which is what you're supposed to do for containers.

Still need to come up with a good test for this - I need some additional access to reproduce the internal customer's crash locally, which I should have in the near term.

/cc @gaaclarke @jkurtw

/cc @goderbauer @cbracken @jmagman who hopefully know something about why this will or will not work 😆

[gaaclarke]

The thing that is a bit of a hangup for me is making the containers UIAccessibilityElements. I've read through the documentation a bit and I think our UIAccessibilityContainer should just be the FlutterView itself. It should override the accessibilityElements method and return the UIAccessibilityElements we calculated in the last update semantics pass. Making the containers inherit from UIAccessibilityElement seems a bit of a hack to give the containers the secret sauce to make them work in lieu of a UIView.

Reworking that is not a trivial change though probably. With adequate testing I'm willing to accept this with the hopes that we someday make the change I proposed.

[dnfield]

Making the containers inherit from UIAccessibilityElement seems a bit of a hack to give the containers the secret sauce to make them work in lieu of a UIView.

I think we'll still need containers in there - I don't think they can all be direct children of the UIView.

[goderbauer]

Here and everywhere else this is called: I believe not all SemanticsObjects have the view as an accessibility container. There are some intermediate a11y containers that we create. Shouldn't the SemanticsObjects be attached to their container, not the root?

[dnfield]

The problem is we don't know the containers at this point - but it seems to work because we later assign it to the right container when we actually create the container.

[goderbauer]

There should probably at least be a comment here then. And I am wondering how we can assure that it is going to be assigned the right container....

[goderbauer]

We can't just remove all containers and make all of our semantics objects children of the view. The containers give the a11y hierarchy structure (which VoiceOver uses to move a11y focus to the correct next element) and you also need containers to implement scrolling etc.

[goderbauer]

Have you tried that VoiceOver still works as expected? We totally need better automated testing for this :(

[dnfield]

I can't come up with a good integration test for this. The reproduction is: turn on voiceover on an app with a scrollable that goes beyond the extent of the screen and use three fingers to scroll the list.

We don't have any tests that use Voiceover. We should, but we don't. It's a non-trivial task to do so - I've filed an issue for that. I'll see if I can at least make the changes to the a11y bridge more testable...

[dnfield]

I've manually confirmed that a11y still works on iOS with these changes. Writing a test for this will require creating a new test harness for iOS. I'm going to take that up in a follow up PR. For now I'd like to get this merged to unblock the internal customer.

[dnfield]

Framework redness is expected ATM due to manual roll needed.