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 all commits
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
19 changes: 6 additions & 13 deletions shell/platform/darwin/ios/framework/Source/SemanticsObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,6 @@ constexpr float kScrollExtentMaxForInf = 1000;
*/
@property(nonatomic, strong) NSArray<SemanticsObject*>* children;

/**
* Used if this SemanticsObject is for a platform view.
*/
@property(strong, nonatomic) FlutterPlatformViewSemanticsContainer* platformViewSemanticsContainer;

/**
* The UIAccessibility that represents this object.
*
Expand Down Expand Up @@ -159,16 +154,14 @@ constexpr float kScrollExtentMaxForInf = 1000;
* * `SemanticsObject` for the other type of semantics objects.
* * `FlutterSemanticsObject` for default implementation of `SemanticsObject`.
*/
@interface FlutterPlatformViewSemanticsContainer : UIAccessibilityElement

/**
* The position inside an accessibility container.
*/
@property(nonatomic) NSInteger index;
@interface FlutterPlatformViewSemanticsContainer : SemanticsObject

- (instancetype)init __attribute__((unavailable("Use initWithAccessibilityContainer: instead")));
- (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)bridge
uid:(int32_t)uid NS_UNAVAILABLE;

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

@end

Expand Down
83 changes: 7 additions & 76 deletions shell/platform/darwin/ios/framework/Source/SemanticsObject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ - (void)dealloc {
[_children release];
_parent = nil;
_container.get().semanticsObject = nil;
[_platformViewSemanticsContainer release];
_inDealloc = YES;
[super dealloc];
}
Expand All @@ -319,9 +318,6 @@ - (void)setChildren:(NSArray<SemanticsObject*>*)children {
}

- (BOOL)hasChildren {
if (_node.IsPlatformViewNode()) {
return YES;
}
return [self.children count] != 0;
}

Expand Down Expand Up @@ -753,31 +749,16 @@ - (UIAccessibilityTraits)accessibilityTraits {
@end

@interface FlutterPlatformViewSemanticsContainer ()
@property(nonatomic, assign) SemanticsObject* semanticsObject;
@property(nonatomic, strong) UIView* platformView;
@end

@implementation FlutterPlatformViewSemanticsContainer

// Method declared as unavailable in the interface
- (instancetype)init {
[self release];
[super doesNotRecognizeSelector:_cmd];
return nil;
}

- (instancetype)initWithSemanticsObject:(SemanticsObject*)object {
FML_CHECK(object);
// Initialize with the UIView as the container.
// The UIView will not necessarily be accessibility parent for this object.
// The bridge informs the OS of the actual structure via
// `accessibilityContainer` and `accessibilityElementAtIndex`.
if (self = [super initWithAccessibilityContainer:object.bridge->view()]) {
_semanticsObject = object;
auto controller = object.bridge->GetPlatformViewsController();
if (controller) {
_platformView = [controller->GetPlatformViewByID(object.node.platformViewId) retain];
}
- (instancetype)initWithBridge:(fml::WeakPtr<flutter::AccessibilityBridgeIos>)bridge
uid:(int32_t)uid
platformView:(nonnull UIView*)platformView {
if (self = [super initWithBridge:bridge uid:uid]) {
_platformView = [platformView retain];
}
return self;
}
Expand All @@ -790,47 +771,8 @@ - (void)dealloc {

#pragma mark - UIAccessibilityContainer overrides

- (NSInteger)accessibilityElementCount {
// This container should only contain 2 elements:
// 1. The semantic object that represents this container.
// 2. The platform view object.
return 2;
}

- (nullable id)accessibilityElementAtIndex:(NSInteger)index {
FML_DCHECK(index < 2);
if (index == 0) {
return _semanticsObject.nativeAccessibility;
} else {
return _platformView;
}
}

- (NSInteger)indexOfAccessibilityElement:(id)element {
FML_DCHECK(element == _semanticsObject || element == _platformView);
if (element == _semanticsObject) {
return 0;
} else {
return 1;
}
}

#pragma mark - UIAccessibilityElement overrides

- (CGRect)accessibilityFrame {
return _semanticsObject.accessibilityFrame;
}

- (BOOL)isAccessibilityElement {
return NO;
}

- (id)accessibilityContainer {
return [_semanticsObject accessibilityContainer];
}

- (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
return [_platformView accessibilityScroll:direction];
- (NSArray*)accessibilityElements {
return @[ _platformView ];
Copy link
Contributor

Choose a reason for hiding this comment

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

you can override the nativeAccessibility prop gettter to return the _platformView

}

@end
Expand Down Expand Up @@ -882,12 +824,6 @@ - (nullable id)accessibilityElementAtIndex:(NSInteger)index {

SemanticsObject* child = [_semanticsObject children][index - 1];

// Swap the original `SemanticsObject` to a `PlatformViewSemanticsContainer`
Copy link
Member

Choose a reason for hiding this comment

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

Why was this pattern used in the first place? #8156

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really remember a particular reason the old pattern was used. I don't think the new pattern was ever considered when I first implemented this.

if (child.node.IsPlatformViewNode()) {
child.platformViewSemanticsContainer.index = index;
return child.platformViewSemanticsContainer;
}

if ([child hasChildren])
return [child accessibilityContainer];
return child.nativeAccessibility;
Expand All @@ -897,11 +833,6 @@ - (NSInteger)indexOfAccessibilityElement:(id)element {
if (element == _semanticsObject)
return 0;

// FlutterPlatformViewSemanticsContainer is always the last element of its parent.
if ([element isKindOfClass:[FlutterPlatformViewSemanticsContainer class]]) {
return ((FlutterPlatformViewSemanticsContainer*)element).index;
}
Comment on lines -901 to -903
Copy link
Member

Choose a reason for hiding this comment

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

Nice, I found this pretty confusing when I was stepping through, part of the bug seemed to be that the index was set by one container, then accessed by another?
Can you update the description to explain exactly what the bug was, for posterity?


NSArray<SemanticsObject*>* children = [_semanticsObject children];
for (size_t i = 0; i < [children count]; i++) {
SemanticsObject* child = children[i];
Expand Down
23 changes: 16 additions & 7 deletions shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -690,16 +690,25 @@ - (void)testShouldDispatchShowOnScreenActionForHidden {
XCTAssertTrue(bridge->observations[0].action == flutter::SemanticsAction::kShowOnScreen);
}

- (void)testSemanticsObjectAndPlatformViewSemanticsContainerDontHaveRetainCycle {
- (void)testFlutterPlatformViewSemanticsContainer {
fml::WeakPtrFactory<flutter::MockAccessibilityBridge> factory(
new flutter::MockAccessibilityBridge());
fml::WeakPtr<flutter::MockAccessibilityBridge> bridge = factory.GetWeakPtr();
SemanticsObject* object = [[SemanticsObject alloc] initWithBridge:bridge uid:1];
object.platformViewSemanticsContainer =
[[FlutterPlatformViewSemanticsContainer alloc] initWithSemanticsObject:object];
__weak SemanticsObject* weakObject = object;
object = nil;
XCTAssertNil(weakObject);
__weak UIView* weakPlatformView;
@autoreleasepool {
UIView* platformView = [[UIView alloc] init];

FlutterPlatformViewSemanticsContainer* container =
[[FlutterPlatformViewSemanticsContainer alloc] initWithBridge:bridge
uid:1
platformView:platformView];
XCTAssertEqualObjects(container.accessibilityElements, @[ platformView ]);
weakPlatformView = platformView;
XCTAssertNotNil(weakPlatformView);
}
// Check if there's no more strong references to `platformView` after container and platformView
// are released.
XCTAssertNil(weakPlatformView);
}

- (void)testFlutterSwitchSemanticsObjectMatchesUISwitch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,6 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
object.accessibilityCustomActions = accessibilityCustomActions;
}

if (object.node.IsPlatformViewNode()) {
auto controller = GetPlatformViewsController();
if (controller) {
object.platformViewSemanticsContainer = [[[FlutterPlatformViewSemanticsContainer alloc]
initWithSemanticsObject:object] autorelease];
}
} else if (object.platformViewSemanticsContainer) {
object.platformViewSemanticsContainer = nil;
}
if (needsAnnouncement) {
// Try to be more polite - iOS 11+ supports
// UIAccessibilitySpeechAttributeQueueAnnouncement which should avoid
Expand Down Expand Up @@ -268,6 +259,12 @@ static void ReplaceSemanticsObject(SemanticsObject* oldObject,
} else if (node.HasFlag(flutter::SemanticsFlags::kHasImplicitScrolling)) {
return [[[FlutterScrollableSemanticsObject alloc] initWithBridge:weak_ptr
uid:node.id] autorelease];
} else if (node.IsPlatformViewNode()) {
return [[[FlutterPlatformViewSemanticsContainer alloc]
initWithBridge:weak_ptr
uid:node.id
platformView:weak_ptr->GetPlatformViewsController()->GetPlatformViewByID(
node.platformViewId)] autorelease];
} else {
return [[[FlutterSemanticsObject alloc] initWithBridge:weak_ptr uid:node.id] autorelease];
}
Expand Down