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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/ui/semantics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 {
/// The field `platformViewId` references the platform view, whose semantics
/// nodes will be added as children to this node. If a platform view is
/// specified, `childrenInHitTestOrder` and `childrenInTraversalOrder` must be
/// empty.
/// empty, and other semantic configurations will be ignored.
///
/// For scrollable nodes `scrollPosition` describes the current scroll
/// position in logical pixel. `scrollExtentMax` and `scrollExtentMin`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@
composition_order_.push_back(view_id);
}

NSObject<FlutterPlatformView>* FlutterPlatformViewsController::GetPlatformViewByID(int view_id) {
if (views_.empty()) {
return nil;
}
return views_[view_id].get();
}

std::vector<SkCanvas*> FlutterPlatformViewsController::GetCurrentCanvases() {
std::vector<SkCanvas*> canvases;
for (size_t i = 0; i < composition_order_.size(); i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class FlutterPlatformViewsController {

void PrerollCompositeEmbeddedView(int view_id);

NSObject<FlutterPlatformView>* GetPlatformViewByID(int view_id);

std::vector<SkCanvas*> GetCurrentCanvases();

SkCanvas* CompositeEmbeddedView(int view_id, const flow::EmbeddedViewParams& params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ class PlatformViewIOS;

class AccessibilityBridge final {
public:
AccessibilityBridge(UIView* view, PlatformViewIOS* platform_view);
AccessibilityBridge(UIView* view,
PlatformViewIOS* platform_view,
FlutterPlatformViewsController* platform_views_controller);
~AccessibilityBridge();

void UpdateSemantics(blink::SemanticsNodeUpdates nodes,
Expand All @@ -129,6 +131,10 @@ class AccessibilityBridge final {

fml::WeakPtr<AccessibilityBridge> GetWeakPtr();

FlutterPlatformViewsController* flutter_platform_views_controller() const {
Copy link
Member

Choose a reason for hiding this comment

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

... GetPlatformViewsController() const for consistency with the rest of the engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return flutter_platform_views_controller_;
};

void clearState();

private:
Expand All @@ -139,6 +145,7 @@ class AccessibilityBridge final {

UIView* view_;
PlatformViewIOS* platform_view_;
FlutterPlatformViewsController* flutter_platform_views_controller_;
Copy link
Member

Choose a reason for hiding this comment

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

platform_views_controller for consistency with elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

fml::scoped_nsobject<NSMutableDictionary<NSNumber*, SemanticsObject*>> objects_;
fml::scoped_nsprotocol<FlutterBasicMessageChannel*> accessibility_channel_;
fml::WeakPtrFactory<AccessibilityBridge> weak_factory_;
Expand Down
31 changes: 29 additions & 2 deletions shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#import <UIKit/UIKit.h>

#include "flutter/fml/logging.h"
#include "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
#include "flutter/shell/platform/darwin/ios/platform_view_ios.h"

namespace {
Expand Down Expand Up @@ -163,8 +164,9 @@ - (BOOL)isAccessibilityElement {
// 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(blink::SemanticsFlags::kScopesRoute))
if ([self node].HasFlag(blink::SemanticsFlags::kScopesRoute) || [self node].platformViewId > -1)
Copy link
Member

Choose a reason for hiding this comment

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

>= kMinPlatformViewId (is there a better name?) and define as constexpr above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, also added a function IsPlatformViewNode() in SemanticsNode to make it more readable.

return false;

return ([self node].flags != 0 &&
[self node].flags != static_cast<int32_t>(blink::SemanticsFlags::kIsHidden)) ||
![self node].label.empty() || ![self node].value.empty() || ![self node].hint.empty() ||
Expand Down Expand Up @@ -436,6 +438,20 @@ - (nullable id)accessibilityElementAtIndex:(NSInteger)index {
return _semanticsObject;
}
SemanticsObject* child = [_semanticsObject children][index - 1];

// This 'if' block handles adding accessibility support for the embedded platform view.
// We first check if the child is a semantic node for a platform view.
// If so, we add the platform view as accessibilityElements of the child.
Copy link
Member

Choose a reason for hiding this comment

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

Avoid documenting the how, and instead focus on the what/why. Maybe something like:
// If the child is a semantic node for a platform view, inject the platform view into the iOS accessibility tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

shell::FlutterPlatformViewsController* flutterPlatformViewsController =
_bridge.get()->flutter_platform_views_controller();
Copy link
Member

Choose a reason for hiding this comment

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

nit: consider naming it controller for readability. See: Long names are long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (child.node.platformViewId > -1 && flutterPlatformViewsController) {
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment here, replace -1 with a named constexpr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

NSObject<FlutterPlatformView>* platformViewProtocolObject =
flutterPlatformViewsController->GetPlatformViewByID(child.node.platformViewId);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a shorter name?

UIView* platformView = [platformViewProtocolObject view];
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to assume platformViewProtocolObject and [platformViewProtocolObject view] will never be nil? If not, put a guard around the next line since nil is disallowed in NSArray.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Done.

child.accessibilityElements = @[ platformView ];
return child;
}

if ([child hasChildren])
return [child accessibilityContainer];
return child;
Expand Down Expand Up @@ -485,9 +501,12 @@ - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {

namespace shell {

AccessibilityBridge::AccessibilityBridge(UIView* view, PlatformViewIOS* platform_view)
AccessibilityBridge::AccessibilityBridge(UIView* view,
PlatformViewIOS* platform_view,
FlutterPlatformViewsController* platform_views_controller)
: view_(view),
platform_view_(platform_view),
flutter_platform_views_controller_(platform_views_controller),
objects_([[NSMutableDictionary alloc] init]),
weak_factory_(this),
previous_route_id_(0),
Expand Down Expand Up @@ -660,6 +679,14 @@ - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
[object.parent.children replaceObjectAtIndex:positionInChildlist withObject:object];
objects_.get()[@(node.id)] = object;
}

BOOL isPlatformViewNode = node.platformViewId > -1;
BOOL wasPlatformViewNode = object.node.platformViewId > -1;
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment -- avoid the -1 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

if (wasPlatformViewNode && !isPlatformViewNode) {
// The node changed its type from platform view node to something else. In this
// case, we need to clean up the accessibility elements that we previously added.
object.accessibilityElements = nil;
}
}
}
return object;
Expand Down
6 changes: 4 additions & 2 deletions shell/platform/darwin/ios/platform_view_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@

if (accessibility_bridge_) {
accessibility_bridge_.reset(
new AccessibilityBridge(static_cast<FlutterView*>(owner_controller_.get().view), this));
new AccessibilityBridge(static_cast<FlutterView*>(owner_controller_.get().view), this,
[owner_controller.get() platformViewsController]));
}
// Do not call `NotifyCreated()` here - let FlutterViewController take care
// of that when its Viewport is sized. If `NotifyCreated()` is called here,
Expand Down Expand Up @@ -96,7 +97,8 @@
}
if (enabled && !accessibility_bridge_) {
accessibility_bridge_ = std::make_unique<AccessibilityBridge>(
static_cast<FlutterView*>(owner_controller_.get().view), this);
static_cast<FlutterView*>(owner_controller_.get().view), this,
[owner_controller_.get() platformViewsController]);
} else if (!enabled && accessibility_bridge_) {
accessibility_bridge_.reset();
}
Expand Down