Skip to content
Snippets Groups Projects
Unverified Commit 38fab638 authored by Jeremy Rose's avatar Jeremy Rose Committed by GitHub
Browse files

chore: cherry-pick 275865e8c237 from chromium (#26470)


* chore: cherry-pick 275865e8c237 from chromium

* resolve conflict

* update patches

Co-authored-by: default avatarJohn Kleinschmidt <jkleinsc@electronjs.org>
Co-authored-by: default avatarElectron Bot <electron@github.com>
parent 1b156c53
No related merge requests found
......@@ -103,3 +103,4 @@ chore_expose_v8_initialization_isolate_callbacks.patch
crashpad-initialize-logging.patch
fix_properly_honor_printing_page_ranges.patch
cherry-pick-8f5a08079948.patch
cherry-pick-275865e8c237.patch
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Martin Robinson <mrobinson@igalia.com>
Date: Tue, 20 Oct 2020 09:00:52 +0000
Subject: Mac: Target a11y actions on certain containers to focused or
activedescendant children
When an action is triggered on a container with selectable children and
that container has a focused or activedescendant child, retarget the
action to that child. This fixes an issue where when the VoiceOver
cursor is on a tree with a focused child, triggered context menus would
use the tree as the target element.
menu on a tree instead of a focused child.
Bug: 1114892
Change-Id: Ibe580beed93aeaac54d1dabaee28443b290fd0e2
AX-Relnotes: Fix an issue where VoiceOver would trigger a context
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466281
Commit-Queue: Martin Robinson <mrobinson@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818831}
diff --git a/content/browser/accessibility/browser_accessibility_cocoa.h b/content/browser/accessibility/browser_accessibility_cocoa.h
index 770c0cb6a4dad8cbf35900311a23a8296407d81d..0ffc8d768299cc5e7f37878fb656c12b028fead9 100644
--- a/content/browser/accessibility/browser_accessibility_cocoa.h
+++ b/content/browser/accessibility/browser_accessibility_cocoa.h
@@ -117,6 +117,14 @@ id AXTextMarkerRangeFrom(id anchor_textmarker, id focus_textmarker);
- (bool)findRowIndex:(BrowserAccessibilityCocoa*)toFind
withCurrentIndex:(int*)currentIndex;
+// Choose the appropriate accessibility object to receive an action depending
+// on the characteristics of this accessibility node.
+- (content::BrowserAccessibility*)actionTarget;
+
+// Return the active descendant for this accessibility object or null if there
+// is no active descendant defined or in the case of an error.
+- (content::BrowserAccessibility*)activeDescendant;
+
// Internally-used property.
@property(nonatomic, readonly) NSPoint origin;
diff --git a/content/browser/accessibility/browser_accessibility_cocoa.mm b/content/browser/accessibility/browser_accessibility_cocoa.mm
index 17aa494d6473866721a8cc5f4c5c646af9307d93..71f927cd35a27dd5f656907dc177fbfe7577d3f0 100644
--- a/content/browser/accessibility/browser_accessibility_cocoa.mm
+++ b/content/browser/accessibility/browser_accessibility_cocoa.mm
@@ -1744,14 +1744,7 @@ - (id)owns {
// supported with VoiceOver.
//
- int activeDescendantId;
- if (!_owner->GetIntAttribute(ax::mojom::IntAttribute::kActivedescendantId,
- &activeDescendantId))
- return nil;
-
- BrowserAccessibilityManager* manager = _owner->manager();
- BrowserAccessibility* activeDescendant =
- manager->GetFromID(activeDescendantId);
+ BrowserAccessibility* activeDescendant = [self activeDescendant];
if (!activeDescendant)
return nil;
@@ -3729,16 +3722,18 @@ - (void)accessibilityPerformAction:(NSString*)action {
return;
// TODO(dmazzoni): Support more actions.
- BrowserAccessibilityManager* manager = _owner->manager();
+ BrowserAccessibility* actionTarget = [self actionTarget];
+ BrowserAccessibilityManager* manager = actionTarget->manager();
if ([action isEqualToString:NSAccessibilityPressAction]) {
- manager->DoDefaultAction(*_owner);
- if (_owner->GetData().GetRestriction() != ax::mojom::Restriction::kNone ||
+ manager->DoDefaultAction(*actionTarget);
+ if (actionTarget->GetData().GetRestriction() !=
+ ax::mojom::Restriction::kNone ||
![self isCheckable])
return;
// Hack: preemptively set the checked state to what it should become,
// otherwise VoiceOver will very likely report the old, incorrect state to
// the user as it requests the value too quickly.
- ui::AXNode* node = _owner->node();
+ ui::AXNode* node = actionTarget->node();
if (!node)
return;
AXNodeData data(node->TakeData()); // Temporarily take data.
@@ -3756,13 +3751,13 @@ - (void)accessibilityPerformAction:(NSString*)action {
}
node->SetData(data); // Set the data back in the node.
} else if ([action isEqualToString:NSAccessibilityShowMenuAction]) {
- manager->ShowContextMenu(*_owner);
+ manager->ShowContextMenu(*actionTarget);
} else if ([action isEqualToString:NSAccessibilityScrollToVisibleAction]) {
- manager->ScrollToMakeVisible(*_owner, gfx::Rect());
+ manager->ScrollToMakeVisible(*actionTarget, gfx::Rect());
} else if ([action isEqualToString:NSAccessibilityIncrementAction]) {
- manager->Increment(*_owner);
+ manager->Increment(*actionTarget);
} else if ([action isEqualToString:NSAccessibilityDecrementAction]) {
- manager->Decrement(*_owner);
+ manager->Decrement(*actionTarget);
}
}
@@ -3871,4 +3866,32 @@ - (BOOL)accessibilityNotifiesWhenDestroyed {
return YES;
}
+// Choose the appropriate accessibility object to receive an action depending
+// on the characteristics of this accessibility node.
+- (BrowserAccessibility*)actionTarget {
+ // When an action is triggered on a container with selectable children and
+ // one of those children is an active descendant or focused, retarget the
+ // action to that child. See https://crbug.com/1114892.
+ if (!ui::IsContainerWithSelectableChildren(_owner->node()->data().role))
+ return _owner;
+
+ if (BrowserAccessibility* activeDescendant = [self activeDescendant])
+ return activeDescendant;
+
+ BrowserAccessibility* focused = _owner->manager()->GetFocus();
+ if (focused && focused->IsDescendantOf(_owner))
+ return focused;
+
+ return _owner;
+}
+
+// Return the active descendant for this accessibility object or null if there
+// is no active descendant defined or in the case of an error.
+- (BrowserAccessibility*)activeDescendant {
+ int activeDescendantId;
+ if (!_owner->GetIntAttribute(ax::mojom::IntAttribute::kActivedescendantId,
+ &activeDescendantId))
+ return nullptr;
+ return _owner->manager()->GetFromID(activeDescendantId);
+}
@end
diff --git a/content/browser/accessibility/browser_accessibility_cocoa_browsertest.mm b/content/browser/accessibility/browser_accessibility_cocoa_browsertest.mm
index 17801b9774813df79e4b944cb0a73535eb4c07c2..6eb2c484ddc479d9cfa566df0b103782264d3af8 100644
--- a/content/browser/accessibility/browser_accessibility_cocoa_browsertest.mm
+++ b/content/browser/accessibility/browser_accessibility_cocoa_browsertest.mm
@@ -44,6 +44,35 @@
return web_contents->GetRootBrowserAccessibilityManager();
}
+ // Trigger a context menu for the provided element without showing it. Returns
+ // the coordinates where the context menu was invoked (calculated based on
+ // the provided element). These coordinates are relative to the RenderView
+ // origin.
+ gfx::Point TriggerContextMenuAndGetMenuLocation(
+ NSAccessibilityElement* element,
+ ContextMenuFilter* filter) {
+ // accessibilityPerformAction is deprecated, but it's still used internally
+ // by AppKit.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ [element accessibilityPerformAction:NSAccessibilityShowMenuAction];
+ filter->Wait();
+
+ UntrustworthyContextMenuParams context_menu_params = filter->get_params();
+ return gfx::Point(context_menu_params.x, context_menu_params.y);
+#pragma clang diagnostic pop
+ }
+
+ void FocusAccessibilityElementAndWaitForFocusChange(
+ NSAccessibilityElement* element) {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ [element accessibilitySetValue:@(1)
+ forAttribute:NSAccessibilityFocusedAttribute];
+#pragma clang diagnostic pop
+ WaitForAccessibilityFocusChange();
+ }
+
private:
BrowserAccessibility* FindNodeInSubtree(BrowserAccessibility& node,
ax::mojom::Role role) {
@@ -466,4 +495,165 @@ GURL url(R"HTML(data:text/html,
EXPECT_NSEQ(@"AXRow", [row_nodes[1] role]);
EXPECT_NSEQ(@"row2", [row_nodes[1] descriptionForAccessibility]);
}
+
+IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
+ TestTreeContextMenuEvent) {
+ AccessibilityNotificationWaiter waiter(shell()->web_contents(),
+ ui::kAXModeComplete,
+ ax::mojom::Event::kLoadComplete);
+
+ GURL url(R"HTML(data:text/html,
+ <div alt="tree" role="tree">
+ <div tabindex="1" role="treeitem">1</div>
+ <div tabindex="2" role="treeitem">2</div>
+ </div>)HTML");
+
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ waiter.WaitForNotification();
+
+ BrowserAccessibility* tree = FindNode(ax::mojom::Role::kTree);
+ base::scoped_nsobject<BrowserAccessibilityCocoa> cocoa_tree(
+ [ToBrowserAccessibilityCocoa(tree) retain]);
+
+ NSArray* tree_children = [cocoa_tree children];
+ EXPECT_NSEQ(@"AXRow", [tree_children[0] role]);
+ EXPECT_NSEQ(@"AXRow", [tree_children[1] role]);
+
+ content::RenderProcessHost* render_process_host =
+ shell()->web_contents()->GetMainFrame()->GetProcess();
+ auto menu_filter = base::MakeRefCounted<ContextMenuFilter>(
+ ContextMenuFilter::ShowBehavior::kPreventShow);
+ render_process_host->AddFilter(menu_filter.get());
+
+ gfx::Point tree_point =
+ TriggerContextMenuAndGetMenuLocation(cocoa_tree, menu_filter.get());
+
+ menu_filter->Reset();
+ gfx::Point item_1_point =
+ TriggerContextMenuAndGetMenuLocation(tree_children[1], menu_filter.get());
+ ASSERT_NE(tree_point, item_1_point);
+
+ // Now focus the second child and trigger a context menu on the tree.
+ EXPECT_TRUE(
+ content::ExecuteScript(shell()->web_contents(),
+ "document.body.children[0].children[1].focus();"));
+ WaitForAccessibilityFocusChange();
+
+ // Triggering a context menu on the tree should now trigger the menu
+ // on the focused child.
+ menu_filter->Reset();
+ gfx::Point new_point =
+ TriggerContextMenuAndGetMenuLocation(cocoa_tree, menu_filter.get());
+ ASSERT_EQ(new_point, item_1_point);
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
+ TestEventRetargetingFocus) {
+ AccessibilityNotificationWaiter waiter(shell()->web_contents(),
+ ui::kAXModeComplete,
+ ax::mojom::Event::kLoadComplete);
+
+ GURL url(R"HTML(data:text/html,
+ <div role="tree">
+ <div tabindex="1" role="treeitem">1</div>
+ <div tabindex="2" role="treeitem">2</div>
+ </div>
+ <div role="treegrid">
+ <div tabindex="1" role="treeitem">1</div>
+ <div tabindex="2" role="treeitem">2</div>
+ </div>
+ <div role="tablist">
+ <div tabindex="1" role="tab">1</div>
+ <div tabindex="2" role="tab">2</div>
+ </div>
+ <div role="table">
+ <div tabindex="1" role="row">1</div>
+ <div tabindex="2" role="row">2</div>
+ </div>
+ <div role="banner">
+ <div tabindex="1" role="link">1</div>
+ <div tabindex="2" role="link">2</div>
+ </div>)HTML");
+
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ waiter.WaitForNotification();
+
+ std::pair<ax::mojom::Role, bool> tests[] = {
+ std::make_pair(ax::mojom::Role::kTree, true),
+ std::make_pair(ax::mojom::Role::kTreeGrid, true),
+ std::make_pair(ax::mojom::Role::kTabList, true),
+ std::make_pair(ax::mojom::Role::kTable, false),
+ std::make_pair(ax::mojom::Role::kBanner, false),
+ };
+
+ for (auto& test : tests) {
+ base::scoped_nsobject<BrowserAccessibilityCocoa> parent(
+ [ToBrowserAccessibilityCocoa(FindNode(test.first)) retain]);
+ BrowserAccessibilityCocoa* child = [parent children][1];
+
+ EXPECT_NE(nullptr, parent.get());
+ EXPECT_EQ([child owner], [child actionTarget]);
+ EXPECT_EQ([parent owner], [parent actionTarget]);
+
+ FocusAccessibilityElementAndWaitForFocusChange(child);
+ ASSERT_EQ(test.second, [parent actionTarget] == [child owner]);
+ }
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
+ TestEventRetargetingActiveDescendant) {
+ AccessibilityNotificationWaiter waiter(shell()->web_contents(),
+ ui::kAXModeComplete,
+ ax::mojom::Event::kLoadComplete);
+
+ GURL url(R"HTML(data:text/html,
+ <div role="tree" aria-activedescendant="tree-child">
+ <div tabindex="1" role="treeitem">1</div>
+ <div id="tree-child" tabindex="2" role="treeitem">2</div>
+ </div>
+ <div role="treegrid" aria-activedescendant="treegrid-child">
+ <div tabindex="1" role="treeitem">1</div>
+ <div id="treegrid-child" tabindex="2" role="treeitem">2</div>
+ </div>
+ <div role="tablist" aria-activedescendant="tablist-child">
+ <div tabindex="1" role="tab">1</div>
+ <div id="tablist-child" tabindex="2" role="tab">2</div>
+ </div>
+ <div role="table" aria-activedescendant="table-child">
+ <div tabindex="1" role="row">1</div>
+ <div id="table-child" tabindex="2" role="row">2</div>
+ </div>
+ <div role="banner" aria-activedescendant="banner-child">
+ <div tabindex="1" role="link">1</div>
+ <div id="banner-child" tabindex="2" role="link">2</div>
+ </div>)HTML");
+
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+ waiter.WaitForNotification();
+
+ std::pair<ax::mojom::Role, bool> tests[] = {
+ std::make_pair(ax::mojom::Role::kTree, true),
+ std::make_pair(ax::mojom::Role::kTreeGrid, true),
+ std::make_pair(ax::mojom::Role::kTabList, true),
+ std::make_pair(ax::mojom::Role::kTable, false),
+ std::make_pair(ax::mojom::Role::kBanner, false),
+ };
+
+ for (auto& test : tests) {
+ base::scoped_nsobject<BrowserAccessibilityCocoa> parent(
+ [ToBrowserAccessibilityCocoa(FindNode(test.first)) retain]);
+ BrowserAccessibilityCocoa* first_child = [parent children][0];
+ BrowserAccessibilityCocoa* second_child = [parent children][1];
+
+ EXPECT_NE(nullptr, parent.get());
+ EXPECT_EQ([second_child owner], [second_child actionTarget]);
+ EXPECT_EQ(test.second, [second_child owner] == [parent actionTarget]);
+
+ // aria-activedescendant should take priority of focus for determining
+ // if an object is the action target.
+ FocusAccessibilityElementAndWaitForFocusChange(first_child);
+ EXPECT_EQ(test.second, [second_child owner] == [parent actionTarget]);
+ }
+}
+
} // namespace content
diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc
index 47161f98b2fcdacece578cde50141860d2ab0a40..a8f988b8465c420224979d805baf454483e49134 100644
--- a/content/public/test/browser_test_utils.cc
+++ b/content/public/test/browser_test_utils.cc
@@ -3220,10 +3220,11 @@ void VerifyStaleContentOnFrameEviction(
#endif // defined(USE_AURA)
-ContextMenuFilter::ContextMenuFilter()
+ContextMenuFilter::ContextMenuFilter(ShowBehavior behavior)
: BrowserMessageFilter(FrameMsgStart),
run_loop_(std::make_unique<base::RunLoop>()),
- quit_closure_(run_loop_->QuitClosure()) {}
+ quit_closure_(run_loop_->QuitClosure()),
+ show_behavior_(behavior) {}
bool ContextMenuFilter::OnMessageReceived(const IPC::Message& message) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -3234,6 +3235,9 @@ bool ContextMenuFilter::OnMessageReceived(const IPC::Message& message) {
GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&ContextMenuFilter::OnContextMenu, this, menu_params));
+ // Returning true here blocks the default action for this message, which
+ // means that the menu will not be shown.
+ return show_behavior_ == ShowBehavior::kPreventShow;
}
return false;
}
@@ -3244,6 +3248,12 @@ void ContextMenuFilter::Wait() {
run_loop_ = nullptr;
}
+void ContextMenuFilter::Reset() {
+ ASSERT_EQ(run_loop_, nullptr);
+ run_loop_ = std::make_unique<base::RunLoop>();
+ quit_closure_ = run_loop_->QuitClosure();
+}
+
ContextMenuFilter::~ContextMenuFilter() = default;
void ContextMenuFilter::OnContextMenu(
diff --git a/content/public/test/browser_test_utils.h b/content/public/test/browser_test_utils.h
index f68d423999ff711b46b7fe9d718163158e1d5223..3568bcceee9867db634306d36e3281d32450ea0a 100644
--- a/content/public/test/browser_test_utils.h
+++ b/content/public/test/browser_test_utils.h
@@ -1688,10 +1688,15 @@ void VerifyStaleContentOnFrameEviction(
// sent by the renderer.
class ContextMenuFilter : public content::BrowserMessageFilter {
public:
- ContextMenuFilter();
+ // Whether or not the ContextMenu should be prevented from performing
+ // its default action, preventing the context menu from showing.
+ enum ShowBehavior { kShow, kPreventShow };
+
+ explicit ContextMenuFilter(ShowBehavior behavior = ShowBehavior::kShow);
bool OnMessageReceived(const IPC::Message& message) override;
void Wait();
+ void Reset();
content::UntrustworthyContextMenuParams get_params() { return last_params_; }
@@ -1703,6 +1708,7 @@ class ContextMenuFilter : public content::BrowserMessageFilter {
std::unique_ptr<base::RunLoop> run_loop_;
base::OnceClosure quit_closure_;
content::UntrustworthyContextMenuParams last_params_;
+ const ShowBehavior show_behavior_;
DISALLOW_COPY_AND_ASSIGN(ContextMenuFilter);
};
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment