Skip to content

Commit

Permalink
Override FlutterPlatformNodeDelegate::GetUniqueId (flutter#30261)
Browse files Browse the repository at this point in the history
The default implementation of GetUniqueId on ui::AXPlatformNodeDelegate
always returns ID 1. We had previously implemented this on the windows
platform node delegate, but for consistency's sake, and because the
default implementation is surprising, we're promoting this to the
FlutterPlatformNodeDelegate base class.

Issue: flutter/flutter#77838
  • Loading branch information
cbracken authored Dec 10, 2021
1 parent 1ed10bb commit 71fdb89
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 deletions.
4 changes: 4 additions & 0 deletions shell/platform/common/flutter_platform_node_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
// |ui::AXPlatformNodeDelegateBase|
virtual ~FlutterPlatformNodeDelegate() override;

// |ui::AXPlatformNodeDelegateBase|
const ui::AXUniqueId& GetUniqueId() const override { return unique_id_; }

// |ui::AXPlatformNodeDelegateBase|
const ui::AXNodeData& GetData() const override;

Expand Down Expand Up @@ -144,6 +147,7 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
private:
ui::AXNode* ax_node_;
std::weak_ptr<OwnerBridge> bridge_;
ui::AXUniqueId unique_id_;
};

} // namespace flutter
Expand Down
28 changes: 28 additions & 0 deletions shell/platform/common/flutter_platform_node_delegate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,34 @@
namespace flutter {
namespace testing {

TEST(FlutterPlatformNodeDelegateTest, NodeDelegateHasUniqueId) {
TestAccessibilityBridgeDelegate* delegate =
new TestAccessibilityBridgeDelegate();
std::unique_ptr<TestAccessibilityBridgeDelegate> ptr(delegate);
std::shared_ptr<AccessibilityBridge> bridge =
std::make_shared<AccessibilityBridge>(std::move(ptr));

// Add node 0: root.
FlutterSemanticsNode node0{sizeof(FlutterSemanticsNode), 0};
std::vector<int32_t> node0_children{1};
node0.child_count = node0_children.size();
node0.children_in_traversal_order = node0_children.data();
node0.children_in_hit_test_order = node0_children.data();

// Add node 1: text child of node 0.
FlutterSemanticsNode node1{sizeof(FlutterSemanticsNode), 1};
node1.label = "prefecture";
node1.value = "Kyoto";

bridge->AddFlutterSemanticsNodeUpdate(&node0);
bridge->AddFlutterSemanticsNodeUpdate(&node1);
bridge->CommitUpdates();

auto node0_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();
auto node1_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock();
EXPECT_TRUE(node0_delegate->GetUniqueId() != node1_delegate->GetUniqueId());
}

TEST(FlutterPlatformNodeDelegateTest, canPerfomActions) {
TestAccessibilityBridgeDelegate* delegate =
new TestAccessibilityBridgeDelegate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,21 +193,6 @@ TEST(AccessibilityBridgeDelegateWin32, GetParentOnRootRetunsNullptr) {
ASSERT_TRUE(node0_delegate->GetParent() == nullptr);
}

TEST(AccessibilityBridgeDelegateWin32, NodeDelegateHasUniqueId) {
auto window_binding_handler =
std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
FlutterWindowsView view(std::move(window_binding_handler));
view.SetEngine(GetTestEngine());
view.OnUpdateSemanticsEnabled(true);

auto bridge = view.GetEngine()->accessibility_bridge().lock();
PopulateAXTree(bridge);

auto node0_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();
auto node1_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock();
EXPECT_TRUE(node0_delegate->GetUniqueId() != node1_delegate->GetUniqueId());
}

TEST(AccessibilityBridgeDelegateWin32, DispatchAccessibilityAction) {
auto window_binding_handler =
std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
Expand Down
3 changes: 0 additions & 3 deletions shell/platform/windows/flutter_platform_node_delegate_win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ class FlutterPlatformNodeDelegateWin32 : public FlutterPlatformNodeDelegate {
const ui::AXClippingBehavior clipping_behavior,
ui::AXOffscreenResult* offscreen_result) const override;

const ui::AXUniqueId& GetUniqueId() const override { return unique_id_; }

// Dispatches a Windows accessibility event of the specified type, generated
// by the accessibility node associated with this object. This is a
// convenience wrapper around |NotifyWinEvent|.
Expand All @@ -49,7 +47,6 @@ class FlutterPlatformNodeDelegateWin32 : public FlutterPlatformNodeDelegate {
private:
ui::AXPlatformNode* ax_platform_node_;
FlutterWindowsEngine* engine_;
ui::AXUniqueId unique_id_;
};

} // namespace flutter
Expand Down

0 comments on commit 71fdb89

Please sign in to comment.