Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Support text editing voiceover feedback in macOS #25600

Merged
merged 1 commit into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,9 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterSurfa
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextureRegistrar.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h
Expand Down
18 changes: 17 additions & 1 deletion shell/platform/common/accessibility_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,22 @@ AccessibilityBridge::GetPendingEvents() {
return result;
}

void AccessibilityBridge::UpdateDelegate(
std::unique_ptr<AccessibilityBridgeDelegate> delegate) {
delegate_ = std::move(delegate);
// Recreate FlutterPlatformNodeDelegates since they may contain stale state
// from the previous AccessibilityBridgeDelegate.
for (const auto& [node_id, old_platform_node_delegate] : id_wrapper_map_) {
std::shared_ptr<FlutterPlatformNodeDelegate> platform_node_delegate =
delegate_->CreateFlutterPlatformNodeDelegate();
platform_node_delegate->Init(
std::static_pointer_cast<FlutterPlatformNodeDelegate::OwnerBridge>(
shared_from_this()),
old_platform_node_delegate->GetAXNode());
id_wrapper_map_[node_id] = platform_node_delegate;
}
}

void AccessibilityBridge::OnNodeWillBeDeleted(ui::AXTree* tree,
ui::AXNode* node) {}

Expand Down Expand Up @@ -329,7 +345,7 @@ void AccessibilityBridge::SetBooleanAttributesFromFlutterUpdate(
node_data.AddBoolAttribute(
ax::mojom::BoolAttribute::kEditableRoot,
flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField &&
(flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsReadOnly) > 0);
(flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsReadOnly) == 0);
}

void AccessibilityBridge::SetIntAttributesFromFlutterUpdate(
Expand Down
7 changes: 6 additions & 1 deletion shell/platform/common/accessibility_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class AccessibilityBridge
/// by accessibility bridge whenever a new AXNode is created in
/// AXTree. Each platform needs to implement this method in
/// order to inject its subclass into the accessibility bridge.
virtual std::unique_ptr<FlutterPlatformNodeDelegate>
virtual std::shared_ptr<FlutterPlatformNodeDelegate>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a mistake I made in previous pr. The caller expects a shared_ptr.

CreateFlutterPlatformNodeDelegate() = 0;
};
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -163,6 +163,11 @@ class AccessibilityBridge
/// all pending events.
const std::vector<ui::AXEventGenerator::TargetedEvent> GetPendingEvents();

//------------------------------------------------------------------------------
/// @brief Update the AccessibilityBridgeDelegate stored in the
/// accessibility bridge to a new one.
void UpdateDelegate(std::unique_ptr<AccessibilityBridgeDelegate> delegate);

private:
// See FlutterSemanticsNode in embedder.h
typedef struct {
Expand Down
89 changes: 89 additions & 0 deletions shell/platform/common/accessibility_bridge_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,66 @@ TEST(AccessibilityBridgeTest, canFireChildrenChangedCorrectly) {
actual_event.end());
}

TEST(AccessibilityBridgeTest, canUpdateDelegate) {
std::shared_ptr<AccessibilityBridge> bridge =
std::make_shared<AccessibilityBridge>(
std::make_unique<TestAccessibilityBridgeDelegate>());
FlutterSemanticsNode root;
root.id = 0;
root.flags = static_cast<FlutterSemanticsFlag>(0);
root.actions = static_cast<FlutterSemanticsAction>(0);
root.text_selection_base = -1;
root.text_selection_extent = -1;
root.label = "root";
root.hint = "";
root.value = "";
root.increased_value = "";
root.decreased_value = "";
root.child_count = 1;
int32_t children[] = {1};
root.children_in_traversal_order = children;
root.custom_accessibility_actions_count = 0;
bridge->AddFlutterSemanticsNodeUpdate(&root);

FlutterSemanticsNode child1;
child1.id = 1;
child1.flags = static_cast<FlutterSemanticsFlag>(0);
child1.actions = static_cast<FlutterSemanticsAction>(0);
child1.text_selection_base = -1;
child1.text_selection_extent = -1;
child1.label = "child 1";
child1.hint = "";
child1.value = "";
child1.increased_value = "";
child1.decreased_value = "";
child1.child_count = 0;
child1.custom_accessibility_actions_count = 0;
bridge->AddFlutterSemanticsNodeUpdate(&child1);

bridge->CommitUpdates();

auto root_node = bridge->GetFlutterPlatformNodeDelegateFromID(0);
auto child1_node = bridge->GetFlutterPlatformNodeDelegateFromID(1);
EXPECT_FALSE(root_node.expired());
EXPECT_FALSE(child1_node.expired());
// Update Delegate
bridge->UpdateDelegate(std::make_unique<TestAccessibilityBridgeDelegate>());

// Old tree is destroyed.
EXPECT_TRUE(root_node.expired());
EXPECT_TRUE(child1_node.expired());

// New tree still has the data.
auto new_root_node = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();
auto new_child1_node = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock();
EXPECT_EQ(new_root_node->GetChildCount(), 1);
EXPECT_EQ(new_root_node->GetData().child_ids[0], 1);
EXPECT_EQ(new_root_node->GetName(), "root");

EXPECT_EQ(new_child1_node->GetChildCount(), 0);
EXPECT_EQ(new_child1_node->GetName(), "child 1");
}

TEST(AccessibilityBridgeTest, canHandleSelectionChangeCorrectly) {
TestAccessibilityBridgeDelegate* delegate =
new TestAccessibilityBridgeDelegate();
Expand Down Expand Up @@ -200,5 +260,34 @@ TEST(AccessibilityBridgeTest, canHandleSelectionChangeCorrectly) {
ui::AXEventGenerator::Event::OTHER_ATTRIBUTE_CHANGED);
}

TEST(AccessibilityBridgeTest, doesNotAssignEditableRootToSelectableText) {
std::shared_ptr<AccessibilityBridge> bridge =
std::make_shared<AccessibilityBridge>(
std::make_unique<TestAccessibilityBridgeDelegate>());
FlutterSemanticsNode root;
root.id = 0;
root.flags = static_cast<FlutterSemanticsFlag>(
FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField |
FlutterSemanticsFlag::kFlutterSemanticsFlagIsReadOnly);
root.actions = static_cast<FlutterSemanticsAction>(0);
root.text_selection_base = -1;
root.text_selection_extent = -1;
root.label = "root";
root.hint = "";
root.value = "";
root.increased_value = "";
root.decreased_value = "";
root.child_count = 0;
root.custom_accessibility_actions_count = 0;
bridge->AddFlutterSemanticsNodeUpdate(&root);

bridge->CommitUpdates();

auto root_node = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();

EXPECT_FALSE(root_node->GetData().GetBoolAttribute(
ax::mojom::BoolAttribute::kEditableRoot));
}

} // namespace testing
} // namespace flutter
5 changes: 5 additions & 0 deletions shell/platform/common/flutter_platform_node_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,9 @@ gfx::Rect FlutterPlatformNodeDelegate::GetBoundsRect(
return gfx::ToEnclosingRect(bounds);
}

std::weak_ptr<FlutterPlatformNodeDelegate::OwnerBridge>
FlutterPlatformNodeDelegate::GetOwnerBridge() const {
return bridge_;
}

} // namespace flutter
37 changes: 22 additions & 15 deletions shell/platform/common/flutter_platform_node_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
public:
virtual ~OwnerBridge() = default;

//---------------------------------------------------------------------------
/// @brief Gets the rectangular bounds of the ax node relative to
/// global coordinate
///
/// @param[in] node The ax node to look up.
/// @param[in] offscreen the bool reference to hold the result whether
/// the ax node is outside of its ancestors' bounds.
/// @param[in] clip_bounds whether to clip the result if the ax node cannot
/// be fully contained in its ancestors' bounds.
virtual gfx::RectF RelativeToGlobalBounds(const ui::AXNode* node,
bool& offscreen,
bool clip_bounds) = 0;

protected:
friend class FlutterPlatformNodeDelegate;

Expand Down Expand Up @@ -78,19 +91,6 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
///
/// @param[in] node_id The id of the focused node.
virtual void SetLastFocusedId(AccessibilityNodeId node_id) = 0;

//---------------------------------------------------------------------------
/// @brief Gets the rectangular bounds of the ax node relative to
/// global coordinate
///
/// @param[in] node The ax node to look up.
/// @param[in] offscreen the bool reference to hold the result whether
/// the ax node is outside of its ancestors' bounds.
/// @param[in] clip_bounds whether to clip the result if the ax node cannot
/// be fully contained in its ancestors' bounds.
virtual gfx::RectF RelativeToGlobalBounds(const ui::AXNode* node,
bool& offscreen,
bool clip_bounds) = 0;
};

FlutterPlatformNodeDelegate();
Expand Down Expand Up @@ -129,11 +129,18 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
/// Subclasses must call super.
virtual void Init(std::weak_ptr<OwnerBridge> bridge, ui::AXNode* node);

protected:
//------------------------------------------------------------------------------
/// @brief Gets the underlying ax node for this accessibility node.
/// @brief Gets the underlying ax node for this platform node delegate.
ui::AXNode* GetAXNode() const;

//------------------------------------------------------------------------------
/// @brief Gets the owner of this platform node delegate. This is useful
/// when you want to get the information about surrounding nodes
/// of this platform node delegate, e.g. the global rect of this
/// platform node delegate. This pointer is only safe in the
/// platform thread.
std::weak_ptr<OwnerBridge> GetOwnerBridge() const;

private:
ui::AXNode* ax_node_;
std::weak_ptr<OwnerBridge> bridge_;
Expand Down
46 changes: 46 additions & 0 deletions shell/platform/common/flutter_platform_node_delegate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,5 +174,51 @@ TEST(FlutterPlatformNodeDelegateTest, canCalculateOffScreenBoundsCorrectly) {
EXPECT_EQ(result, ui::AXOffscreenResult::kOffscreen);
}

TEST(FlutterPlatformNodeDelegateTest, canUseOwnerBridge) {
std::shared_ptr<AccessibilityBridge> bridge =
std::make_shared<AccessibilityBridge>(
std::make_unique<TestAccessibilityBridgeDelegate>());
FlutterSemanticsNode root;
root.id = 0;
root.label = "root";
root.hint = "";
root.value = "";
root.increased_value = "";
root.decreased_value = "";
root.child_count = 1;
int32_t children[] = {1};
root.children_in_traversal_order = children;
root.custom_accessibility_actions_count = 0;
root.rect = {0, 0, 100, 100}; // LTRB
root.transform = {1, 0, 0, 0, 1, 0, 0, 0, 1};
bridge->AddFlutterSemanticsNodeUpdate(&root);

FlutterSemanticsNode child1;
child1.id = 1;
child1.label = "child 1";
child1.hint = "";
child1.value = "";
child1.increased_value = "";
child1.decreased_value = "";
child1.child_count = 0;
child1.custom_accessibility_actions_count = 0;
child1.rect = {0, 0, 50, 50}; // LTRB
child1.transform = {0.5, 0, 0, 0, 0.5, 0, 0, 0, 1};
bridge->AddFlutterSemanticsNodeUpdate(&child1);

bridge->CommitUpdates();
auto child1_node = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock();
auto owner_bridge = child1_node->GetOwnerBridge().lock();

bool result;
gfx::RectF bounds = owner_bridge->RelativeToGlobalBounds(
child1_node->GetAXNode(), result, true);
EXPECT_EQ(bounds.x(), 0);
EXPECT_EQ(bounds.y(), 0);
EXPECT_EQ(bounds.width(), 25);
EXPECT_EQ(bounds.height(), 25);
EXPECT_EQ(result, false);
}

} // namespace testing
} // namespace flutter
2 changes: 1 addition & 1 deletion shell/platform/common/test_accessibility_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace flutter {

std::unique_ptr<FlutterPlatformNodeDelegate>
std::shared_ptr<FlutterPlatformNodeDelegate>
TestAccessibilityBridgeDelegate::CreateFlutterPlatformNodeDelegate() {
return std::make_unique<FlutterPlatformNodeDelegate>();
};
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/common/test_accessibility_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class TestAccessibilityBridgeDelegate
void DispatchAccessibilityAction(AccessibilityNodeId target,
FlutterSemanticsAction action,
fml::MallocMapping data) override;
std::unique_ptr<FlutterPlatformNodeDelegate>
CreateFlutterPlatformNodeDelegate();
std::shared_ptr<FlutterPlatformNodeDelegate>
CreateFlutterPlatformNodeDelegate() override;

std::vector<ui::AXEventGenerator::TargetedEvent> accessibilitiy_events;
std::vector<FlutterSemanticsAction> performed_actions;
Expand Down
3 changes: 3 additions & 0 deletions shell/platform/darwin/macos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterSurfaceManager.mm",
"framework/Source/FlutterTextInputPlugin.h",
"framework/Source/FlutterTextInputPlugin.mm",
"framework/Source/FlutterTextInputSemanticsObject.h",
"framework/Source/FlutterTextInputSemanticsObject.mm",
"framework/Source/FlutterTextureRegistrar.h",
"framework/Source/FlutterTextureRegistrar.mm",
"framework/Source/FlutterView.h",
Expand Down Expand Up @@ -181,6 +183,7 @@ executable("flutter_desktop_darwin_unittests") {
"framework/Source/FlutterOpenGLRendererTest.mm",
"framework/Source/FlutterPlatformNodeDelegateMacTest.mm",
"framework/Source/FlutterTextInputPluginTest.mm",
"framework/Source/FlutterTextInputSemanticsObjectTest.mm",
"framework/Source/FlutterViewControllerTest.mm",
"framework/Source/FlutterViewControllerTestUtils.h",
"framework/Source/FlutterViewControllerTestUtils.mm",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/shell/platform/common/accessibility_bridge.h"

@class FlutterEngine;
@class FlutterViewController;

namespace flutter {

Expand All @@ -20,8 +21,10 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility
public:
//---------------------------------------------------------------------------
/// @brief Creates an AccessibilityBridgeMacDelegate.
/// @param[in] flutterEngine The weak reference to the FlutterEngine.
explicit AccessibilityBridgeMacDelegate(__weak FlutterEngine* flutter_engine);
/// @param[in] flutter_engine The weak reference to the FlutterEngine.
/// @param[in] view_controller The weak reference to the FlutterViewController.
explicit AccessibilityBridgeMacDelegate(__weak FlutterEngine* flutter_engine,
__weak FlutterViewController* view_controller);
virtual ~AccessibilityBridgeMacDelegate() = default;

// |AccessibilityBridge::AccessibilityBridgeDelegate|
Expand All @@ -33,7 +36,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility
fml::MallocMapping data) override;

// |AccessibilityBridge::AccessibilityBridgeDelegate|
std::unique_ptr<FlutterPlatformNodeDelegate> CreateFlutterPlatformNodeDelegate() override;
std::shared_ptr<FlutterPlatformNodeDelegate> CreateFlutterPlatformNodeDelegate() override;

private:
/// A wrapper structure to wraps macOS native accessibility events.
Expand Down Expand Up @@ -64,7 +67,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility

//---------------------------------------------------------------------------
/// @brief Whether the given event is in current pending events.
/// @param[in] event_type The event you would like to look up.
/// @param[in] event_type The event to look up.
bool HasPendingEvent(ui::AXEventGenerator::Event event) const;

//---------------------------------------------------------------------------
Expand All @@ -76,6 +79,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility
const ui::AXNode& ax_node) const;

__weak FlutterEngine* flutter_engine_;
__weak FlutterViewController* view_controller_;
};

} // namespace flutter
Expand Down
Loading