From e4e7b3242e5aee47edf398fd5a1a0c808cc32320 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Tue, 14 May 2024 09:30:52 -0700 Subject: [PATCH] Refactor Semantics in preparation for ARC migration (flutter/engine#52729) Split the too-large `Semantics` [MRC to ARC migration](https://github.com/flutter/flutter/issues/137801) into two PRs: this one, which refactors, and the next which will actually do the migration. 1. Use properties instead of their backing ivars (except in the usual init, dealloc, getters/setters) 2. For Foundation collections prefer `copy` over `strong`. 3. Dot notation for properties 4. Change `privateSetParent:` to instead use a `readwrite` property in `SemanticsObject ()`. 5. Switch the `semanticsObject` property from `weak` to `retain` to get the synthesized property (keeping it as `weak` is a compilation error in MRC) but I'll swap it back to a `weak` in the ARC migration PR coming next. 6. `SemanticsObjectTest` fails on my machine and passes on CI. Switched the cleaner `CGRectEqualToRect` (and related) checks to instead assert x, y, width, height so we can see the value when it fails: ``` ((CGSizeEqualToSize( scrollView.contentSize, CGSizeMake((w + scrollExtentMax) * effectivelyScale, h * effectivelyScale))) is true) failed ``` becomes: ``` ((scrollView.contentSize.height) equal to (h * effectivelyScale)) failed: ("33.3333333333") is not equal to ("33.333336") ``` Use `XCTAssertEqualWithAccuracy` now that I can see it's a floating point precision issue. --- .../Source/FlutterSemanticsScrollView.mm | 36 +-- .../ios/framework/Source/SemanticsObject.h | 8 +- .../ios/framework/Source/SemanticsObject.mm | 274 +++++++++--------- .../framework/Source/SemanticsObjectTest.mm | 96 ++++-- 4 files changed, 216 insertions(+), 198 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm index 87b9d8b00b5..2a0083a22d1 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm @@ -25,81 +25,81 @@ // UIScrollView class, the base class. - (BOOL)isAccessibilityElement { - if (![_semanticsObject isAccessibilityBridgeAlive]) { + if (![self.semanticsObject isAccessibilityBridgeAlive]) { return NO; } - if ([_semanticsObject isAccessibilityElement]) { + if (self.semanticsObject.isAccessibilityElement) { return YES; } if (self.contentSize.width > self.frame.size.width || self.contentSize.height > self.frame.size.height) { // In SwitchControl or VoiceControl, the isAccessibilityElement must return YES // in order to use scroll actions. - return ![_semanticsObject bridge]->isVoiceOverRunning(); + return ![self.semanticsObject bridge]->isVoiceOverRunning(); } else { return NO; } } - (NSString*)accessibilityLabel { - return [_semanticsObject accessibilityLabel]; + return self.semanticsObject.accessibilityLabel; } - (NSAttributedString*)accessibilityAttributedLabel { - return [_semanticsObject accessibilityAttributedLabel]; + return self.semanticsObject.accessibilityAttributedLabel; } - (NSString*)accessibilityValue { - return [_semanticsObject accessibilityValue]; + return self.semanticsObject.accessibilityValue; } - (NSAttributedString*)accessibilityAttributedValue { - return [_semanticsObject accessibilityAttributedValue]; + return self.semanticsObject.accessibilityAttributedValue; } - (NSString*)accessibilityHint { - return [_semanticsObject accessibilityHint]; + return self.semanticsObject.accessibilityHint; } - (NSAttributedString*)accessibilityAttributedHint { - return [_semanticsObject accessibilityAttributedHint]; + return self.semanticsObject.accessibilityAttributedHint; } - (BOOL)accessibilityActivate { - return [_semanticsObject accessibilityActivate]; + return [self.semanticsObject accessibilityActivate]; } - (void)accessibilityIncrement { - [_semanticsObject accessibilityIncrement]; + [self.semanticsObject accessibilityIncrement]; } - (void)accessibilityDecrement { - [_semanticsObject accessibilityDecrement]; + [self.semanticsObject accessibilityDecrement]; } - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction { - return [_semanticsObject accessibilityScroll:direction]; + return [self.semanticsObject accessibilityScroll:direction]; } - (BOOL)accessibilityPerformEscape { - return [_semanticsObject accessibilityPerformEscape]; + return [self.semanticsObject accessibilityPerformEscape]; } - (void)accessibilityElementDidBecomeFocused { - [_semanticsObject accessibilityElementDidBecomeFocused]; + [self.semanticsObject accessibilityElementDidBecomeFocused]; } - (void)accessibilityElementDidLoseFocus { - [_semanticsObject accessibilityElementDidLoseFocus]; + [self.semanticsObject accessibilityElementDidLoseFocus]; } - (id)accessibilityContainer { - return [_semanticsObject accessibilityContainer]; + return self.semanticsObject.accessibilityContainer; } - (NSInteger)accessibilityElementCount { - return [[_semanticsObject children] count]; + return self.semanticsObject.children.count; } @end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h index 5298d5fabac..792113c5016 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h @@ -38,7 +38,7 @@ constexpr float kScrollExtentMaxForInf = 1000; * The parent of this node in the node tree. Will be nil for the root node and * during transient state changes. */ -@property(nonatomic, assign) SemanticsObject* parent; +@property(nonatomic, assign, readonly) SemanticsObject* parent; /** * The accessibility bridge that this semantics object is attached to. This @@ -64,13 +64,13 @@ constexpr float kScrollExtentMaxForInf = 1000; * Direct children of this semantics object. Each child's `parent` property must * be equal to this object. */ -@property(nonatomic, strong) NSArray* children; +@property(nonatomic, copy) NSArray* children; /** * Direct children of this semantics object in hit test order. Each child's `parent` property * must be equal to this object. */ -@property(nonatomic, strong) NSArray* childrenInHitTestOrder; +@property(nonatomic, copy) NSArray* childrenInHitTestOrder; /** * The UIAccessibility that represents this object. @@ -231,7 +231,7 @@ constexpr float kScrollExtentMaxForInf = 1000; bridge:(fml::WeakPtr)bridge NS_DESIGNATED_INITIALIZER; -@property(nonatomic, weak) SemanticsObject* semanticsObject; +@property(nonatomic, assign) SemanticsObject* semanticsObject; @end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index cb30bc36a65..ef97d929f05 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -4,7 +4,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" @@ -55,11 +54,11 @@ CGPoint ConvertPointToGlobal(SemanticsObject* reference, CGPoint local_point) { // `rect` is in the physical pixel coordinate system. iOS expects the accessibility frame in // the logical pixel coordinate system. Therefore, we divide by the `scale` (pixel ratio) to // convert. - UIScreen* screen = [[[reference bridge]->view() window] screen]; + UIScreen* screen = reference.bridge->view().window.screen; // Screen can be nil if the FlutterView is covered by another native view. - CGFloat scale = screen == nil ? [UIScreen mainScreen].scale : screen.scale; + CGFloat scale = (screen ?: UIScreen.mainScreen).scale; auto result = CGPointMake(point.x() / scale, point.y() / scale); - return [[reference bridge]->view() convertPoint:result toView:nil]; + return [reference.bridge->view() convertPoint:result toView:nil]; } CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { @@ -83,18 +82,18 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { // `rect` is in the physical pixel coordinate system. iOS expects the accessibility frame in // the logical pixel coordinate system. Therefore, we divide by the `scale` (pixel ratio) to // convert. - UIScreen* screen = [[[reference bridge]->view() window] screen]; + UIScreen* screen = reference.bridge->view().window.screen; // Screen can be nil if the FlutterView is covered by another native view. - CGFloat scale = screen == nil ? [UIScreen mainScreen].scale : screen.scale; + CGFloat scale = (screen ?: UIScreen.mainScreen).scale; auto result = CGRectMake(rect.x() / scale, rect.y() / scale, rect.width() / scale, rect.height() / scale); - return UIAccessibilityConvertFrameToScreenCoordinates(result, [reference bridge]->view()); + return UIAccessibilityConvertFrameToScreenCoordinates(result, reference.bridge->view()); } } // namespace @interface FlutterSwitchSemanticsObject () -@property(nonatomic, readonly) UISwitch* nativeSwitch; +@property(nonatomic, retain, readonly) UISwitch* nativeSwitch; @end @implementation FlutterSwitchSemanticsObject @@ -116,39 +115,31 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { - (NSMethodSignature*)methodSignatureForSelector:(SEL)sel { NSMethodSignature* result = [super methodSignatureForSelector:sel]; if (!result) { - result = [_nativeSwitch methodSignatureForSelector:sel]; + result = [self.nativeSwitch methodSignatureForSelector:sel]; } return result; } - (void)forwardInvocation:(NSInvocation*)anInvocation { - [anInvocation setTarget:_nativeSwitch]; + anInvocation.target = self.nativeSwitch; [anInvocation invoke]; } - (NSString*)accessibilityValue { - if ([self node].HasFlag(flutter::SemanticsFlags::kIsToggled) || - [self node].HasFlag(flutter::SemanticsFlags::kIsChecked)) { - _nativeSwitch.on = YES; - } else { - _nativeSwitch.on = NO; - } + self.nativeSwitch.on = self.node.HasFlag(flutter::SemanticsFlags::kIsToggled) || + self.node.HasFlag(flutter::SemanticsFlags::kIsChecked); if (![self isAccessibilityBridgeAlive]) { return nil; } else { - return _nativeSwitch.accessibilityValue; + return self.nativeSwitch.accessibilityValue; } } - (UIAccessibilityTraits)accessibilityTraits { - if ([self node].HasFlag(flutter::SemanticsFlags::kIsEnabled)) { - _nativeSwitch.enabled = YES; - } else { - _nativeSwitch.enabled = NO; - } + self.nativeSwitch.enabled = self.node.HasFlag(flutter::SemanticsFlags::kIsEnabled); - return _nativeSwitch.accessibilityTraits; + return self.nativeSwitch.accessibilityTraits; } @end // FlutterSwitchSemanticsObject @@ -187,13 +178,13 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { // Once the requirements are met, the iOS uses contentOffset to determine // what scroll actions are available. e.g. If the view scrolls vertically and // contentOffset is 0.0, only the scroll down action is available. - [_scrollView setFrame:[self accessibilityFrame]]; - [_scrollView setContentSize:[self contentSizeInternal]]; - [_scrollView setContentOffset:[self contentOffsetInternal] animated:NO]; + self.scrollView.frame = self.accessibilityFrame; + self.scrollView.contentSize = [self contentSizeInternal]; + [self.scrollView setContentOffset:[self contentOffsetInternal] animated:NO]; } - (id)nativeAccessibility { - return _scrollView; + return self.scrollView; } // private methods @@ -239,7 +230,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { - (CGPoint)contentOffsetInternal { CGPoint result; - CGPoint origin = _scrollView.frame.origin; + CGPoint origin = self.scrollView.frame.origin; const SkRect& rect = self.node.rect; if (self.node.actions & flutter::kVerticalScrollSemanticsActions) { result = ConvertPointToGlobal(self, CGPointMake(rect.x(), rect.y() + [self scrollPosition])); @@ -258,14 +249,15 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { @end @interface SemanticsObject () +@property(nonatomic, retain) SemanticsObjectContainer* container; + /** Should only be called in conjunction with setting child/parent relationship. */ -- (void)privateSetParent:(SemanticsObject*)parent; +@property(nonatomic, assign, readwrite) SemanticsObject* parent; + @end @implementation SemanticsObject { - fml::scoped_nsobject _container; NSMutableArray* _children; - NSMutableArray* _childrenInHitTestOrder; BOOL _inDealloc; } @@ -294,7 +286,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { _bridge = bridge; _uid = uid; _children = [[NSMutableArray alloc] init]; - _childrenInHitTestOrder = [[NSMutableArray alloc] init]; + _childrenInHitTestOrder = [[NSArray alloc] init]; } return self; @@ -302,15 +294,14 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { - (void)dealloc { for (SemanticsObject* child in _children) { - [child privateSetParent:nil]; + child.parent = nil; } [_children removeAllObjects]; - [_childrenInHitTestOrder removeAllObjects]; [_children release]; [_childrenInHitTestOrder release]; _parent = nil; - _container.get().semanticsObject = nil; + [_container release]; _inDealloc = YES; [super dealloc]; } @@ -319,23 +310,23 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { - (void)setChildren:(NSArray*)children { for (SemanticsObject* child in _children) { - [child privateSetParent:nil]; + child.parent = nil; } [_children release]; - _children = [[NSMutableArray alloc] initWithArray:children]; + _children = [children mutableCopy]; for (SemanticsObject* child in _children) { - [child privateSetParent:self]; + child.parent = self; } } - (void)setChildrenInHitTestOrder:(NSArray*)childrenInHitTestOrder { for (SemanticsObject* child in _childrenInHitTestOrder) { - [child privateSetParent:nil]; + child.parent = nil; } [_childrenInHitTestOrder release]; - _childrenInHitTestOrder = [[NSMutableArray alloc] initWithArray:childrenInHitTestOrder]; + _childrenInHitTestOrder = [childrenInHitTestOrder copy]; for (SemanticsObject* child in _childrenInHitTestOrder) { - [child privateSetParent:self]; + child.parent = self; } } @@ -346,7 +337,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { #pragma mark - Semantic object method - (BOOL)isAccessibilityBridgeAlive { - return [self bridge].get() != nil; + return self.bridge.get() != nil; } - (void)setSemanticsNode:(const flutter::SemanticsNode*)node { @@ -360,15 +351,15 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { * Whether calling `setSemanticsNode:` with `node` would cause a layout change. */ - (BOOL)nodeWillCauseLayoutChange:(const flutter::SemanticsNode*)node { - return [self node].rect != node->rect || [self node].transform != node->transform; + return self.node.rect != node->rect || self.node.transform != node->transform; } /** * Whether calling `setSemanticsNode:` with `node` would cause a scroll event. */ - (BOOL)nodeWillCauseScroll:(const flutter::SemanticsNode*)node { - return !isnan([self node].scrollPosition) && !isnan(node->scrollPosition) && - [self node].scrollPosition != node->scrollPosition; + return !isnan(self.node.scrollPosition) && !isnan(node->scrollPosition) && + self.node.scrollPosition != node->scrollPosition; } /** @@ -382,26 +373,26 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } // The node has gained a new live region flag, always announce. - if (![self node].HasFlag(flutter::SemanticsFlags::kIsLiveRegion)) { + if (!self.node.HasFlag(flutter::SemanticsFlags::kIsLiveRegion)) { return YES; } // The label has updated, and the new node has a live region flag. - return [self node].label != node->label; + return self.node.label != node->label; } - (void)replaceChildAtIndex:(NSInteger)index withChild:(SemanticsObject*)child { SemanticsObject* oldChild = _children[index]; - [oldChild privateSetParent:nil]; - [child privateSetParent:self]; + oldChild.parent = nil; + child.parent = self; [_children replaceObjectAtIndex:index withObject:child]; } - (NSString*)routeName { // Returns the first non-null and non-empty semantic label of a child // with an NamesRoute flag. Otherwise returns nil. - if ([self node].HasFlag(flutter::SemanticsFlags::kNamesRoute)) { - NSString* newName = [self accessibilityLabel]; + if (self.node.HasFlag(flutter::SemanticsFlags::kNamesRoute)) { + NSString* newName = self.accessibilityLabel; if (newName != nil && [newName length] > 0) { return newName; } @@ -421,12 +412,6 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return self; } -#pragma mark - Semantic object private method - -- (void)privateSetParent:(SemanticsObject*)parent { - _parent = parent; -} - - (NSAttributedString*)createAttributedStringFromString:(NSString*)string withAttributes: (const flutter::StringAttributes&)attributes { @@ -459,7 +444,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } - (void)showOnScreen { - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kShowOnScreen); + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kShowOnScreen); } #pragma mark - UIAccessibility overrides @@ -474,7 +459,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { // entire element tree looking for such a hit. // We enforce in the framework that no other useful semantics are merged with these nodes. - if ([self node].HasFlag(flutter::SemanticsFlags::kScopesRoute)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kScopesRoute)) { return false; } @@ -490,14 +475,14 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { // hidden but still is a valid target for a11y focus in the tree, e.g. a list // item that is currently off screen but the a11y navigation needs to know // about. - return (([self node].flags & flutter::kScrollableSemanticsFlags) != 0 && - ([self node].flags & static_cast(flutter::SemanticsFlags::kIsHidden)) != 0) || - ![self node].label.empty() || ![self node].value.empty() || ![self node].hint.empty() || - ([self node].actions & ~flutter::kScrollableSemanticsActions) != 0; + return ((self.node.flags & flutter::kScrollableSemanticsFlags) != 0 && + (self.node.flags & static_cast(flutter::SemanticsFlags::kIsHidden)) != 0) || + !self.node.label.empty() || !self.node.value.empty() || !self.node.hint.empty() || + (self.node.actions & ~flutter::kScrollableSemanticsActions) != 0; } - (void)collectRoutes:(NSMutableArray*)edges { - if ([self node].HasFlag(flutter::SemanticsFlags::kScopesRoute)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kScopesRoute)) { [edges addObject:self]; } if ([self hasChildren]) { @@ -508,7 +493,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } - (BOOL)onCustomAccessibilityAction:(FlutterCustomAccessibilityAction*)action { - if (![self node].HasAction(flutter::SemanticsAction::kCustomAction)) { + if (!self.node.HasAction(flutter::SemanticsAction::kCustomAction)) { return NO; } int32_t action_id = action.uid; @@ -518,8 +503,8 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { args.push_back(action_id >> 8); args.push_back(action_id >> 16); args.push_back(action_id >> 24); - [self bridge]->DispatchSemanticsAction( - [self uid], flutter::SemanticsAction::kCustomAction, + self.bridge->DispatchSemanticsAction( + self.uid, flutter::SemanticsAction::kCustomAction, fml::MallocMapping::Copy(args.data(), args.size() * sizeof(uint8_t))); return YES; } @@ -529,10 +514,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } - if ([self node].identifier.empty()) { + if (self.node.identifier.empty()) { return nil; } - return @([self node].identifier.data()); + return @(self.node.identifier.data()); } - (NSString*)accessibilityLabel { @@ -540,12 +525,12 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } NSString* label = nil; - if (![self node].label.empty()) { - label = @([self node].label.data()); + if (!self.node.label.empty()) { + label = @(self.node.label.data()); } - if (![self node].tooltip.empty()) { - label = label ? [NSString stringWithFormat:@"%@\n%@", label, @([self node].tooltip.data())] - : @([self node].tooltip.data()); + if (!self.node.tooltip.empty()) { + label = label ? [NSString stringWithFormat:@"%@\n%@", label, @(self.node.tooltip.data())] + : @(self.node.tooltip.data()); } return label; } @@ -600,11 +585,11 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } - (NSAttributedString*)accessibilityAttributedLabel { - NSString* label = [self accessibilityLabel]; + NSString* label = self.accessibilityLabel; if (label.length == 0) { return nil; } - return [self createAttributedStringFromString:label withAttributes:[self node].labelAttributes]; + return [self createAttributedStringFromString:label withAttributes:self.node.labelAttributes]; } - (NSString*)accessibilityHint { @@ -612,10 +597,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } - if ([self node].hint.empty()) { + if (self.node.hint.empty()) { return nil; } - return @([self node].hint.data()); + return @(self.node.hint.data()); } - (NSAttributedString*)accessibilityAttributedHint { @@ -623,7 +608,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (hint.length == 0) { return nil; } - return [self createAttributedStringFromString:hint withAttributes:[self node].hintAttributes]; + return [self createAttributedStringFromString:hint withAttributes:self.node.hintAttributes]; } - (NSString*)accessibilityValue { @@ -631,20 +616,20 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } - if (![self node].value.empty()) { - return @([self node].value.data()); + if (!self.node.value.empty()) { + return @(self.node.value.data()); } // iOS does not announce values of native radio buttons. - if ([self node].HasFlag(flutter::SemanticsFlags::kIsInMutuallyExclusiveGroup)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsInMutuallyExclusiveGroup)) { return nil; } // FlutterSwitchSemanticsObject should supercede these conditionals. - if ([self node].HasFlag(flutter::SemanticsFlags::kHasToggledState) || - [self node].HasFlag(flutter::SemanticsFlags::kHasCheckedState)) { - if ([self node].HasFlag(flutter::SemanticsFlags::kIsToggled) || - [self node].HasFlag(flutter::SemanticsFlags::kIsChecked)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kHasToggledState) || + self.node.HasFlag(flutter::SemanticsFlags::kHasCheckedState)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsToggled) || + self.node.HasFlag(flutter::SemanticsFlags::kIsChecked)) { return @"1"; } else { return @"0"; @@ -659,7 +644,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (value.length == 0) { return nil; } - return [self createAttributedStringFromString:value withAttributes:[self node].valueAttributes]; + return [self createAttributedStringFromString:value withAttributes:self.node.valueAttributes]; } - (CGRect)accessibilityFrame { @@ -667,14 +652,14 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return CGRectMake(0, 0, 0, 0); } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsHidden)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsHidden)) { return [super accessibilityFrame]; } return [self globalRect]; } - (CGRect)globalRect { - const SkRect& rect = [self node].rect; + const SkRect& rect = self.node.rect; CGRect localRect = CGRectMake(rect.x(), rect.y(), rect.width(), rect.height()); return ConvertRectToGlobal(self, localRect); } @@ -699,20 +684,21 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } - if ([self hasChildren] || [self uid] == kRootNodeId) { - if (_container == nil) { - _container.reset([[SemanticsObjectContainer alloc] initWithSemanticsObject:self - bridge:[self bridge]]); + if ([self hasChildren] || self.uid == kRootNodeId) { + if (self.container == nil) { + self.container = + [[[SemanticsObjectContainer alloc] initWithSemanticsObject:self + bridge:self.bridge] autorelease]; } - return _container.get(); + return self.container; } - if ([self parent] == nil) { + if (self.parent == nil) { // This can happen when we have released the accessibility tree but iOS is // still holding onto our objects. iOS can take some time before it // realizes that the tree has changed. return nil; } - return [[self parent] accessibilityContainer]; + return self.parent.accessibilityContainer; } #pragma mark - UIAccessibilityAction overrides @@ -721,10 +707,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return NO; } - if (![self node].HasAction(flutter::SemanticsAction::kTap)) { + if (!self.node.HasAction(flutter::SemanticsAction::kTap)) { return NO; } - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kTap); + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kTap); return YES; } @@ -732,9 +718,9 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return; } - if ([self node].HasAction(flutter::SemanticsAction::kIncrease)) { - [self node].value = [self node].increasedValue; - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kIncrease); + if (self.node.HasAction(flutter::SemanticsAction::kIncrease)) { + self.node.value = self.node.increasedValue; + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kIncrease); } } @@ -742,9 +728,9 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return; } - if ([self node].HasAction(flutter::SemanticsAction::kDecrease)) { - [self node].value = [self node].decreasedValue; - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kDecrease); + if (self.node.HasAction(flutter::SemanticsAction::kDecrease)) { + self.node.value = self.node.decreasedValue; + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kDecrease); } } @@ -753,10 +739,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return NO; } flutter::SemanticsAction action = GetSemanticsActionForScrollDirection(direction); - if (![self node].HasAction(action)) { + if (!self.node.HasAction(action)) { return NO; } - [self bridge]->DispatchSemanticsAction([self uid], action); + self.bridge->DispatchSemanticsAction(self.uid, action); return YES; } @@ -764,10 +750,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return NO; } - if (![self node].HasAction(flutter::SemanticsAction::kDismiss)) { + if (!self.node.HasAction(flutter::SemanticsAction::kDismiss)) { return NO; } - [self bridge]->DispatchSemanticsAction([self uid], flutter::SemanticsAction::kDismiss); + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kDismiss); return YES; } @@ -777,14 +763,14 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return; } - [self bridge]->AccessibilityObjectDidBecomeFocused([self uid]); - if ([self node].HasFlag(flutter::SemanticsFlags::kIsHidden) || - [self node].HasFlag(flutter::SemanticsFlags::kIsHeader)) { + self.bridge->AccessibilityObjectDidBecomeFocused(self.uid); + if (self.node.HasFlag(flutter::SemanticsFlags::kIsHidden) || + self.node.HasFlag(flutter::SemanticsFlags::kIsHeader)) { [self showOnScreen]; } - if ([self node].HasAction(flutter::SemanticsAction::kDidGainAccessibilityFocus)) { - [self bridge]->DispatchSemanticsAction([self uid], - flutter::SemanticsAction::kDidGainAccessibilityFocus); + if (self.node.HasAction(flutter::SemanticsAction::kDidGainAccessibilityFocus)) { + self.bridge->DispatchSemanticsAction(self.uid, + flutter::SemanticsAction::kDidGainAccessibilityFocus); } } @@ -792,10 +778,10 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { if (![self isAccessibilityBridgeAlive]) { return; } - [self bridge]->AccessibilityObjectDidLoseFocus([self uid]); - if ([self node].HasAction(flutter::SemanticsAction::kDidLoseAccessibilityFocus)) { - [self bridge]->DispatchSemanticsAction([self uid], - flutter::SemanticsAction::kDidLoseAccessibilityFocus); + self.bridge->AccessibilityObjectDidLoseFocus(self.uid); + if (self.node.HasAction(flutter::SemanticsAction::kDidLoseAccessibilityFocus)) { + self.bridge->DispatchSemanticsAction(self.uid, + flutter::SemanticsAction::kDidLoseAccessibilityFocus); } } @@ -825,40 +811,40 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { - (UIAccessibilityTraits)accessibilityTraits { UIAccessibilityTraits traits = UIAccessibilityTraitNone; - if ([self node].HasAction(flutter::SemanticsAction::kIncrease) || - [self node].HasAction(flutter::SemanticsAction::kDecrease)) { + if (self.node.HasAction(flutter::SemanticsAction::kIncrease) || + self.node.HasAction(flutter::SemanticsAction::kDecrease)) { traits |= UIAccessibilityTraitAdjustable; } // This should also capture radio buttons. - if ([self node].HasFlag(flutter::SemanticsFlags::kHasToggledState) || - [self node].HasFlag(flutter::SemanticsFlags::kHasCheckedState)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kHasToggledState) || + self.node.HasFlag(flutter::SemanticsFlags::kHasCheckedState)) { traits |= UIAccessibilityTraitButton; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsSelected)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsSelected)) { traits |= UIAccessibilityTraitSelected; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsButton)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsButton)) { traits |= UIAccessibilityTraitButton; } - if ([self node].HasFlag(flutter::SemanticsFlags::kHasEnabledState) && - ![self node].HasFlag(flutter::SemanticsFlags::kIsEnabled)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kHasEnabledState) && + !self.node.HasFlag(flutter::SemanticsFlags::kIsEnabled)) { traits |= UIAccessibilityTraitNotEnabled; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsHeader)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsHeader)) { traits |= UIAccessibilityTraitHeader; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsImage)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsImage)) { traits |= UIAccessibilityTraitImage; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsLiveRegion)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsLiveRegion)) { traits |= UIAccessibilityTraitUpdatesFrequently; } - if ([self node].HasFlag(flutter::SemanticsFlags::kIsLink)) { + if (self.node.HasFlag(flutter::SemanticsFlags::kIsLink)) { traits |= UIAccessibilityTraitLink; } if (traits == UIAccessibilityTraitNone && ![self hasChildren] && - [[self accessibilityLabel] length] != 0 && - ![self node].HasFlag(flutter::SemanticsFlags::kIsTextField)) { + self.accessibilityLabel.length != 0 && + !self.node.HasFlag(flutter::SemanticsFlags::kIsTextField)) { traits = UIAccessibilityTraitStaticText; } return traits; @@ -888,13 +874,12 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } - (id)nativeAccessibility { - return _platformView; + return self.platformView; } @end @implementation SemanticsObjectContainer { - SemanticsObject* _semanticsObject; fml::WeakPtr _bridge; } @@ -927,8 +912,7 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { #pragma mark - UIAccessibilityContainer overrides - (NSInteger)accessibilityElementCount { - NSInteger count = [[_semanticsObject children] count] + 1; - return count; + return self.semanticsObject.children.count + 1; } - (nullable id)accessibilityElementAtIndex:(NSInteger)index { @@ -936,23 +920,23 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { return nil; } if (index == 0) { - return _semanticsObject.nativeAccessibility; + return self.semanticsObject.nativeAccessibility; } - SemanticsObject* child = [_semanticsObject children][index - 1]; + SemanticsObject* child = self.semanticsObject.children[index - 1]; if ([child hasChildren]) { - return [child accessibilityContainer]; + return child.accessibilityContainer; } return child.nativeAccessibility; } - (NSInteger)indexOfAccessibilityElement:(id)element { - if (element == _semanticsObject.nativeAccessibility) { + if (element == self.semanticsObject.nativeAccessibility) { return 0; } - NSArray* children = [_semanticsObject children]; + NSArray* children = self.semanticsObject.children; for (size_t i = 0; i < [children count]; i++) { SemanticsObject* child = children[i]; if ((![child hasChildren] && child.nativeAccessibility == element) || @@ -970,22 +954,22 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } - (CGRect)accessibilityFrame { - return [_semanticsObject accessibilityFrame]; + return self.semanticsObject.accessibilityFrame; } - (id)accessibilityContainer { if (!_bridge) { return nil; } - return ([_semanticsObject uid] == kRootNodeId) + return ([self.semanticsObject uid] == kRootNodeId) ? _bridge->view() - : [[_semanticsObject parent] accessibilityContainer]; + : self.semanticsObject.parent.accessibilityContainer; } #pragma mark - UIAccessibilityAction overrides - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction { - return [_semanticsObject accessibilityScroll:direction]; + return [self.semanticsObject accessibilityScroll:direction]; } @end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index 7452c5c097c..df683278397 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -14,6 +14,8 @@ FLUTTER_ASSERT_ARC +const float kFloatCompareEpsilon = 0.001; + @interface SemanticsObjectTest : XCTestCase @end @@ -314,14 +316,22 @@ FLUTTER_ASSERT_ARC [scrollable setSemanticsNode:&node]; [scrollable accessibilityBridgeDidFinishUpdate]; UIScrollView* scrollView = [scrollable nativeAccessibility]; - XCTAssertTrue( - CGRectEqualToRect(scrollView.frame, CGRectMake(x * effectivelyScale, y * effectivelyScale, - w * effectivelyScale, h * effectivelyScale))); - XCTAssertTrue(CGSizeEqualToSize( - scrollView.contentSize, - CGSizeMake(w * effectivelyScale, (h + scrollExtentMax) * effectivelyScale))); - XCTAssertTrue(CGPointEqualToPoint(scrollView.contentOffset, - CGPointMake(0, scrollPosition * effectivelyScale))); + + XCTAssertEqualWithAccuracy(scrollView.frame.origin.x, x * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.origin.y, y * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.height, h * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqualWithAccuracy(scrollView.contentSize.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.contentSize.height, + (h + scrollExtentMax) * effectivelyScale, kFloatCompareEpsilon); + + XCTAssertEqual(scrollView.contentOffset.x, 0); + XCTAssertEqualWithAccuracy(scrollView.contentOffset.y, scrollPosition * effectivelyScale, + kFloatCompareEpsilon); } - (void)testVerticalFlutterScrollableSemanticsObjectNoWindowDoesNotCrash { @@ -380,14 +390,22 @@ FLUTTER_ASSERT_ARC [scrollable setSemanticsNode:&node]; [scrollable accessibilityBridgeDidFinishUpdate]; UIScrollView* scrollView = [scrollable nativeAccessibility]; - XCTAssertTrue( - CGRectEqualToRect(scrollView.frame, CGRectMake(x * effectivelyScale, y * effectivelyScale, - w * effectivelyScale, h * effectivelyScale))); - XCTAssertTrue(CGSizeEqualToSize( - scrollView.contentSize, - CGSizeMake((w + scrollExtentMax) * effectivelyScale, h * effectivelyScale))); - XCTAssertTrue(CGPointEqualToPoint(scrollView.contentOffset, - CGPointMake(scrollPosition * effectivelyScale, 0))); + + XCTAssertEqualWithAccuracy(scrollView.frame.origin.x, x * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.origin.y, y * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.height, h * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqualWithAccuracy(scrollView.contentSize.width, (w + scrollExtentMax) * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.contentSize.height, h * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqualWithAccuracy(scrollView.contentOffset.x, scrollPosition * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqual(scrollView.contentOffset.y, 0); } - (void)testCanHandleInfiniteScrollExtent { @@ -418,15 +436,22 @@ FLUTTER_ASSERT_ARC [scrollable setSemanticsNode:&node]; [scrollable accessibilityBridgeDidFinishUpdate]; UIScrollView* scrollView = [scrollable nativeAccessibility]; - XCTAssertTrue( - CGRectEqualToRect(scrollView.frame, CGRectMake(x * effectivelyScale, y * effectivelyScale, - w * effectivelyScale, h * effectivelyScale))); - XCTAssertTrue(CGSizeEqualToSize( - scrollView.contentSize, - CGSizeMake(w * effectivelyScale, - (h + kScrollExtentMaxForInf + scrollPosition) * effectivelyScale))); - XCTAssertTrue(CGPointEqualToPoint(scrollView.contentOffset, - CGPointMake(0, scrollPosition * effectivelyScale))); + XCTAssertEqualWithAccuracy(scrollView.frame.origin.x, x * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.origin.y, y * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.height, h * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqualWithAccuracy(scrollView.contentSize.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.contentSize.height, + (h + kScrollExtentMaxForInf + scrollPosition) * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqual(scrollView.contentOffset.x, 0); + XCTAssertEqualWithAccuracy(scrollView.contentOffset.y, scrollPosition * effectivelyScale, + kFloatCompareEpsilon); } - (void)testCanHandleNaNScrollExtentAndScrollPoisition { @@ -457,13 +482,22 @@ FLUTTER_ASSERT_ARC [scrollable setSemanticsNode:&node]; [scrollable accessibilityBridgeDidFinishUpdate]; UIScrollView* scrollView = [scrollable nativeAccessibility]; - XCTAssertTrue( - CGRectEqualToRect(scrollView.frame, CGRectMake(x * effectivelyScale, y * effectivelyScale, - w * effectivelyScale, h * effectivelyScale))); + + XCTAssertEqualWithAccuracy(scrollView.frame.origin.x, x * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.origin.y, y * effectivelyScale, kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.frame.size.height, h * effectivelyScale, + kFloatCompareEpsilon); + // Content size equal to the scrollable size. - XCTAssertTrue(CGSizeEqualToSize(scrollView.contentSize, - CGSizeMake(w * effectivelyScale, h * effectivelyScale))); - XCTAssertTrue(CGPointEqualToPoint(scrollView.contentOffset, CGPointMake(0, 0))); + XCTAssertEqualWithAccuracy(scrollView.contentSize.width, w * effectivelyScale, + kFloatCompareEpsilon); + XCTAssertEqualWithAccuracy(scrollView.contentSize.height, h * effectivelyScale, + kFloatCompareEpsilon); + + XCTAssertEqual(scrollView.contentOffset.x, 0); + XCTAssertEqual(scrollView.contentOffset.y, 0); } - (void)testFlutterScrollableSemanticsObjectIsNotHittestable {