Skip to content

GitLab

T
tangled
Commit 96f005bd authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot
Browse files
Options

Fix Windows hit testing from views to web. 

http://crrev.com/c/2102783 fixed a case where hit testing
was getting into an infinite loop, but it broke calling
accHitTest on a View and getting a result within Web.

Fix it by allowing accHitTest to recurse into a different
HWND, and add a new Windows-specific test for that scenario.

Bug: 1061323
Relnotes: N/A
Change-Id: I395bb6d3ccccfd41f80fde68b33852c0e6268f47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113807
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarKevin Babbitt <kbabbitt@microsoft.com>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752752}
parent de9404fb
Hide whitespace changes
Inline Side-by-side
Showing with 102 additions and 4 deletions
content/browser/accessibility/ax_platform_node_win_browsertest.cc
View file @ 96f005bd
......@@ -8,6 +8,7 @@
#include "content/browser/accessibility/accessibility_content_browsertest.h"
#include "content/browser/accessibility/browser_accessibility.h"
#include "content/browser/accessibility/browser_accessibility_com_win.h"
#include "content/browser/renderer_host/render_widget_host_view_aura.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/test/accessibility_notification_waiter.h"
#include "content/public/test/browser_test_utils.h"
......@@ -427,4 +428,84 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeWinBrowserTest,
UIA_AutomationIdPropertyId, scoped_variant.Receive()));
EXPECT_EQ(0, expected_scoped_variant.Compare(scoped_variant));
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeWinBrowserTest,
HitTestOnAncestorOfWebRoot) {
EXPECT_TRUE(NavigateToURL(shell(), GURL(url::kAboutBlankURL)));
// Load the page.
AccessibilityNotificationWaiter waiter(shell()->web_contents(),
ui::kAXModeComplete,
ax::mojom::Event::kLoadComplete);
const char url_str[] =
"data:text/html,"
"<!doctype html>"
"<html><head><title>Accessibility Test</title></head>"
"<body>"
"<button>This is a button</button>"
"</body></html>";
GURL url(url_str);
EXPECT_TRUE(NavigateToURL(shell(), url));
waiter.WaitForNotification();
WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(shell()->web_contents());
BrowserAccessibilityManager* manager =
web_contents->GetRootBrowserAccessibilityManager();
// Find a node to hit test. Note that this is a really simple page,
// so synchronous hit testing will work fine.
BrowserAccessibility* node = manager->GetRoot();
while (node && node->GetRole() != ax::mojom::Role::kButton)
node = manager->NextInTreeOrder(node);
DCHECK(node);
// Get the screen bounds of the hit target and find the point in the middle.
gfx::Rect bounds = node->GetClippedScreenBoundsRect();
gfx::Point point = bounds.CenterPoint();
// Get the root AXPlatformNodeWin.
ui::AXPlatformNodeWin* root_platform_node =
static_cast<ui::AXPlatformNodeWin*>(
ui::AXPlatformNode::FromNativeViewAccessible(
manager->GetRoot()->GetNativeViewAccessible()));
// First test that calling accHitTest on the root node returns the button.
{
base::win::ScopedVariant hit_child_variant;
ASSERT_EQ(S_OK, root_platform_node->accHitTest(
point.x(), point.y(), hit_child_variant.Receive()));
ASSERT_EQ(VT_DISPATCH, hit_child_variant.type());
ASSERT_NE(nullptr, hit_child_variant.ptr());
ComPtr<IAccessible> accessible;
ASSERT_HRESULT_SUCCEEDED(V_DISPATCH(hit_child_variant.ptr())
->QueryInterface(IID_PPV_ARGS(&accessible)));
ui::AXPlatformNode* hit_child =
ui::AXPlatformNode::FromNativeViewAccessible(accessible.Get());
ASSERT_NE(nullptr, hit_child);
EXPECT_EQ(node->GetId(), hit_child->GetDelegate()->GetData().id);
}
// Now test it again, but this time caliing accHitTest on the parent
// IAccessible of the web root node.
{
RenderWidgetHostViewAura* rwhva = static_cast<RenderWidgetHostViewAura*>(
shell()->web_contents()->GetRenderWidgetHostView());
IAccessible* ancestor = rwhva->GetParentNativeViewAccessible();
base::win::ScopedVariant hit_child_variant;
ASSERT_EQ(S_OK, ancestor->accHitTest(point.x(), point.y(),
hit_child_variant.Receive()));
ASSERT_EQ(VT_DISPATCH, hit_child_variant.type());
ASSERT_NE(nullptr, hit_child_variant.ptr());
ComPtr<IAccessible> accessible;
ASSERT_HRESULT_SUCCEEDED(V_DISPATCH(hit_child_variant.ptr())
->QueryInterface(IID_PPV_ARGS(&accessible)));
ui::AXPlatformNode* hit_child =
ui::AXPlatformNode::FromNativeViewAccessible(accessible.Get());
ASSERT_NE(nullptr, hit_child);
EXPECT_EQ(node->GetId(), hit_child->GetDelegate()->GetData().id);
}
}
} // namespace content
ui/accessibility/platform/ax_platform_node_win.cc
View file @ 96f005bd
......@@ -772,17 +772,34 @@ IFACEMETHODIMP AXPlatformNodeWin::accHitTest(LONG x_left,
return S_FALSE;
}
// Check if the hit child is a descendant. If it's not a descendant,
// just ignore the result and stick with the current result.
// Ideally this shouldn't happen - see http://crbug.com/1061323
AXPlatformNode* hit_child_node =
AXPlatformNode::FromNativeViewAccessible(hit_child);
if (!hit_child_node || !hit_child_node->IsDescendantOf(current_result))
if (!hit_child_node)
break;
// If we get the same node, we're done.
if (hit_child_node == current_result)
break;
// Prevent cycles / loops.
//
// This is a workaround for a bug where a hit test in web content might
// return a node that's not a strict descendant. To catch that case
// without disallowing other valid cases of hit testing, add the
// following check:
//
// If the hit child comes from the same HWND, but it's not a descendant,
// just ignore the result and stick with the current result. Note that
// GetTargetForNativeAccessibilityEvent returns a node's owning HWND.
//
// Ideally this shouldn't happen - see http://crbug.com/1061323
bool is_descendant = hit_child_node->IsDescendantOf(current_result);
bool is_same_hwnd =
hit_child_node->GetDelegate()->GetTargetForNativeAccessibilityEvent() ==
current_result->GetDelegate()->GetTargetForNativeAccessibilityEvent();
if (!is_descendant && is_same_hwnd)
break;
// Continue to check recursively. That's because HitTestSync may have
// returned the best result within a particular accessibility tree,
// but we might need to recurse further in a tree of a different type
......
Markdown is supported
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