From de4715d0dd0ee0f2c446cbf1ae2389f0e35226ba Mon Sep 17 00:00:00 2001 From: LongCat is Looong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:13:42 -0700 Subject: [PATCH 01/14] WIP --- lib/ui/semantics.dart | 9 +++- lib/ui/semantics/semantics_node.h | 1 + .../Source/SemanticsObject+UIFocusSystem.mm | 43 +++++++++++++++++++ .../fuchsia/flutter/accessibility_bridge.cc | 3 ++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index bb9d586e41e54..41ba70df04e1c 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -45,6 +45,7 @@ class SemanticsAction { static const int _kMoveCursorBackwardByWordIndex = 1 << 20; static const int _kSetTextIndex = 1 << 21; static const int _kFocusIndex = 1 << 22; + static const int _kScrollToOffset = 1 << 23; // READ THIS: if you add an action here, you MUST update the // numSemanticsActions value in testing/dart/semantics_test.dart and // lib/web_ui/test/engine/semantics/semantics_api_test.dart, or tests @@ -86,6 +87,12 @@ class SemanticsAction { /// scrollable. static const SemanticsAction scrollDown = SemanticsAction._(_kScrollDownIndex, 'scrollDown'); + /// A request to sroll the scrollable container to a given scroll offset. + /// + /// This action is used by iOS Full Keyboard Access to reveal contents that + /// are currently not visible in the viewport. + static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffset, 'scrollToOffset'); + /// A request to increase the value represented by the semantics node. /// /// For example, this action might be recognized by a slider control. @@ -764,7 +771,7 @@ base class LocaleStringAttribute extends StringAttribute { _initLocaleStringAttribute(this, range.start, range.end, locale.toLanguageTag()); } - /// The lanuage of this attribute. + /// The language of this attribute. final Locale locale; @Native(symbol: 'NativeStringAttribute::initLocaleStringAttribute') diff --git a/lib/ui/semantics/semantics_node.h b/lib/ui/semantics/semantics_node.h index 98d84c6c6aebb..0f02ff3202f18 100644 --- a/lib/ui/semantics/semantics_node.h +++ b/lib/ui/semantics/semantics_node.h @@ -43,6 +43,7 @@ enum class SemanticsAction : int32_t { kMoveCursorBackwardByWord = 1 << 20, kSetText = 1 << 21, kFocus = 1 << 22, + kScrollToOffset = 1 << 23, }; const int kVerticalScrollSemanticsActions = diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 3448b9280536b..ea993662d7e50 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #import "SemanticsObject.h" +#include "flutter/lib/ui/semantics/semantics_node.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" FLUTTER_ASSERT_ARC @@ -100,3 +101,45 @@ - (CGRect)frame { return self.bridge->view(); } @end + +@interface FlutterScrollableSemanticsObject () +@property(nonatomic, readonly) UIScrollView* scrollView; +@end + +@interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < + UIFocusItemScrollableContainer> +@end + +@implementation FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) +- (CGPoint)contentOffset { + // After the focus system calls setContentOffset, this value may not + // immediately reflect the new offset value as the accessibility updates from + // the framework only comes in once per frame. + return self.scrollView.contentOffset; +} + +- (void)setContentOffset:(CGPoint)contentOffset { + if (![self isAccessibilityBridgeAlive]) { + return; + } + + // The following code assumes the memory layout of CGPoint is exactly the + // same as an array defined as: `double coords[] = { x, y };` + // + // The size of a CGFloat is architecture-dependent and it's typically the word + // size. The last iOS with 32-bit support was iOS 10. + static_assert(sizeof(CGPoint) == sizeof(uint8_t) * 8); + self.bridge->DispatchSemanticsAction( + self.uid, flutter::SemanticsAction::kScrollToOffset, + fml::MallocMapping::Copy((uint8_t*)&contentOffset, sizeof(CGPoint) / sizeof(uint8_t));); +} + +- (CGSize)contentSize { + return self.scrollView.contentSize; +} + +- (CGSize)visibleSize { + return self.scrollView.frame.size; +} + +@end diff --git a/shell/platform/fuchsia/flutter/accessibility_bridge.cc b/shell/platform/fuchsia/flutter/accessibility_bridge.cc index b9f443035a37f..987b9303bace1 100644 --- a/shell/platform/fuchsia/flutter/accessibility_bridge.cc +++ b/shell/platform/fuchsia/flutter/accessibility_bridge.cc @@ -161,6 +161,9 @@ std::string NodeActionsToString(const flutter::SemanticsNode& node) { if (node.HasAction(flutter::SemanticsAction::kScrollUp)) { output += "kScrollUp|"; } + if (node.HasAction(flutter::SemanticsAction::kScrollToOffset)) { + output += "kScrollToOffset|"; + } if (node.HasAction(flutter::SemanticsAction::kSetSelection)) { output += "kSetSelection|"; } From 93f866ab8e85a2ad5d0949d901d849b9fa4449ab Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 31 Oct 2024 13:38:52 -0700 Subject: [PATCH 02/14] Basic scrolling --- lib/ui/semantics.dart | 1 + .../Source/SemanticsObject+UIFocusSystem.mm | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index 41ba70df04e1c..fc4b084a1e89f 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -272,6 +272,7 @@ class SemanticsAction { _kScrollRightIndex: scrollRight, _kScrollUpIndex: scrollUp, _kScrollDownIndex: scrollDown, + _kScrollToOffset: scrollToOffset, _kIncreaseIndex: increase, _kDecreaseIndex: decrease, _kShowOnScreenIndex: showOnScreen, diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index ea993662d7e50..e5ce0c6569570 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -124,14 +124,26 @@ - (void)setContentOffset:(CGPoint)contentOffset { } // The following code assumes the memory layout of CGPoint is exactly the - // same as an array defined as: `double coords[] = { x, y };` + // same as an array defined as: `double coords[] = { x, y };`. When converted + // to dart:typed_data it's a Float64List. // // The size of a CGFloat is architecture-dependent and it's typically the word // size. The last iOS with 32-bit support was iOS 10. - static_assert(sizeof(CGPoint) == sizeof(uint8_t) * 8); + static_assert(sizeof(CGPoint) == sizeof(CGFloat) * 2); + static_assert(sizeof(CGFloat) == sizeof(double)); + constexpr size_t bytesPerElement = sizeof(CGFloat); + CGFloat* data = reinterpret_cast(malloc(3 * bytesPerElement)); + + static_assert(sizeof(uint64_t) == bytesPerElement); + + // Assumes the host is little endian. + ((uint64_t*)data)[0] = 11ULL // 11 means the payload is a Float64List. + | 2ULL << 8; // 2 elements in the Float64List. + memcpy(data + 1, &contentOffset, sizeof(CGPoint)); + self.bridge->DispatchSemanticsAction( self.uid, flutter::SemanticsAction::kScrollToOffset, - fml::MallocMapping::Copy((uint8_t*)&contentOffset, sizeof(CGPoint) / sizeof(uint8_t));); + fml::MallocMapping(reinterpret_cast(data), 3 * bytesPerElement)); } - (CGSize)contentSize { From 24eb36d5a7755a37d26f8848852f48683b003c6c Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 4 Nov 2024 17:07:44 -0800 Subject: [PATCH 03/14] FKA scrolling --- .../Source/SemanticsObject+UIFocusSystem.mm | 44 +++++++++++++------ .../framework/Source/SemanticsObjectTest.mm | 28 ++++++++++++ .../Source/SemanticsObjectTestMocks.h | 14 ++++-- 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index e5ce0c6569570..ab8bec2d04513 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -4,6 +4,7 @@ #import "SemanticsObject.h" #include "flutter/lib/ui/semantics/semantics_node.h" +#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" FLUTTER_ASSERT_ARC @@ -102,6 +103,8 @@ - (CGRect)frame { } @end +#pragma mark - UIFocusItemScrollableContainer Conformance + @interface FlutterScrollableSemanticsObject () @property(nonatomic, readonly) UIScrollView* scrollView; @end @@ -124,26 +127,41 @@ - (void)setContentOffset:(CGPoint)contentOffset { } // The following code assumes the memory layout of CGPoint is exactly the - // same as an array defined as: `double coords[] = { x, y };`. When converted - // to dart:typed_data it's a Float64List. - // + // same as an array defined as: `double coords[] = { x, y };`. Converts + // to a Float64List in dart. // The size of a CGFloat is architecture-dependent and it's typically the word // size. The last iOS with 32-bit support was iOS 10. static_assert(sizeof(CGPoint) == sizeof(CGFloat) * 2); static_assert(sizeof(CGFloat) == sizeof(double)); - constexpr size_t bytesPerElement = sizeof(CGFloat); - CGFloat* data = reinterpret_cast(malloc(3 * bytesPerElement)); + FlutterStandardTypedData* offsetData = [FlutterStandardTypedData + typedDataWithFloat64:[NSData dataWithBytes:&contentOffset length:sizeof(CGPoint)]]; + NSData* encoded = [[FlutterStandardMessageCodec sharedInstance] encode:offsetData]; - static_assert(sizeof(uint64_t) == bytesPerElement); + self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kScrollToOffset, + fml::MallocMapping::Copy(encoded.bytes, encoded.length)); +} - // Assumes the host is little endian. - ((uint64_t*)data)[0] = 11ULL // 11 means the payload is a Float64List. - | 2ULL << 8; // 2 elements in the Float64List. - memcpy(data + 1, &contentOffset, sizeof(CGPoint)); +- (NSArray>*)focusItemsInRect:(CGRect)rect { + // It seems the iOS focus system relies heavily on focusItemsInRect + // (instead of preferredFocusEnvironments) for directional navigation. + // + // The order of the items seems to be important, menus and dialogs become + // unreachable via FKA if the returned children are organized + // in hit-test order. + // + // This method is only supposed to return items within the given + // rect but returning everything in the subtree seems to work fine. + NSMutableArray>* reversedItems = + [[NSMutableArray alloc] initWithCapacity:self.childrenInHitTestOrder.count]; + for (NSUInteger i = 0; i < self.childrenInHitTestOrder.count; ++i) { + [reversedItems + addObject:self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]]; + } + return reversedItems; +} - self.bridge->DispatchSemanticsAction( - self.uid, flutter::SemanticsAction::kScrollToOffset, - fml::MallocMapping(reinterpret_cast(data), 3 * bytesPerElement)); +- (BOOL)canBecomeFocused { + return NO; } - (CGSize)contentSize { diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index d0fcaeab2bb31..fa510a3e359ac 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -19,6 +19,10 @@ @interface SemanticsObject (UIFocusSystem) @end +@interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < + UIFocusItemScrollableContainer> +@end + @interface TextInputSemanticsObject (Test) - (UIView*)textInputSurrogate; @end @@ -1206,4 +1210,28 @@ - (void)testUIFocusItemContainerConformance { XCTAssertTrue([itemsInRect containsObject:child1]); XCTAssertTrue([itemsInRect containsObject:child2]); } + +- (void)testUIFocusItemScrollableContainerConformance { + fml::WeakPtrFactory factory( + new flutter::testing::MockAccessibilityBridge()); + fml::WeakPtr bridge = factory.GetWeakPtr(); + FlutterScrollableSemanticsObject* scrollable = + [[FlutterScrollableSemanticsObject alloc] initWithBridge:bridge uid:5]; + + // setContentOffset + const CGPoint p = CGPointMake(123.0, 456.0); + [scrollable setContentOffset:p]; + XCTAssertEqual(bridge->observations.size(), (size_t)1); + XCTAssertEqual(bridge->observations[0].id, 5); + XCTAssertEqual(bridge->observations[0].action, flutter::SemanticsAction::kScrollToOffset); + + std::vector args = bridge->observations[0].args; + XCTAssertEqual(args.size(), 3 * sizeof(CGFloat)); + + NSData* encoded = [NSData dataWithBytes:args.data() length:args.size()]; + FlutterStandardTypedData* decoded = [[FlutterStandardMessageCodec sharedInstance] decode:encoded]; + CGPoint point = CGPointZero; + memcpy(&point, decoded.data.bytes, decoded.data.length); + XCTAssertTrue(CGPointEqualToPoint(point, p)); +} @end diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTestMocks.h b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTestMocks.h index 4ccd57b1bbf6b..cae8cde08986b 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTestMocks.h +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTestMocks.h @@ -15,10 +15,18 @@ namespace testing { class SemanticsActionObservation { public: SemanticsActionObservation(int32_t observed_id, SemanticsAction observed_action) - : id(observed_id), action(observed_action) {} + : id(observed_id), action(observed_action), args({}) {} + + SemanticsActionObservation(int32_t observed_id, + SemanticsAction observed_action, + fml::MallocMapping& args) + : id(observed_id), + action(observed_action), + args(args.GetMapping(), args.GetMapping() + args.GetSize()) {} int32_t id; SemanticsAction action; + std::vector args; }; class MockAccessibilityBridge : public AccessibilityBridgeIos { @@ -38,7 +46,7 @@ class MockAccessibilityBridge : public AccessibilityBridgeIos { void DispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) override { - SemanticsActionObservation observation(id, action); + SemanticsActionObservation observation(id, action, args); observations.push_back(observation); } void AccessibilityObjectDidBecomeFocused(int32_t id) override {} @@ -69,7 +77,7 @@ class MockAccessibilityBridgeNoWindow : public AccessibilityBridgeIos { void DispatchSemanticsAction(int32_t id, SemanticsAction action, fml::MallocMapping args) override { - SemanticsActionObservation observation(id, action); + SemanticsActionObservation observation(id, action, args); observations.push_back(observation); } void AccessibilityObjectDidBecomeFocused(int32_t id) override {} From 74503220b0a449fc7a5e94dd3beb2841e07b6644 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 14 Nov 2024 14:38:21 -0800 Subject: [PATCH 04/14] scroll view scroll --- .../Source/FlutterSemanticsScrollView.h | 7 + .../Source/SemanticsObject+UIFocusSystem.mm | 152 +++++++++++++----- .../ios/framework/Source/SemanticsObject.mm | 24 ++- 3 files changed, 140 insertions(+), 43 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h index 28b9b5636296b..ee6b19b48a95e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h @@ -22,6 +22,13 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic, weak, nullable) SemanticsObject* semanticsObject; +/// Whether this scroll view's geometry is actively being updated by the accessibility bridge when +/// the framework sends new semantics data. +/// +/// When this flag is true, this scroll view will not dispatch any `kScrollToOffset` +/// SemanticsActions to prevent feedback loops. +@property(nonatomic, assign) BOOL isProcessFrameworkUpdates; + - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE; - (instancetype)initWithCoder:(NSCoder*)coder NS_UNAVAILABLE; diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index ab8bec2d04513..4a75fd4b967ff 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -6,6 +6,7 @@ #include "flutter/lib/ui/semantics/semantics_node.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h" FLUTTER_ASSERT_ARC @@ -31,6 +32,10 @@ @interface SemanticsObject (UIFocusSystem) @end +@interface FlutterScrollableSemanticsObject () +@property(nonatomic, readonly) FlutterSemanticsScrollView* scrollView; +@end + @implementation SemanticsObject (UIFocusSystem) #pragma mark - UIFocusEnvironment Conformance @@ -50,6 +55,9 @@ - (void)didUpdateFocusInContext:(UIFocusUpdateContext*)context } - (id)parentFocusEnvironment { + if ([self.parent isKindOfClass:[FlutterScrollableSemanticsObject class]]) { + return ((FlutterScrollableSemanticsObject*)self.parent).scrollView; + } // The root SemanticsObject node's parent is the FlutterView. return self.parent ?: self.bridge->view(); } @@ -73,8 +81,56 @@ - (BOOL)canBecomeFocused { return self.node.HasAction(flutter::SemanticsAction::kTap); } +// The frame is described in the `coordinateSpace` of the +// `parentFocusEnvironment` (all `parentFocusEnvironment`s are `UIFocusItem`s). +// +// Seealso the `coordinateSpace` implementation. - (CGRect)frame { - return self.accessibilityFrame; + SkPoint quad[4] = {SkPoint::Make(self.node.rect.left(), self.node.rect.top()), + SkPoint::Make(self.node.rect.left(), self.node.rect.bottom()), + SkPoint::Make(self.node.rect.right(), self.node.rect.top()), + SkPoint::Make(self.node.rect.right(), self.node.rect.bottom())}; + + SkM44 transform = self.node.transform; + FlutterSemanticsScrollView* scrollView; + for (SemanticsObject* ancestor = self.parent; ancestor; ancestor = ancestor.parent) { + if ([ancestor isKindOfClass:[FlutterScrollableSemanticsObject class]]) { + scrollView = ((FlutterScrollableSemanticsObject*)ancestor).scrollView; + break; + } + transform = ancestor.node.transform * transform; + } + + for (auto& vertex : quad) { + SkV4 vector = transform.map(vertex.x(), vertex.y(), 0, 1); + vertex = SkPoint::Make(vector.x / vector.w, vector.y / vector.w); + } + + SkRect rect; + rect.setBounds(quad, 4); + // If this UIFocusItemContainer's coordinateSpace is a UIScrollView, offset + // the rect by `contentOffset` because the contentOffset translation is + // incorporated into the paint transform at different node depth in UIKit + // and Flutter. In Flutter, the translation is added to the cells + // while in UIKit the viewport's bounds is manipulated (IOW, each cell's frame + // in the UIScrollView coordinateSpace does not change when the UIScrollView + // scrolls). + CGRect unscaledRect = + CGRectMake(rect.x() + scrollView.bounds.origin.x, rect.y() + scrollView.bounds.origin.y, + rect.width(), rect.height()); + if (scrollView) { + return unscaledRect; + } + // `rect` could be in physical pixels since the root RenderObject ("RenderView") + // applies a transform that turns logical pixels to physical pixels. Undo the + // transform by dividing the coordinates by the screen's scale factor, if this + // UIFocusItem's reported `coordinateSpace` is the root view (which means this + // UIFocusItem is not inside of a scroll view). + // + // Screen can be nil if the FlutterView is covered by another native view. + CGFloat scale = self.bridge->view().window.screen ?: UIScreen.mainScreen).scale; + return CGRectMake(unscaledRect.origin.x / scale, unscaledRect.origin.y / scale, + unscaledRect.size.width / scale, unscaledRect.size.height / scale); } #pragma mark - UIFocusItemContainer Conformance @@ -89,40 +145,68 @@ - (CGRect)frame { // // This method is only supposed to return items within the given // rect but returning everything in the subtree seems to work fine. - NSMutableArray* reversedItems = + NSMutableArray>* reversedItems = [[NSMutableArray alloc] initWithCapacity:self.childrenInHitTestOrder.count]; for (NSUInteger i = 0; i < self.childrenInHitTestOrder.count; ++i) { - [reversedItems - addObject:self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]]; + SemanticsObject* child = self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]; + if ([child isKindOfClass:[FlutterScrollableSemanticsObject class]]) { + [reversedItems addObject:((FlutterScrollableSemanticsObject*)child).scrollView]; + } else { + [reversedItems addObject:child]; + } } return reversedItems; } - (id)coordinateSpace { - return self.bridge->view(); + // A regular SemanticsObject uses the same coordinate space as its parent. + return self.parent.coordinateSpace ?: self.bridge->view(); } + @end -#pragma mark - UIFocusItemScrollableContainer Conformance +@interface FlutterScrollableSemanticsObject (CoordinateSpace) +@end -@interface FlutterScrollableSemanticsObject () -@property(nonatomic, readonly) UIScrollView* scrollView; +@implementation FlutterScrollableSemanticsObject (CoordinateSpace) +- (id)coordinateSpace { + // A scrollable SemanticsObject uses the same coordinate space as the scroll view. + // This may not work very well in nested scroll views. + return self.scrollView; +} @end -@interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < +/// Scrollable containers interact with the iOS focus engine using the +/// `UIFocusItemScrollableContainer` protocol. The said protocol (and other focus-related protocols) +/// does not provide means to inform the focus system of layout changes. In order for the focus +/// highlight to update properly as the scroll view scrolls, this implementation incorporates a +/// UIScrollView into the focus hierarchy to workaround the highlight update problem. +/// +/// As a result, in the current implementation only scrollable containers and the root node +/// establish their own `coordinateSpace`s. All other `UIFocusItemContainter`s use the same +/// `coordinateSpace` as the containing UIScrollView, or the root `FlutterView`, whichever is +/// closer. +/// +/// Seealso the `frame` method implementation. +#pragma mark - Scrolling + +@interface FlutterSemanticsScrollView (UIFocusItemScrollableContainer) < UIFocusItemScrollableContainer> @end -@implementation FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) -- (CGPoint)contentOffset { - // After the focus system calls setContentOffset, this value may not - // immediately reflect the new offset value as the accessibility updates from - // the framework only comes in once per frame. - return self.scrollView.contentOffset; +@implementation FlutterSemanticsScrollView (UIFocusItemScrollableContainer) + +#pragma mark - FlutterSemanticsScrollView UIFocusItemScrollableContainer Conformance + +- (CGSize)visibleSize { + return self.frame.size; } - (void)setContentOffset:(CGPoint)contentOffset { - if (![self isAccessibilityBridgeAlive]) { + [super setContentOffset:contentOffset]; + // Do no send flutter::SemanticsAction::kScrollToOffset if it's triggered + // by a framework update. + if (![self.semanticsObject isAccessibilityBridgeAlive] || self.isProcessFrameworkUpdates) { return; } @@ -136,40 +220,24 @@ - (void)setContentOffset:(CGPoint)contentOffset { FlutterStandardTypedData* offsetData = [FlutterStandardTypedData typedDataWithFloat64:[NSData dataWithBytes:&contentOffset length:sizeof(CGPoint)]]; NSData* encoded = [[FlutterStandardMessageCodec sharedInstance] encode:offsetData]; - - self.bridge->DispatchSemanticsAction(self.uid, flutter::SemanticsAction::kScrollToOffset, - fml::MallocMapping::Copy(encoded.bytes, encoded.length)); -} - -- (NSArray>*)focusItemsInRect:(CGRect)rect { - // It seems the iOS focus system relies heavily on focusItemsInRect - // (instead of preferredFocusEnvironments) for directional navigation. - // - // The order of the items seems to be important, menus and dialogs become - // unreachable via FKA if the returned children are organized - // in hit-test order. - // - // This method is only supposed to return items within the given - // rect but returning everything in the subtree seems to work fine. - NSMutableArray>* reversedItems = - [[NSMutableArray alloc] initWithCapacity:self.childrenInHitTestOrder.count]; - for (NSUInteger i = 0; i < self.childrenInHitTestOrder.count; ++i) { - [reversedItems - addObject:self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]]; - } - return reversedItems; + self.semanticsObject.bridge->DispatchSemanticsAction( + self.semanticsObject.uid, flutter::SemanticsAction::kScrollToOffset, + fml::MallocMapping::Copy(encoded.bytes, encoded.length)); } - (BOOL)canBecomeFocused { return NO; } -- (CGSize)contentSize { - return self.scrollView.contentSize; +- (id)parentFocusEnvironment { + return self.semanticsObject.parentFocusEnvironment; } -- (CGSize)visibleSize { - return self.scrollView.frame.size; +- (NSArray>*)preferredFocusEnvironments { + return nil; } +- (NSArray>*)focusItemsInRect:(CGRect)rect { + return [self.semanticsObject focusItemsInRect:rect]; +} @end diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 2fe38516f4f5f..6308c9363670d 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -9,6 +9,8 @@ FLUTTER_ASSERT_ARC +constexpr CGFloat kScrollUpdatePixelThreshould = 0.0000001; + namespace { flutter::SemanticsAction GetSemanticsActionForScrollDirection( @@ -154,6 +156,9 @@ - (instancetype)initWithBridge:(fml::WeakPtr)br _scrollView = [[FlutterSemanticsScrollView alloc] initWithSemanticsObject:self]; [_scrollView setShowsHorizontalScrollIndicator:NO]; [_scrollView setShowsVerticalScrollIndicator:NO]; + [_scrollView setContentInset:UIEdgeInsetsMake(0, 0, 0, 0)]; + [_scrollView setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever]; + _scrollView.isProcessFrameworkUpdates = NO; [self.bridge->view() addSubview:_scrollView]; } return self; @@ -164,6 +169,7 @@ - (void)dealloc { } - (void)accessibilityBridgeDidFinishUpdate { + self.scrollView.isProcessFrameworkUpdates = YES; // In order to make iOS think this UIScrollView is scrollable, the following // requirements must be true. // 1. contentSize must be bigger than the frame size. @@ -174,7 +180,23 @@ - (void)accessibilityBridgeDidFinishUpdate { // contentOffset is 0.0, only the scroll down action is available. self.scrollView.frame = self.accessibilityFrame; self.scrollView.contentSize = [self contentSizeInternal]; - [self.scrollView setContentOffset:[self contentOffsetInternal] animated:NO]; + CGPoint newOffset = [self contentOffsetInternal]; + // The iOS focus system uses a display link to scroll a scrollable container + // to the desired offset animatedly. If the user changes the scroll offset + // during the animation, the display link will be invalidated and + // the scrolling animation will be interrupted. Vet out the framework scroll + // offset updates that are generated as a result of the scroll animation to avoid + // calling setContentOffset again(which would otherwise interrupt the scrolling). + // + // The `newOffset` and `currentOffset` values can be slightly different even + // if they are the result of the same scroll animation. This happens likely + // because we're converting the offset values to single recision somewhere + // when sending this to the framework. + if (fabs(self.scrollView.contentOffset.x - newOffset.x) > kScrollUpdatePixelThreshould || + fabs(self.scrollView.contentOffset.y - newOffset.y) > kScrollUpdatePixelThreshould) { + [self.scrollView setContentOffset:newOffset animated:NO]; + } + self.scrollView.isProcessFrameworkUpdates = NO; } - (id)nativeAccessibility { From a57d56e507eccc775f63971f6444cc610126c1fa Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:44:07 -0800 Subject: [PATCH 05/14] tests --- lib/ui/semantics.dart | 5 +++ .../Source/SemanticsObject+UIFocusSystem.mm | 24 ++++++------- .../framework/Source/SemanticsObjectTest.mm | 35 +++++++++++++++++-- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index fc4b084a1e89f..848fede8e8345 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -88,6 +88,11 @@ class SemanticsAction { static const SemanticsAction scrollDown = SemanticsAction._(_kScrollDownIndex, 'scrollDown'); /// A request to sroll the scrollable container to a given scroll offset. + /// + /// The payload of this [SemanticsAction] is a flutter-stanard-encoded + /// [Float64List] of length 2 containing the target horizontal and vertical + /// offsets (in logical pixels) the receiving scrollable container should + /// scroll to. /// /// This action is used by iOS Full Keyboard Access to reveal contents that /// are currently not visible in the viewport. diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 4a75fd4b967ff..5b401f1d76dee 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -128,7 +128,7 @@ - (CGRect)frame { // UIFocusItem is not inside of a scroll view). // // Screen can be nil if the FlutterView is covered by another native view. - CGFloat scale = self.bridge->view().window.screen ?: UIScreen.mainScreen).scale; + CGFloat scale = (self.bridge->view().window.screen ?: UIScreen.mainScreen).scale; return CGRectMake(unscaledRect.origin.x / scale, unscaledRect.origin.y / scale, unscaledRect.size.width / scale, unscaledRect.size.height / scale); } @@ -165,17 +165,6 @@ - (CGRect)frame { @end -@interface FlutterScrollableSemanticsObject (CoordinateSpace) -@end - -@implementation FlutterScrollableSemanticsObject (CoordinateSpace) -- (id)coordinateSpace { - // A scrollable SemanticsObject uses the same coordinate space as the scroll view. - // This may not work very well in nested scroll views. - return self.scrollView; -} -@end - /// Scrollable containers interact with the iOS focus engine using the /// `UIFocusItemScrollableContainer` protocol. The said protocol (and other focus-related protocols) /// does not provide means to inform the focus system of layout changes. In order for the focus @@ -190,6 +179,17 @@ @implementation FlutterScrollableSemanticsObject (CoordinateSpace) /// Seealso the `frame` method implementation. #pragma mark - Scrolling +@interface FlutterScrollableSemanticsObject (CoordinateSpace) +@end + +@implementation FlutterScrollableSemanticsObject (CoordinateSpace) +- (id)coordinateSpace { + // A scrollable SemanticsObject uses the same coordinate space as the scroll view. + // This may not work very well in nested scroll views. + return self.scrollView; +} +@end + @interface FlutterSemanticsScrollView (UIFocusItemScrollableContainer) < UIFocusItemScrollableContainer> @end diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index e359cf561d523..15030f053d310 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -17,6 +17,7 @@ const float kFloatCompareEpsilon = 0.001; @interface SemanticsObject (UIFocusSystem) +@property(nonatomic) UIScrollView* scrollView; @end @interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < @@ -669,7 +670,7 @@ - (void)testFlutterScrollableSemanticsObjectReturnsParentContainerIfNoChildren { XCTAssertEqual(container.semanticsObject, parentObject); } -- (void)testFlutterScrollableSemanticsObjectHidesScrollBar { +- (void)testFlutterScrollableSemanticsObjectNoScrollBarOrContentInsets { fml::WeakPtrFactory factory( new flutter::testing::MockAccessibilityBridge()); fml::WeakPtr bridge = factory.GetWeakPtr(); @@ -689,6 +690,10 @@ - (void)testFlutterScrollableSemanticsObjectHidesScrollBar { XCTAssertFalse(scrollView.showsHorizontalScrollIndicator); XCTAssertFalse(scrollView.showsVerticalScrollIndicator); + XCTAssertEqual(scrollView.contentInsetAdjustmentBehavior, + UIScrollViewContentInsetAdjustmentNever); + XCTAssertTrue( + UIEdgeInsetsEqualToEdgeInsets(scrollView.contentInset, UIEdgeInsetsMake(0, 0, 0, 0))); } - (void)testSemanticsObjectBuildsAttributedString { @@ -1233,7 +1238,7 @@ - (void)testUIFocusItemScrollableContainerConformance { // setContentOffset const CGPoint p = CGPointMake(123.0, 456.0); - [scrollable setContentOffset:p]; + scrollable.scrollView.contentOffset = p; XCTAssertEqual(bridge->observations.size(), (size_t)1); XCTAssertEqual(bridge->observations[0].id, 5); XCTAssertEqual(bridge->observations[0].action, flutter::SemanticsAction::kScrollToOffset); @@ -1247,4 +1252,30 @@ - (void)testUIFocusItemScrollableContainerConformance { memcpy(&point, decoded.data.bytes, decoded.data.length); XCTAssertTrue(CGPointEqualToPoint(point, p)); } + +- (void)testUIFocusItemScrollableContainerNoFeedbackLoops { + fml::WeakPtrFactory factory( + new flutter::testing::MockAccessibilityBridge()); + fml::WeakPtr bridge = factory.GetWeakPtr(); + FlutterScrollableSemanticsObject* scrollable = + [[FlutterScrollableSemanticsObject alloc] initWithBridge:bridge uid:5]; + + // setContentOffset + const CGPoint p = CGPointMake(0.0, 456.0); + scrollable.scrollView.contentOffset = p; + bridge->observations.clear(); + + const SkScalar scrollPosition = p.y + 0.0000000000000001; + flutter::SemanticsNode node; + node.flags = static_cast(flutter::SemanticsFlags::kHasImplicitScrolling); + node.actions = flutter::kVerticalScrollSemanticsActions; + node.rect = SkRect::MakeXYWH(0, 0, 100, 200); + node.scrollExtentMax = 10000; + node.scrollPosition = scrollPosition; + node.transform = {1.0, 0, 0, 0, 0, 1.0, 0, 0, 0, 0, 1.0, 0, 0, scrollPosition, 0, 1.0}; + [scrollable setSemanticsNode:&node]; + [scrollable accessibilityBridgeDidFinishUpdate]; + + XCTAssertEqual(bridge->observations.size(), (size_t)0); +} @end From 07036655928b2bec3c2f287abfe241bcc3f79362 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:25:51 -0800 Subject: [PATCH 06/14] tests --- lib/ui/semantics.dart | 8 +++---- .../Source/FlutterSemanticsScrollView.h | 24 ++++++++++++++----- .../Source/FlutterSemanticsScrollView.mm | 12 ++++++++++ .../Source/SemanticsObject+UIFocusSystem.mm | 2 +- .../ios/framework/Source/SemanticsObject.mm | 24 +++---------------- .../framework/Source/SemanticsObjectTest.mm | 9 +++++-- 6 files changed, 45 insertions(+), 34 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index 848fede8e8345..9648427090cd4 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -87,10 +87,10 @@ class SemanticsAction { /// scrollable. static const SemanticsAction scrollDown = SemanticsAction._(_kScrollDownIndex, 'scrollDown'); - /// A request to sroll the scrollable container to a given scroll offset. - /// - /// The payload of this [SemanticsAction] is a flutter-stanard-encoded - /// [Float64List] of length 2 containing the target horizontal and vertical + /// A request to scroll the scrollable container to a given scroll offset. + /// + /// The payload of this [SemanticsAction] is a flutter-stanard-encoded + /// [Float64List] of length 2 containing the target horizontal and vertical /// offsets (in logical pixels) the receiving scrollable container should /// scroll to. /// diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h index ee6b19b48a95e..2a10d67c488d4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h @@ -18,16 +18,28 @@ NS_ASSUME_NONNULL_BEGIN * sends all of selector calls from accessibility services to the * owner SemanticsObject. */ -@interface FlutterSemanticsScrollView : UIScrollView +@interface FlutterSemanticsScrollView : UIScrollView @property(nonatomic, weak, nullable) SemanticsObject* semanticsObject; -/// Whether this scroll view's geometry is actively being updated by the accessibility bridge when -/// the framework sends new semantics data. +/// Whether this scroll view's content offset is actively being updated by UIKit +/// or other the system services. /// -/// When this flag is true, this scroll view will not dispatch any `kScrollToOffset` -/// SemanticsActions to prevent feedback loops. -@property(nonatomic, assign) BOOL isProcessFrameworkUpdates; +/// This flag is set by the `FlutterSemanticsScrollView` itself, typically in +/// one of the `UIScrollViewDelegate` methods. +/// +/// When this flag is true, the Flutter framework should typically cease +/// updating the content offset of this scroll view until the flag becomes +/// false, to prevent potential feedback loops (especially when the framework is +/// only echoing the new content offset back to this scroll view). +/// +/// For example, to scroll a scrollable container with iOS full keyboard access, +/// the iOS focus system uses a display link to scroll the container to the +/// desired offset animatedly. If the user changes the scroll offset during the +/// animation, the display link will be invalidated and the scrolling animation +/// will be interrupted. For simplicity, content offset updates coming from the +/// framework will be ignored in the relatively short animation duration (~1s). +@property(nonatomic, readonly) BOOL isDoingSystemScrolling; - (instancetype)init NS_UNAVAILABLE; - (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm index 9b74e2ddabc79..c6952c72b71ec 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.mm @@ -15,6 +15,8 @@ - (instancetype)initWithSemanticsObject:(SemanticsObject*)semanticsObject { self = [super initWithFrame:CGRectZero]; if (self) { _semanticsObject = semanticsObject; + _isDoingSystemScrolling = NO; + self.delegate = self; } return self; } @@ -105,4 +107,14 @@ - (NSInteger)accessibilityElementCount { return self.semanticsObject.children.count; } +- (void)scrollViewWillEndDragging:(UIScrollView*)scrollView + withVelocity:(CGPoint)velocity + targetContentOffset:(inout CGPoint*)targetContentOffset { + _isDoingSystemScrolling = YES; +} + +- (void)scrollViewDidEndDecelerating:(UIScrollView*)scrollView { + _isDoingSystemScrolling = NO; +} + @end diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 5b401f1d76dee..4783d0c9c0e83 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -206,7 +206,7 @@ - (void)setContentOffset:(CGPoint)contentOffset { [super setContentOffset:contentOffset]; // Do no send flutter::SemanticsAction::kScrollToOffset if it's triggered // by a framework update. - if (![self.semanticsObject isAccessibilityBridgeAlive] || self.isProcessFrameworkUpdates) { + if (![self.semanticsObject isAccessibilityBridgeAlive] || !self.isDoingSystemScrolling) { return; } diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 2b5951abaa5d2..f3e7a46dcc2b0 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -9,8 +9,6 @@ FLUTTER_ASSERT_ARC -constexpr CGFloat kScrollUpdatePixelThreshould = 0.0000001; - namespace { flutter::SemanticsAction GetSemanticsActionForScrollDirection( @@ -158,7 +156,6 @@ - (instancetype)initWithBridge:(fml::WeakPtr)br [_scrollView setShowsVerticalScrollIndicator:NO]; [_scrollView setContentInset:UIEdgeInsetsMake(0, 0, 0, 0)]; [_scrollView setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever]; - _scrollView.isProcessFrameworkUpdates = NO; [self.bridge->view() addSubview:_scrollView]; } return self; @@ -169,7 +166,6 @@ - (void)dealloc { } - (void)accessibilityBridgeDidFinishUpdate { - self.scrollView.isProcessFrameworkUpdates = YES; // In order to make iOS think this UIScrollView is scrollable, the following // requirements must be true. // 1. contentSize must be bigger than the frame size. @@ -180,23 +176,9 @@ - (void)accessibilityBridgeDidFinishUpdate { // contentOffset is 0.0, only the scroll down action is available. self.scrollView.frame = self.accessibilityFrame; self.scrollView.contentSize = [self contentSizeInternal]; - CGPoint newOffset = [self contentOffsetInternal]; - // The iOS focus system uses a display link to scroll a scrollable container - // to the desired offset animatedly. If the user changes the scroll offset - // during the animation, the display link will be invalidated and - // the scrolling animation will be interrupted. Vet out the framework scroll - // offset updates that are generated as a result of the scroll animation to avoid - // calling setContentOffset again(which would otherwise interrupt the scrolling). - // - // The `newOffset` and `currentOffset` values can be slightly different even - // if they are the result of the same scroll animation. This happens likely - // because we're converting the offset values to single recision somewhere - // when sending this to the framework. - if (fabs(self.scrollView.contentOffset.x - newOffset.x) > kScrollUpdatePixelThreshould || - fabs(self.scrollView.contentOffset.y - newOffset.y) > kScrollUpdatePixelThreshould) { - [self.scrollView setContentOffset:newOffset animated:NO]; - } - self.scrollView.isProcessFrameworkUpdates = NO; + if (!self.scrollView.isDoingSystemScrolling) { + [self.scrollView setContentOffset:self.contentOffsetInternal animated:NO]; + } } - (id)nativeAccessibility { diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index 15030f053d310..21d39d2cf4df8 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -7,6 +7,7 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTouchInterceptingView_Test.h" #import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h" #import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObjectTestMocks.h" @@ -17,7 +18,7 @@ const float kFloatCompareEpsilon = 0.001; @interface SemanticsObject (UIFocusSystem) -@property(nonatomic) UIScrollView* scrollView; +@property(nonatomic) FlutterSemanticsScrollView* scrollView; @end @interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < @@ -1237,8 +1238,12 @@ - (void)testUIFocusItemScrollableContainerConformance { [[FlutterScrollableSemanticsObject alloc] initWithBridge:bridge uid:5]; // setContentOffset - const CGPoint p = CGPointMake(123.0, 456.0); + CGPoint p = CGPointMake(123.0, 456.0); + [scrollable.scrollView scrollViewWillEndDragging:scrollable.scrollView + withVelocity:CGPointMake(0, 0) + targetContentOffset:&p]; scrollable.scrollView.contentOffset = p; + [scrollable.scrollView scrollViewDidEndDecelerating:scrollable.scrollView]; XCTAssertEqual(bridge->observations.size(), (size_t)1); XCTAssertEqual(bridge->observations[0].id, 5); XCTAssertEqual(bridge->observations[0].action, flutter::SemanticsAction::kScrollToOffset); From 63776e65f083662705f6586716bd2f16d3669715 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:29:43 -0800 Subject: [PATCH 07/14] docs --- .../darwin/ios/framework/Source/FlutterSemanticsScrollView.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h index 2a10d67c488d4..d61ee014ad1a4 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h @@ -38,7 +38,8 @@ NS_ASSUME_NONNULL_BEGIN /// desired offset animatedly. If the user changes the scroll offset during the /// animation, the display link will be invalidated and the scrolling animation /// will be interrupted. For simplicity, content offset updates coming from the -/// framework will be ignored in the relatively short animation duration (~1s). +/// framework will be ignored in the relatively short animation duration (~1s), +/// allowing the scrolling animation to finish. @property(nonatomic, readonly) BOOL isDoingSystemScrolling; - (instancetype)init NS_UNAVAILABLE; From bc501a2e6ea0e85b6cb30daaa834780555a0b740 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:40:11 -0800 Subject: [PATCH 08/14] tests --- lib/web_ui/test/engine/semantics/semantics_api_test.dart | 2 +- testing/dart/semantics_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/engine/semantics/semantics_api_test.dart b/lib/web_ui/test/engine/semantics/semantics_api_test.dart index 8df41a7cae384..e56242639b44c 100644 --- a/lib/web_ui/test/engine/semantics/semantics_api_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_api_test.dart @@ -29,7 +29,7 @@ void testMain() { }); // This must match the number of actions in lib/ui/semantics.dart - const int numSemanticsActions = 23; + const int numSemanticsActions = 24; test('SemanticsAction.values refers to all actions.', () async { expect(SemanticsAction.values.length, equals(numSemanticsActions)); for (int index = 0; index < numSemanticsActions; ++index) { diff --git a/testing/dart/semantics_test.dart b/testing/dart/semantics_test.dart index a5841e7d1d2fe..8b2f88921054c 100644 --- a/testing/dart/semantics_test.dart +++ b/testing/dart/semantics_test.dart @@ -22,7 +22,7 @@ void main() { }); // This must match the number of actions in lib/ui/semantics.dart - const int numSemanticsActions = 23; + const int numSemanticsActions = 24; test('SemanticsAction.values refers to all actions.', () async { expect(SemanticsAction.values.length, equals(numSemanticsActions)); for (int index = 0; index < numSemanticsActions; ++index) { From b91b878ae4f746e7fb09df16e5fe98065636242f Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:18:06 -0800 Subject: [PATCH 09/14] add to embedder enums --- lib/web_ui/lib/semantics.dart | 3 +++ .../platform/android/io/flutter/view/AccessibilityBridge.java | 3 ++- shell/platform/embedder/embedder.h | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/semantics.dart b/lib/web_ui/lib/semantics.dart index be36411520314..80fbb6cfd3c69 100644 --- a/lib/web_ui/lib/semantics.dart +++ b/lib/web_ui/lib/semantics.dart @@ -33,6 +33,7 @@ class SemanticsAction { static const int _kMoveCursorBackwardByWordIndex = 1 << 20; static const int _kSetTextIndex = 1 << 21; static const int _kFocusIndex = 1 << 22; + static const int _kScrollToOffset = 1 << 23; static const SemanticsAction tap = SemanticsAction._(_kTapIndex, 'tap'); static const SemanticsAction longPress = SemanticsAction._(_kLongPressIndex, 'longPress'); @@ -40,6 +41,7 @@ class SemanticsAction { static const SemanticsAction scrollRight = SemanticsAction._(_kScrollRightIndex, 'scrollRight'); static const SemanticsAction scrollUp = SemanticsAction._(_kScrollUpIndex, 'scrollUp'); static const SemanticsAction scrollDown = SemanticsAction._(_kScrollDownIndex, 'scrollDown'); + static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffset, 'scrollToOffset'); static const SemanticsAction increase = SemanticsAction._(_kIncreaseIndex, 'increase'); static const SemanticsAction decrease = SemanticsAction._(_kDecreaseIndex, 'decrease'); static const SemanticsAction showOnScreen = SemanticsAction._(_kShowOnScreenIndex, 'showOnScreen'); @@ -65,6 +67,7 @@ class SemanticsAction { _kScrollRightIndex: scrollRight, _kScrollUpIndex: scrollUp, _kScrollDownIndex: scrollDown, + _kScrollToOffset: scrollToOffset, _kIncreaseIndex: increase, _kDecreaseIndex: decrease, _kShowOnScreenIndex: showOnScreen, diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 0f28573023593..51d2070929f3d 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -2120,7 +2120,8 @@ public enum Action { MOVE_CURSOR_FORWARD_BY_WORD(1 << 19), MOVE_CURSOR_BACKWARD_BY_WORD(1 << 20), SET_TEXT(1 << 21), - FOCUS(1 << 22); + FOCUS(1 << 22), + SCROLL_TO_OFFSET(1 << 23); public final int value; diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index f0df9973f49ee..8ca474222d39e 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -164,6 +164,9 @@ typedef enum { kFlutterSemanticsActionSetText = 1 << 21, /// Request that the respective focusable widget gain input focus. kFlutterSemanticsActionFocus = 1 << 22, + /// Request that scrolls the current scrollable container to a given scroll + /// offset. + kFlutterSemanticsActionScrollToOffset = 1 << 23, } FlutterSemanticsAction; /// The set of properties that may be associated with a semantics node. From 4196be83bc1d9d167bed7d1b1e064e76148f18db Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:53:48 -0800 Subject: [PATCH 10/14] fix variable names --- lib/ui/semantics.dart | 6 +++--- lib/web_ui/lib/semantics.dart | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index 9648427090cd4..ff13c4ff333a3 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -45,7 +45,7 @@ class SemanticsAction { static const int _kMoveCursorBackwardByWordIndex = 1 << 20; static const int _kSetTextIndex = 1 << 21; static const int _kFocusIndex = 1 << 22; - static const int _kScrollToOffset = 1 << 23; + static const int _kScrollToOffsetIndex = 1 << 23; // READ THIS: if you add an action here, you MUST update the // numSemanticsActions value in testing/dart/semantics_test.dart and // lib/web_ui/test/engine/semantics/semantics_api_test.dart, or tests @@ -96,7 +96,7 @@ class SemanticsAction { /// /// This action is used by iOS Full Keyboard Access to reveal contents that /// are currently not visible in the viewport. - static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffset, 'scrollToOffset'); + static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffsetIndex, 'scrollToOffset'); /// A request to increase the value represented by the semantics node. /// @@ -277,7 +277,7 @@ class SemanticsAction { _kScrollRightIndex: scrollRight, _kScrollUpIndex: scrollUp, _kScrollDownIndex: scrollDown, - _kScrollToOffset: scrollToOffset, + _kScrollToOffsetIndex: scrollToOffset, _kIncreaseIndex: increase, _kDecreaseIndex: decrease, _kShowOnScreenIndex: showOnScreen, diff --git a/lib/web_ui/lib/semantics.dart b/lib/web_ui/lib/semantics.dart index 80fbb6cfd3c69..786eaacc5454f 100644 --- a/lib/web_ui/lib/semantics.dart +++ b/lib/web_ui/lib/semantics.dart @@ -33,7 +33,7 @@ class SemanticsAction { static const int _kMoveCursorBackwardByWordIndex = 1 << 20; static const int _kSetTextIndex = 1 << 21; static const int _kFocusIndex = 1 << 22; - static const int _kScrollToOffset = 1 << 23; + static const int _kScrollToOffsetIndex = 1 << 23; static const SemanticsAction tap = SemanticsAction._(_kTapIndex, 'tap'); static const SemanticsAction longPress = SemanticsAction._(_kLongPressIndex, 'longPress'); @@ -41,7 +41,7 @@ class SemanticsAction { static const SemanticsAction scrollRight = SemanticsAction._(_kScrollRightIndex, 'scrollRight'); static const SemanticsAction scrollUp = SemanticsAction._(_kScrollUpIndex, 'scrollUp'); static const SemanticsAction scrollDown = SemanticsAction._(_kScrollDownIndex, 'scrollDown'); - static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffset, 'scrollToOffset'); + static const SemanticsAction scrollToOffset = SemanticsAction._(_kScrollToOffsetIndex, 'scrollToOffset'); static const SemanticsAction increase = SemanticsAction._(_kIncreaseIndex, 'increase'); static const SemanticsAction decrease = SemanticsAction._(_kDecreaseIndex, 'decrease'); static const SemanticsAction showOnScreen = SemanticsAction._(_kShowOnScreenIndex, 'showOnScreen'); @@ -67,7 +67,7 @@ class SemanticsAction { _kScrollRightIndex: scrollRight, _kScrollUpIndex: scrollUp, _kScrollDownIndex: scrollDown, - _kScrollToOffset: scrollToOffset, + _kScrollToOffsetIndex: scrollToOffset, _kIncreaseIndex: increase, _kDecreaseIndex: decrease, _kShowOnScreenIndex: showOnScreen, From b0725225c264ac125cf75b6b45a6be1dc6ac6967 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:55:40 -0800 Subject: [PATCH 11/14] Update FlutterSemanticsScrollView.h --- .../ios/framework/Source/FlutterSemanticsScrollView.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h index d61ee014ad1a4..e5f2f6343b843 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h @@ -28,10 +28,10 @@ NS_ASSUME_NONNULL_BEGIN /// This flag is set by the `FlutterSemanticsScrollView` itself, typically in /// one of the `UIScrollViewDelegate` methods. /// -/// When this flag is true, the Flutter framework should typically cease -/// updating the content offset of this scroll view until the flag becomes -/// false, to prevent potential feedback loops (especially when the framework is -/// only echoing the new content offset back to this scroll view). +/// When this flag is true, the `SemanticsObject` implementation ignores all +/// content offset updates coming from the Flutter framework, to prevent +/// potential feedback loops (especially when the framework is only echoing +/// the new content offset back to this scroll view). /// /// For example, to scroll a scrollable container with iOS full keyboard access, /// the iOS focus system uses a display link to scroll the container to the From 15dedf579b91c5b9238ce53ee1991aa751aac809 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:28:03 -0800 Subject: [PATCH 12/14] Apply suggestions from code review Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com> Co-authored-by: Chris Bracken --- .../ios/framework/Source/SemanticsObject+UIFocusSystem.mm | 4 ++-- shell/platform/darwin/ios/framework/Source/SemanticsObject.mm | 2 +- .../darwin/ios/framework/Source/SemanticsObjectTest.mm | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 4783d0c9c0e83..50f201f22b4eb 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -84,7 +84,7 @@ - (BOOL)canBecomeFocused { // The frame is described in the `coordinateSpace` of the // `parentFocusEnvironment` (all `parentFocusEnvironment`s are `UIFocusItem`s). // -// Seealso the `coordinateSpace` implementation. +// See also the `coordinateSpace` implementation. - (CGRect)frame { SkPoint quad[4] = {SkPoint::Make(self.node.rect.left(), self.node.rect.top()), SkPoint::Make(self.node.rect.left(), self.node.rect.bottom()), @@ -176,7 +176,7 @@ - (CGRect)frame { /// `coordinateSpace` as the containing UIScrollView, or the root `FlutterView`, whichever is /// closer. /// -/// Seealso the `frame` method implementation. +/// See also the `frame` method implementation. #pragma mark - Scrolling @interface FlutterScrollableSemanticsObject (CoordinateSpace) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index f3e7a46dcc2b0..572e61946a82a 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -154,7 +154,7 @@ - (instancetype)initWithBridge:(fml::WeakPtr)br _scrollView = [[FlutterSemanticsScrollView alloc] initWithSemanticsObject:self]; [_scrollView setShowsHorizontalScrollIndicator:NO]; [_scrollView setShowsVerticalScrollIndicator:NO]; - [_scrollView setContentInset:UIEdgeInsetsMake(0, 0, 0, 0)]; + [_scrollView setContentInset:UIEdgeInsetsZero]; [_scrollView setContentInsetAdjustmentBehavior:UIScrollViewContentInsetAdjustmentNever]; [self.bridge->view() addSubview:_scrollView]; } diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index 21d39d2cf4df8..e3f255b15c9ed 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -694,7 +694,7 @@ - (void)testFlutterScrollableSemanticsObjectNoScrollBarOrContentInsets { XCTAssertEqual(scrollView.contentInsetAdjustmentBehavior, UIScrollViewContentInsetAdjustmentNever); XCTAssertTrue( - UIEdgeInsetsEqualToEdgeInsets(scrollView.contentInset, UIEdgeInsetsMake(0, 0, 0, 0))); + UIEdgeInsetsEqualToEdgeInsets(scrollView.contentInset, UIEdgeInsetsZero)); } - (void)testSemanticsObjectBuildsAttributedString { @@ -1240,7 +1240,7 @@ - (void)testUIFocusItemScrollableContainerConformance { // setContentOffset CGPoint p = CGPointMake(123.0, 456.0); [scrollable.scrollView scrollViewWillEndDragging:scrollable.scrollView - withVelocity:CGPointMake(0, 0) + withVelocity:CGPointZero targetContentOffset:&p]; scrollable.scrollView.contentOffset = p; [scrollable.scrollView scrollViewDidEndDecelerating:scrollable.scrollView]; From e14b808a25a2798b2694f3078ff52e4a0fd3b6bc Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:58:56 -0800 Subject: [PATCH 13/14] review --- .../Source/SemanticsObject+UIFocusSystem.mm | 40 +++++++++---------- .../ios/framework/Source/SemanticsObject.h | 2 +- .../framework/Source/SemanticsObjectTest.mm | 4 +- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 50f201f22b4eb..1e8d4e476124d 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -30,14 +30,19 @@ // translated to calls such as -[NSObject accessibilityActivate]), while most // other key events are dispatched to the framework. @interface SemanticsObject (UIFocusSystem) -@end - -@interface FlutterScrollableSemanticsObject () -@property(nonatomic, readonly) FlutterSemanticsScrollView* scrollView; +/// The `UIFocusItem` that represents this SemanticsObject. +/// +/// For regular `SemanticsObject`s, this method returns `self`, +/// for `FlutterScrollableSemanticsObject`s, this method returns its scroll view. +- (id)focusItem; @end @implementation SemanticsObject (UIFocusSystem) +- (id)focusItem { + return self; +} + #pragma mark - UIFocusEnvironment Conformance - (void)setNeedsFocusUpdate { @@ -55,11 +60,8 @@ - (void)didUpdateFocusInContext:(UIFocusUpdateContext*)context } - (id)parentFocusEnvironment { - if ([self.parent isKindOfClass:[FlutterScrollableSemanticsObject class]]) { - return ((FlutterScrollableSemanticsObject*)self.parent).scrollView; - } // The root SemanticsObject node's parent is the FlutterView. - return self.parent ?: self.bridge->view(); + return self.parent.focusItem ?: self.bridge->view(); } - (NSArray>*)preferredFocusEnvironments { @@ -85,6 +87,7 @@ - (BOOL)canBecomeFocused { // `parentFocusEnvironment` (all `parentFocusEnvironment`s are `UIFocusItem`s). // // See also the `coordinateSpace` implementation. +// TODO(LongCatIsLooong): use CoreGraphics types. - (CGRect)frame { SkPoint quad[4] = {SkPoint::Make(self.node.rect.left(), self.node.rect.top()), SkPoint::Make(self.node.rect.left(), self.node.rect.bottom()), @@ -149,11 +152,7 @@ - (CGRect)frame { [[NSMutableArray alloc] initWithCapacity:self.childrenInHitTestOrder.count]; for (NSUInteger i = 0; i < self.childrenInHitTestOrder.count; ++i) { SemanticsObject* child = self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]; - if ([child isKindOfClass:[FlutterScrollableSemanticsObject class]]) { - [reversedItems addObject:((FlutterScrollableSemanticsObject*)child).scrollView]; - } else { - [reversedItems addObject:child]; - } + [reversedItems addObject:child.focusItem]; } return reversedItems; } @@ -188,6 +187,11 @@ @implementation FlutterScrollableSemanticsObject (CoordinateSpace) // This may not work very well in nested scroll views. return self.scrollView; } + +- (id)focusItem { + return self.scrollView; +} + @end @interface FlutterSemanticsScrollView (UIFocusItemScrollableContainer) < @@ -210,15 +214,9 @@ - (void)setContentOffset:(CGPoint)contentOffset { return; } - // The following code assumes the memory layout of CGPoint is exactly the - // same as an array defined as: `double coords[] = { x, y };`. Converts - // to a Float64List in dart. - // The size of a CGFloat is architecture-dependent and it's typically the word - // size. The last iOS with 32-bit support was iOS 10. - static_assert(sizeof(CGPoint) == sizeof(CGFloat) * 2); - static_assert(sizeof(CGFloat) == sizeof(double)); + double offsetData[2] = {contentOffset.x, contentOffset.y}; FlutterStandardTypedData* offsetData = [FlutterStandardTypedData - typedDataWithFloat64:[NSData dataWithBytes:&contentOffset length:sizeof(CGPoint)]]; + typedDataWithFloat64:[NSData dataWithBytes:&offsetData length:sizeof(offsetData)]]; NSData* encoded = [[FlutterStandardMessageCodec sharedInstance] encode:offsetData]; self.semanticsObject.bridge->DispatchSemanticsAction( self.semanticsObject.uid, flutter::SemanticsAction::kScrollToOffset, diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.h b/shell/platform/darwin/ios/framework/Source/SemanticsObject.h index d3441ec0058b1..b779ea9b29fd9 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.h +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.h @@ -186,7 +186,7 @@ constexpr float kScrollExtentMaxForInf = 1000; /// The semantics object for scrollable. This class creates an UIScrollView to interact with the /// iOS. @interface FlutterScrollableSemanticsObject : SemanticsObject - +@property(nonatomic, readonly) FlutterSemanticsScrollView* scrollView; @end /** diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm index e3f255b15c9ed..02fb11f98336c 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm @@ -18,7 +18,6 @@ const float kFloatCompareEpsilon = 0.001; @interface SemanticsObject (UIFocusSystem) -@property(nonatomic) FlutterSemanticsScrollView* scrollView; @end @interface FlutterScrollableSemanticsObject (UIFocusItemScrollableContainer) < @@ -693,8 +692,7 @@ - (void)testFlutterScrollableSemanticsObjectNoScrollBarOrContentInsets { XCTAssertFalse(scrollView.showsVerticalScrollIndicator); XCTAssertEqual(scrollView.contentInsetAdjustmentBehavior, UIScrollViewContentInsetAdjustmentNever); - XCTAssertTrue( - UIEdgeInsetsEqualToEdgeInsets(scrollView.contentInset, UIEdgeInsetsZero)); + XCTAssertTrue(UIEdgeInsetsEqualToEdgeInsets(scrollView.contentInset, UIEdgeInsetsZero)); } - (void)testSemanticsObjectBuildsAttributedString { From 1bf0f5cdebf43333a1b41819308e2d81fea8f78d Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 21 Nov 2024 13:44:59 -0800 Subject: [PATCH 14/14] review --- lib/ui/semantics.dart | 2 +- .../ios/framework/Source/SemanticsObject+UIFocusSystem.mm | 4 ++-- shell/platform/darwin/ios/framework/Source/SemanticsObject.h | 1 + shell/platform/darwin/ios/framework/Source/SemanticsObject.mm | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index ff13c4ff333a3..59052b157b687 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -89,7 +89,7 @@ class SemanticsAction { /// A request to scroll the scrollable container to a given scroll offset. /// - /// The payload of this [SemanticsAction] is a flutter-stanard-encoded + /// The payload of this [SemanticsAction] is a flutter-standard-encoded /// [Float64List] of length 2 containing the target horizontal and vertical /// offsets (in logical pixels) the receiving scrollable container should /// scroll to. diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm index 1e8d4e476124d..c3262de538dab 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject+UIFocusSystem.mm @@ -214,9 +214,9 @@ - (void)setContentOffset:(CGPoint)contentOffset { return; } - double offsetData[2] = {contentOffset.x, contentOffset.y}; + double offset[2] = {contentOffset.x, contentOffset.y}; FlutterStandardTypedData* offsetData = [FlutterStandardTypedData - typedDataWithFloat64:[NSData dataWithBytes:&offsetData length:sizeof(offsetData)]]; + typedDataWithFloat64:[NSData dataWithBytes:&offset length:sizeof(offset)]]; NSData* encoded = [[FlutterStandardMessageCodec sharedInstance] encode:offsetData]; self.semanticsObject.bridge->DispatchSemanticsAction( self.semanticsObject.uid, flutter::SemanticsAction::kScrollToOffset, diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.h b/shell/platform/darwin/ios/framework/Source/SemanticsObject.h index b779ea9b29fd9..2c9e0516112ea 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.h +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.h @@ -10,6 +10,7 @@ #include "flutter/fml/macros.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/lib/ui/semantics/semantics_node.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterSemanticsScrollView.h" #import "flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_ios.h" constexpr int32_t kRootNodeId = 0; diff --git a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 572e61946a82a..39882196e8e99 100644 --- a/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -176,6 +176,7 @@ - (void)accessibilityBridgeDidFinishUpdate { // contentOffset is 0.0, only the scroll down action is available. self.scrollView.frame = self.accessibilityFrame; self.scrollView.contentSize = [self contentSizeInternal]; + // See the documentation on `isDoingSystemScrolling`. if (!self.scrollView.isDoingSystemScrolling) { [self.scrollView setContentOffset:self.contentOffsetInternal animated:NO]; }