Enables semantics when voice control is turned on

[chunhtai]

Added a catch all workaround to make sure semantics is enabled.
flutter/flutter#75887

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.
• The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[gaaclarke]

This in itself doesn't fix any issue right? There should be an update to FlutterEngine who implements FlutterViewEngineDelegate

[gaaclarke]

I'd add a docstring since it isn't clear what this method is used for.

[chunhtai]

This in itself doesn't fix any issue right? There should be an update to FlutterEngine who implements FlutterViewEngineDelegate

Without this patch, the voice control will not work for flutter application, because flutter can't detect whether voice control is turned on or off. If it can't detect, it won't enable the semantics

iOS does not provide a public API to listen to the notification for voice control either, that is why I have to use this workaround to catch all...

Not sure about what update is needed to FlutterEngine, the ensureSemanticsEnabled is an existing selector defined in header/FlutterEngine.h, I am not sure if this is a good practice, should i define a different selector?

[gaaclarke]

@chunhtai Right, I'm just saying there is more to come, right? You have added a method to a protocol, but you didn't implement that method. Maybe you forgot to push a change to the FlutterEngine?

[chunhtai]

@chunhtai Right, I'm just saying there is more to come, right? You have added a method to a protocol, but you didn't implement that method. Maybe you forgot to push a change to the FlutterEngine?

The method i added to the protocol matches an existing method in FlutterEngine,

engine/shell/platform/darwin/ios/framework/Headers/FlutterEngine.h

Line 264 in e9d0b4a

- (void)ensureSemanticsEnabled;

[gaaclarke]

Ahh ok. Here's some food for thought. In Apple's API's delegates are typically called with a pointer to the class like this:

@class Foo;
@protocol FooDelegate
-(void)somethingHappened:(Foo*)foo;
@end

I think you are leaking FlutterEngine implementation details into the FlutterView. The protocol is meant to be an abstraction, not just a limited view of the FlutterEngine. I think you'd be better off calling it something generic instead of calling it something specific to FlutterEngine such that you'll end up with:

@implementation FlutterView
- (BOOL)isAccessibilityElement {
  if (self.accessibilityElements == nil) {
    [_delegate flutterViewDidReceiveAccessibilityPing:self];
  }
  return NO;
}
@end

@implementation FlutterEngine
- (BOOL)flutterViewDidReceiveAccessibilityPing:(FlutterView*)flutterView {
  [self ensureSemanticsEnabled];
}
@end

This way it's much more clear about the relationship between the FlutterView and the FlutterEngine and mocking out the delegate makes more sense since the abstraction is talking about what it is notifying about, not what to do about it.

[chunhtai]

Thanks @gaaclarke this is ready for another look

[gaaclarke]

LGTM once you fix the test

[gaaclarke]

This method has to be updated, this test should be failing.

[chunhtai]

No I inherited from the FlutterEngine so I only need to override ensureSemanticsEnabled. I want to test the FlutterView.isAccessibilityElement call eventually lead to ensureSemanticsEnabled

[chunhtai]

otheriwse, I will need to write two tests.
one for FlutterView.isAccessibilityElement calls FlutterViewEngineDelegate.flutterViewAccessibilityDidCall

the other one for FlutterEngine.flutterViewAccessibilityDidCall calls FlutterEngine.ensureSemanticsEnabled

[gaaclarke]

Ahh yea, sorry missed that. I think you should use a mock object to keep the test hermetic. Now you're tying your testing of functionality on FlutterView to implementation details in FlutterEngine which is fragile. It easy to replace it if you use OCMock, there are examples in other tests.