Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,15 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
}

UIView* FlutterPlatformViewsController::GetPlatformViewByID(int64_t view_id) {
return [GetFlutterTouchInterceptingViewByID(view_id) embeddedView];
}

FlutterTouchInterceptingView* FlutterPlatformViewsController::GetFlutterTouchInterceptingViewByID(
int64_t view_id) {
if (views_.empty()) {
return nil;
}
return [touch_interceptors_[view_id].get() embeddedView];
return touch_interceptors_[view_id].get();
}

long FlutterPlatformViewsController::FindFirstResponderPlatformViewId() {
Expand Down Expand Up @@ -957,6 +962,7 @@ @implementation FlutterTouchInterceptingView {
fml::scoped_nsobject<DelayingGestureRecognizer> _delayingRecognizer;
FlutterPlatformViewGestureRecognizersBlockingPolicy _blockingPolicy;
UIView* _embeddedView;
NSObject* _flutterAccessibilityContainer;
Comment thread
stuartmorgan-g marked this conversation as resolved.
}
- (instancetype)initWithEmbeddedView:(UIView*)embeddedView
platformViewsController:
Expand Down Expand Up @@ -1035,6 +1041,14 @@ - (void)touchesCancelled:(NSSet<UITouch*>*)touches withEvent:(UIEvent*)event {
- (void)touchesEnded:(NSSet*)touches withEvent:(UIEvent*)event {
}

- (void)setFlutterAccessibilityContainer:(NSObject*)flutterAccessibilityContainer {
Comment thread
stuartmorgan-g marked this conversation as resolved.
_flutterAccessibilityContainer = flutterAccessibilityContainer;
}

- (id)accessibilityContainer {
return _flutterAccessibilityContainer;
}

@end

@implementation DelayingGestureRecognizer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
static FlutterPlatformViewsTestMockPlatformView* gMockPlatformView = nil;
const float kFloatCompareEpsilon = 0.001;

@interface FlutterTouchInterceptingView (Test)
- (id)accessibilityContainer;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a _Test.h file instead of re-declaring methods directly in test files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire redeclaration block should be removed in favor of including the test header.

@end

@interface FlutterPlatformViewsTestMockPlatformView : UIView
@end
@implementation FlutterPlatformViewsTestMockPlatformView
Expand Down Expand Up @@ -3099,4 +3103,12 @@ - (void)testOnlyPlatformViewsAreRemovedWhenReset {
XCTAssertEqual(mockFlutterView.subviews.firstObject, someView);
}

- (void)testFlutterTouchInterceptingViewLinksToAccessibilityContainer {
FlutterTouchInterceptingView* touchInteceptorView =
[[[FlutterTouchInterceptingView alloc] init] autorelease];
NSObject* container = [[[NSObject alloc] init] autorelease];
[touchInteceptorView setFlutterAccessibilityContainer:container];
XCTAssertEqualObjects([touchInteceptorView accessibilityContainer], container);
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h"
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterPlatformViews.h"
#import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterPlugin.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h"
#import "flutter/shell/platform/darwin/ios/ios_context.h"

@class FlutterTouchInterceptingView;
Expand Down Expand Up @@ -234,6 +235,8 @@ class FlutterPlatformViewsController {
// returns nil.
UIView* GetPlatformViewByID(int64_t view_id);

FlutterTouchInterceptingView* GetFlutterTouchInterceptingViewByID(int64_t);
Comment thread
stuartmorgan-g marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I missed that this parameter doesn't have a name; it should be int64_t view_id.


PostPrerollResult PostPrerollAction(
const fml::RefPtr<fml::RasterThreadMerger>& raster_thread_merger);

Expand Down Expand Up @@ -409,7 +412,7 @@ class FlutterPlatformViewsController {
// This view has 2 roles:
// 1. Delay or prevent touch events from arriving the embedded view.
// 2. Dispatching all events that are hittested to the embedded view to the FlutterView.
@interface FlutterTouchInterceptingView : UIView
@interface FlutterTouchInterceptingView : UIView <FlutterSemanticsProtocol>
- (instancetype)initWithEmbeddedView:(UIView*)embeddedView
platformViewsController:
(fml::WeakPtr<flutter::FlutterPlatformViewsController>)platformViewsController
Expand All @@ -424,6 +427,7 @@ class FlutterPlatformViewsController {

// Get embedded view
- (UIView*)embeddedView;

@end

@interface UIView (FirstResponder)
Expand Down
11 changes: 10 additions & 1 deletion shell/platform/darwin/ios/framework/Source/SemanticsObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ constexpr float kScrollExtentMaxForInf = 1000;
@class FlutterCustomAccessibilityAction;
@class FlutterPlatformViewSemanticsContainer;

@protocol FlutterSemanticsProtocol <NSObject>
// Adding functionalities for any object to assign accessibilityContainer.
// For example, this allows the PlatformView be able to link back to the accessibility tree.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by what this property is actually for. I see where it's set, but I don't see anything reading it except in tests. Am I missing some non-test behavior that's influenced by this?

@cyanglaz cyanglaz Oct 2, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is read in FlutterTouchInterceptingView: accessibilityContainer, it is a UIAccessibilty property that is read by the UIKit.

I initially added the method to the FlutterTouchInterceptingView so that FlutterTouchInterceptingView can access the container in its accessibiltiyContainer method.

Then I discovered that SemanticsObjectTests is ARC, so we couldn't not include FlutterTouchInterceptingView (which uses MRC features) in the header of SemanticsObjectTests, thus the protocl was added.

This does make the code more confusing. Maybe I should revert back to my initial implementation (no protocol, FlutterTouchInterceptingView has a setFlutterAccessiblityContainer method), then move the related tests to mrc.

// See: https://github.com/flutter/flutter/issues/135504
- (void)setFlutterAccessibilityContainer:(NSObject*)flutterAccessibilityContainer;

@end

/**
* A node in the iOS semantics tree. This object is a wrapper over a native accessibiliy
* object, which is stored in the property `nativeAccessibility`. In the most case, the
Expand Down Expand Up @@ -171,7 +179,8 @@ constexpr float kScrollExtentMaxForInf = 1000;

- (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)bridge
uid:(int32_t)uid
platformView:(UIView*)platformView NS_DESIGNATED_INITIALIZER;
platformView:(UIView<FlutterSemanticsProtocol>*)platformView
NS_DESIGNATED_INITIALIZER;

@end

Expand Down
14 changes: 3 additions & 11 deletions shell/platform/darwin/ios/framework/Source/SemanticsObject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h"

#include "flutter/fml/platform/darwin/scoped_nsobject.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h"

namespace {
Expand Down Expand Up @@ -157,9 +156,7 @@ @interface FlutterScrollableSemanticsObject ()
@property(nonatomic, retain) FlutterSemanticsScrollView* scrollView;
@end

@implementation FlutterScrollableSemanticsObject {
fml::scoped_nsobject<SemanticsObjectContainer> _container;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to the PR, but the variable is unused so I removed it

}
@implementation FlutterScrollableSemanticsObject

- (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)bridge
uid:(int32_t)uid {
Expand Down Expand Up @@ -865,9 +862,10 @@ @implementation FlutterPlatformViewSemanticsContainer

- (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)bridge
uid:(int32_t)uid
platformView:(nonnull UIView*)platformView {
platformView:(UIView<FlutterSemanticsProtocol>*)platformView {
if (self = [super initWithBridge:bridge uid:uid]) {
_platformView = [platformView retain];
[platformView setFlutterAccessibilityContainer:self];
}
return self;
}
Expand All @@ -882,12 +880,6 @@ - (id)nativeAccessibility {
return _platformView;
}

#pragma mark - UIAccessibilityContainer overrides

- (NSArray*)accessibilityElements {
return @[ _platformView ];
}
Comment on lines -887 to -889

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We implemented nativeAccessibility, this is not necessary.


@end

@implementation SemanticsObjectContainer {
Expand Down
29 changes: 25 additions & 4 deletions shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,26 @@ void AccessibilityObjectDidLoseFocus(int32_t id) override {}
} // namespace
} // namespace flutter

// A mock platform view for testing
@interface TestPlatformView : UIView <FlutterSemanticsProtocol>

@property(weak, nonatomic) NSObject* container;
- (id)accessibilityContainer;

@end

@implementation TestPlatformView

- (void)setFlutterAccessibilityContainer:(NSObject*)flutterAccessibilityContainer {
self.container = flutterAccessibilityContainer;
}

- (id)accessibilityContainer {
return self.container;
}

@end

@interface SemanticsObjectTest : XCTestCase
@end

Expand Down Expand Up @@ -210,7 +230,8 @@ - (void)testAccessibilityHitTestSearchCanReturnPlatformView {
SemanticsObject* object0 = [[SemanticsObject alloc] initWithBridge:bridge uid:0];
SemanticsObject* object1 = [[SemanticsObject alloc] initWithBridge:bridge uid:1];
SemanticsObject* object3 = [[SemanticsObject alloc] initWithBridge:bridge uid:3];
UIView* platformView = [[UIView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)];
TestPlatformView* platformView =
[[TestPlatformView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)];
FlutterPlatformViewSemanticsContainer* platformViewSemanticsContainer =
[[FlutterPlatformViewSemanticsContainer alloc] initWithBridge:bridge
uid:1
Expand Down Expand Up @@ -901,15 +922,15 @@ - (void)testFlutterPlatformViewSemanticsContainer {
fml::WeakPtrFactory<flutter::MockAccessibilityBridge> factory(
new flutter::MockAccessibilityBridge());
fml::WeakPtr<flutter::MockAccessibilityBridge> bridge = factory.GetWeakPtr();
__weak UIView* weakPlatformView;
__weak TestPlatformView* weakPlatformView;
@autoreleasepool {
UIView* platformView = [[UIView alloc] init];
TestPlatformView* platformView = [[TestPlatformView alloc] init];

FlutterPlatformViewSemanticsContainer* container =
[[FlutterPlatformViewSemanticsContainer alloc] initWithBridge:bridge
uid:1
platformView:platformView];
XCTAssertEqualObjects(container.accessibilityElements, @[ platformView ]);
XCTAssertEqualObjects(platformView.accessibilityContainer, container);
weakPlatformView = platformView;
XCTAssertNotNil(weakPlatformView);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ static void ReplaceSemanticsObject(SemanticsObject* oldObject,
return [[[FlutterPlatformViewSemanticsContainer alloc]
initWithBridge:weak_ptr
uid:node.id
platformView:weak_ptr->GetPlatformViewsController()->GetPlatformViewByID(
platformView:weak_ptr->GetPlatformViewsController()->GetFlutterTouchInterceptingViewByID(
node.platformViewId)] autorelease];
} else {
return [[[FlutterSemanticsObject alloc] initWithBridge:weak_ptr uid:node.id] autorelease];
Expand Down