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 11 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
6 changes: 5 additions & 1 deletion lib/ui/semantics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,11 @@ 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. The ignored configurations
/// include:[flags], [actions], [textSelectionBase], [textSelectionExtent], [scrollChildren],
Copy link
Member

Choose a reason for hiding this comment

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

nit: space missing after :

/// [scrollIndex], [scrollPosition], [scrollExtentMax], [scrollExtentMin], [elevation],
/// [thickness], [label], [hint], [value], [increasedValue], [decreasedValue], [textDirection],
Copy link
Member

Choose a reason for hiding this comment

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

This explicit list seems hard to maintain. Nobody will remember to add a property here if we add a new one.

Copy link
Contributor Author

@cyanglaz cyanglaz Mar 15, 2019

Choose a reason for hiding this comment

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

This is a good point, we also have the same problem in the framework semantics code. I think one problem could be that we have a very long list of properties inside these semantics class, this can lead to maintain hell.
There are ways I can think of.

  1. We can refactor SemanticNode, SemanticConfiguration(in framework), SemanticData(in framework), SemanticProperty(in framework) classes, group this long list of properties to make them shorter. For example, the semanticNode can have scrollingConfig, textConfig, actionConfig, transformConfig, etc and each contains reasonable amount of properties. This requires a lot of refactoring work and regression test, and it is also a breaking change. But I feel like it would be a better long term solution since it can be easily scaled up.
  2. A non breaking way: We can have a different semantic node class to represent a node for platform view.

Or maybe we can do both 1 and 2?

Do you have a different way to do it? Also copying @cbracken on this. Maybe we should open a new issue to track this effort?

Copy link
Member

Choose a reason for hiding this comment

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

  1. would probably be nice, but since it is a breaking change I am not sure if it is currently worth doing. It also doesn't fix the problem. You'd still have to keep a (now shorter) list upto date.

I do not understand what you mean by 2.

Instead of listing all these properties, I'd probably just explain what it means on the platform side to set a paltformViewId.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After offline discussion with @goderbauer, I am going to restructure the platform view subtree.

/// [transform], [childrenInTraversalOrder], [childrenInHitTestOrder], [additionalActions]
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised to see that transform is ignored. What if the PlatForm viewed is scaled up or down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we current support transforming in platform view. @amirh

///
/// For scrollable nodes `scrollPosition` describes the current scroll
/// position in logical pixel. `scrollExtentMax` and `scrollExtentMin`
Expand Down
6 changes: 6 additions & 0 deletions lib/ui/semantics/semantics_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include <string.h>

namespace blink {

constexpr int32_t kMinPlatfromViewId = -1;

SemanticsNode::SemanticsNode() = default;

Expand All @@ -21,5 +23,9 @@ bool SemanticsNode::HasAction(SemanticsAction action) {
bool SemanticsNode::HasFlag(SemanticsFlags flag) {
return (flags & static_cast<int32_t>(flag)) != 0;
}

bool SemanticsNode::IsPlatformViewNode() {
return platformViewId > kMinPlatfromViewId;
}

} // namespace blink
3 changes: 3 additions & 0 deletions lib/ui/semantics/semantics_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ struct SemanticsNode {

bool HasAction(SemanticsAction action);
bool HasFlag(SemanticsFlags flag);

// Weather this node is for embeded platform views.
bool IsPlatformViewNode();

int32_t id = 0;
int32_t flags = 0;
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,13 @@ class FlutterPlatformViewsController {

void PrerollCompositeEmbeddedView(int view_id);

// Returns the FlutterPlatformView object associated with the view_id.
//
// If the FlutterPlatformViewsController does not contain any FlutterPlatformView object or
// a FlutterPlatformView object asscociated with the view_id cannot be found, the method returns
// nil.
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* GetFlutterPlatformViewsController() const {
return platform_views_controller_;
};

void clearState();

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

UIView* view_;
PlatformViewIOS* platform_view_;
FlutterPlatformViewsController* platform_views_controller_;
fml::scoped_nsobject<NSMutableDictionary<NSNumber*, SemanticsObject*>> objects_;
fml::scoped_nsprotocol<FlutterBasicMessageChannel*> accessibility_channel_;
fml::WeakPtrFactory<AccessibilityBridge> weak_factory_;
Expand Down
32 changes: 30 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].IsPlatformViewNode())
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,21 @@ - (nullable id)accessibilityElementAtIndex:(NSInteger)index {
return _semanticsObject;
}
SemanticsObject* child = [_semanticsObject children][index - 1];

// If the child is a semantic node for a platform view,
// inject the platform view into the iOS accessibility tree.
shell::FlutterPlatformViewsController* controller =
_bridge.get()->GetFlutterPlatformViewsController();
if (child.node.IsPlatformViewNode() && controller) {
NSObject<FlutterPlatformView>* platformViewContainer =
controller->GetPlatformViewByID(child.node.platformViewId);
UIView* platformView = [platformViewContainer view];
if (platformView) {
child.accessibilityElements = @[ platformView ];
return child;
}
}

if ([child hasChildren])
return [child accessibilityContainer];
return child;
Expand Down Expand Up @@ -485,9 +502,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),
platform_views_controller_(platform_views_controller),
objects_([[NSMutableDictionary alloc] init]),
weak_factory_(this),
previous_route_id_(0),
Expand Down Expand Up @@ -660,6 +680,14 @@ - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
[object.parent.children replaceObjectAtIndex:positionInChildlist withObject:object];
objects_.get()[@(node.id)] = object;
}

BOOL isPlatformViewNode = node.IsPlatformViewNode();
BOOL wasPlatformViewNode = object.node.IsPlatformViewNode();
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