From 15f9811ca238354c65263347014c726261589d59 Mon Sep 17 00:00:00 2001 From: schectman Date: Wed, 4 Jan 2023 10:38:19 -0500 Subject: [PATCH 1/6] Limit selection change to focused node on Windows --- .../platform/common/accessibility_bridge_unittests.cc | 4 +++- shell/platform/windows/accessibility_bridge_windows.cc | 10 ++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/shell/platform/common/accessibility_bridge_unittests.cc b/shell/platform/common/accessibility_bridge_unittests.cc index e6a72f98d4d21..0aa078f925f5d 100644 --- a/shell/platform/common/accessibility_bridge_unittests.cc +++ b/shell/platform/common/accessibility_bridge_unittests.cc @@ -152,7 +152,9 @@ TEST(AccessibilityBridgeTest, canHandleSelectionChangeCorrectly) { std::shared_ptr bridge = std::make_shared(); FlutterSemanticsNode root = CreateSemanticsNode(0, "root"); - root.flags = FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField; + root.flags = static_cast( + FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField | + FlutterSemanticsFlag::kFlutterSemanticsFlagIsFocused); bridge->AddFlutterSemanticsNodeUpdate(&root); bridge->CommitUpdates(); diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index 83435ce3a41b1..eebc26bbc7272 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -41,10 +41,16 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( DispatchWinAccessibilityEvent(win_delegate, ax::mojom::Event::kChildrenChanged); break; - case ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED: + case ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED: { + ui::AXNode::AXID focus_id = GetAXTreeData().sel_focus_object_id; + auto focus_delegate = + GetFlutterPlatformNodeDelegateFromID(focus_id).lock(); DispatchWinAccessibilityEvent( - win_delegate, ax::mojom::Event::kDocumentSelectionChanged); + std::static_pointer_cast( + focus_delegate), + ax::mojom::Event::kDocumentSelectionChanged); break; + } case ui::AXEventGenerator::Event::FOCUS_CHANGED: DispatchWinAccessibilityEvent(win_delegate, ax::mojom::Event::kFocus); SetFocus(win_delegate); From 6ff2b0fcfc51309b120339d4da3fe31772622269 Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 5 Jan 2023 09:29:16 -0500 Subject: [PATCH 2/6] Focus fix --- shell/platform/common/accessibility_bridge.cc | 5 +++-- .../platform/windows/accessibility_bridge_windows.cc | 12 ++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/shell/platform/common/accessibility_bridge.cc b/shell/platform/common/accessibility_bridge.cc index 4b1ea03563815..8a98f3d724b4c 100644 --- a/shell/platform/common/accessibility_bridge.cc +++ b/shell/platform/common/accessibility_bridge.cc @@ -527,11 +527,12 @@ void AccessibilityBridge::SetTooltipFromFlutterUpdate( void AccessibilityBridge::SetTreeData(const SemanticsNode& node, ui::AXTreeUpdate& tree_update) { FlutterSemanticsFlag flags = node.flags; - // Set selection if: + // Set selection of the focused node if: // 1. this text field has a valid selection // 2. this text field doesn't have a valid selection but had selection stored // in the tree. - if (flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField) { + if (flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsTextField && + flags & FlutterSemanticsFlag::kFlutterSemanticsFlagIsFocused) { if (node.text_selection_base != -1) { tree_update.tree_data.sel_anchor_object_id = node.id; tree_update.tree_data.sel_anchor_offset = node.text_selection_base; diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index eebc26bbc7272..8bc1924faa538 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -42,13 +42,17 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( ax::mojom::Event::kChildrenChanged); break; case ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED: { + // An event indicating a change in document selection should be fired + // only for the focused node whose selection has changed. ui::AXNode::AXID focus_id = GetAXTreeData().sel_focus_object_id; auto focus_delegate = GetFlutterPlatformNodeDelegateFromID(focus_id).lock(); - DispatchWinAccessibilityEvent( - std::static_pointer_cast( - focus_delegate), - ax::mojom::Event::kDocumentSelectionChanged); + if (focus_delegate) { + DispatchWinAccessibilityEvent( + std::static_pointer_cast( + focus_delegate), + ax::mojom::Event::kDocumentSelectionChanged); + } break; } case ui::AXEventGenerator::Event::FOCUS_CHANGED: From 22a7a5203c742ce4568e7e69b97401f4f73dec31 Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 5 Jan 2023 13:29:56 -0500 Subject: [PATCH 3/6] Test document selection change --- shell/platform/windows/accessibility_bridge_windows.cc | 9 +++++---- .../windows/accessibility_bridge_windows_unittests.cc | 6 ++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index 8bc1924faa538..3c37707f3fc35 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -47,12 +47,13 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( ui::AXNode::AXID focus_id = GetAXTreeData().sel_focus_object_id; auto focus_delegate = GetFlutterPlatformNodeDelegateFromID(focus_id).lock(); - if (focus_delegate) { - DispatchWinAccessibilityEvent( + if (!focus_delegate) { + win_delegate = std::static_pointer_cast( - focus_delegate), - ax::mojom::Event::kDocumentSelectionChanged); + focus_delegate); } + DispatchWinAccessibilityEvent(win_delegate, + ax::mojom::Event::kDocumentSelectionChanged); break; } case ui::AXEventGenerator::Event::FOCUS_CHANGED: diff --git a/shell/platform/windows/accessibility_bridge_windows_unittests.cc b/shell/platform/windows/accessibility_bridge_windows_unittests.cc index e3cc3b58a0f98..ecada69ff8d80 100644 --- a/shell/platform/windows/accessibility_bridge_windows_unittests.cc +++ b/shell/platform/windows/accessibility_bridge_windows_unittests.cc @@ -338,5 +338,11 @@ TEST(AccessibilityBridgeWindows, OnAccessibilityStateChanged) { ax::mojom::Event::kStateChanged); } +TEST(AccessibilityBridgeWindows, OnDocumentSelectionChanged) { + ExpectWinEventFromAXEvent( + 1, ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED, + ax::mojom::Event::kDocumentSelectionChanged); +} + } // namespace testing } // namespace flutter From 53a07d8bca2ef01b12dca34a03ea84a1f9410120 Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 5 Jan 2023 13:31:52 -0500 Subject: [PATCH 4/6] Comment --- shell/platform/windows/accessibility_bridge_windows.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index 3c37707f3fc35..079b146913ffe 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -43,7 +43,9 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( break; case ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED: { // An event indicating a change in document selection should be fired - // only for the focused node whose selection has changed. + // only for the focused node whose selection has changed. If a valid + // carat and selection exist in the app tree, they must both be within + // the focus node. ui::AXNode::AXID focus_id = GetAXTreeData().sel_focus_object_id; auto focus_delegate = GetFlutterPlatformNodeDelegateFromID(focus_id).lock(); From ea0ccec013c53b806beb5d4f5971a50944abe30b Mon Sep 17 00:00:00 2001 From: schectman Date: Thu, 5 Jan 2023 13:44:58 -0500 Subject: [PATCH 5/6] Formatting --- shell/platform/windows/accessibility_bridge_windows.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index 079b146913ffe..a531298dcb36c 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -54,8 +54,8 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( std::static_pointer_cast( focus_delegate); } - DispatchWinAccessibilityEvent(win_delegate, - ax::mojom::Event::kDocumentSelectionChanged); + DispatchWinAccessibilityEvent( + win_delegate, ax::mojom::Event::kDocumentSelectionChanged); break; } case ui::AXEventGenerator::Event::FOCUS_CHANGED: From 1fd98a60ec0b9d920326c6da1db553911585a93b Mon Sep 17 00:00:00 2001 From: yaakovschectman <109111084+yaakovschectman@users.noreply.github.com> Date: Thu, 5 Jan 2023 17:30:18 -0500 Subject: [PATCH 6/6] Update shell/platform/windows/accessibility_bridge_windows.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com> --- shell/platform/windows/accessibility_bridge_windows.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/windows/accessibility_bridge_windows.cc b/shell/platform/windows/accessibility_bridge_windows.cc index a531298dcb36c..d5b252ae8d43a 100644 --- a/shell/platform/windows/accessibility_bridge_windows.cc +++ b/shell/platform/windows/accessibility_bridge_windows.cc @@ -44,7 +44,7 @@ void AccessibilityBridgeWindows::OnAccessibilityEvent( case ui::AXEventGenerator::Event::DOCUMENT_SELECTION_CHANGED: { // An event indicating a change in document selection should be fired // only for the focused node whose selection has changed. If a valid - // carat and selection exist in the app tree, they must both be within + // caret and selection exist in the app tree, they must both be within // the focus node. ui::AXNode::AXID focus_id = GetAXTreeData().sel_focus_object_id; auto focus_delegate =