Commit a11f636a authored by Hiroki Sato's avatar Hiroki Sato Committed by Commit Bot

Fix ARC a11y bounds computation

AXRelativeBounds assumes that the bounds are relative to the root of the
tree by default, but in ARC, every nodes' bounds are computed relative
to its container.

AutomationInternalCustomBindings::ComputeGlobalNodeBounds computes the
bounds by first calling AXTree::RelativeToTreeBoundsInternal to compute
the node's relative bounds to its root using AXRelativeBounds, and then
applying the difference between the root and the container.

In Dialog or Popup window, the root node can have some amount of offset
to its container. As a result, every computed bounds in such window
has "doubled" offset to ARC container.

This CL fixes the computation.

This CL also simplifies geometry_util in arc a11y because
exo::WMHelper::GetInstance() never returns nullptr.

Bug: b:145260260
Test: manual. With TalkBack-test app, Alerts window has correct bounds.
Change-Id: I93033ad0b2771828cdf00d17d1ebe039ef1d7044
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2061930
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743386}
parent 6bd49e8a
...@@ -39,7 +39,7 @@ void AccessibilityInfoDataWrapper::Serialize(ui::AXNodeData* out_data) const { ...@@ -39,7 +39,7 @@ void AccessibilityInfoDataWrapper::Serialize(ui::AXNodeData* out_data) const {
bounds.width(), bounds.height()); bounds.width(), bounds.height());
} }
// TODO(katie): Try using offset_container_id to make bounds calculations // TODO(hirokisato): Try using offset_container_id to make bounds calculations
// more efficient. If this is the child of the root, set the // more efficient. If this is the child of the root, set the
// offset_container_id to be the root. Otherwise, set it to the first node // offset_container_id to be the root. Otherwise, set it to the first node
// child of the root. // child of the root.
......
...@@ -54,16 +54,13 @@ void DispatchFocusChange(arc::mojom::AccessibilityNodeInfoData* node_data, ...@@ -54,16 +54,13 @@ void DispatchFocusChange(arc::mojom::AccessibilityNodeInfoData* node_data,
accessibility_manager->profile() != profile) accessibility_manager->profile() != profile)
return; return;
exo::WMHelper* wm_helper = exo::WMHelper::GetInstance(); DCHECK(exo::WMHelper::HasInstance());
if (!wm_helper) aura::Window* active_window = exo::WMHelper::GetInstance()->GetActiveWindow();
return;
aura::Window* active_window = wm_helper->GetActiveWindow();
if (!active_window) if (!active_window)
return; return;
gfx::Rect bounds_in_screen = gfx::ToEnclosingRect(arc::ToChromeBounds( gfx::Rect bounds_in_screen = gfx::ToEnclosingRect(arc::ToChromeBounds(
node_data->bounds_in_screen, wm_helper, node_data->bounds_in_screen,
views::Widget::GetWidgetForNativeView(active_window))); views::Widget::GetWidgetForNativeView(active_window)));
accessibility_manager->OnViewFocusedInArc(bounds_in_screen); accessibility_manager->OnViewFocusedInArc(bounds_in_screen);
...@@ -621,15 +618,12 @@ ArcAccessibilityHelperBridge::OnGetTextLocationDataResultInternal( ...@@ -621,15 +618,12 @@ ArcAccessibilityHelperBridge::OnGetTextLocationDataResultInternal(
if (!result_rect) if (!result_rect)
return base::nullopt; return base::nullopt;
exo::WMHelper* wm_helper = exo::WMHelper::GetInstance(); DCHECK(exo::WMHelper::HasInstance());
if (!wm_helper) aura::Window* active_window = exo::WMHelper::GetInstance()->GetActiveWindow();
return base::nullopt;
aura::Window* active_window = wm_helper->GetActiveWindow();
if (!active_window) if (!active_window)
return base::nullopt; return base::nullopt;
gfx::RectF rect_f = arc::ToChromeScale(*result_rect, wm_helper); gfx::RectF rect_f = arc::ToChromeScale(*result_rect);
arc::ScaleDeviceFactor(rect_f, active_window->GetToplevelWindow()); arc::ScaleDeviceFactor(rect_f, active_window->GetToplevelWindow());
return gfx::ToEnclosingRect(rect_f); return gfx::ToEnclosingRect(rect_f);
} }
......
...@@ -259,30 +259,33 @@ const gfx::Rect AXTreeSourceArc::GetBounds( ...@@ -259,30 +259,33 @@ const gfx::Rect AXTreeSourceArc::GetBounds(
gfx::Rect info_data_bounds = info_data->GetBounds(); gfx::Rect info_data_bounds = info_data->GetBounds();
if (!active_window) { if (!active_window) {
gfx::Rect root_bounds = GetRoot()->GetBounds(); const gfx::Rect root_bounds = GetRoot()->GetBounds();
info_data_bounds.Offset(-1 * root_bounds.x(), -1 * root_bounds.y()); info_data_bounds.Offset(-1 * root_bounds.x(), -1 * root_bounds.y());
return info_data_bounds; return info_data_bounds;
} }
exo::WMHelper* wm_helper = exo::WMHelper::GetInstance(); // By default, the node bounds is relative to the tree root.
if (!wm_helper) if (info_data->GetId() != root_id_) {
return info_data_bounds; const gfx::Rect root_bounds = GetRoot()->GetBounds();
info_data_bounds.Offset(-1 * root_bounds.x(), -1 * root_bounds.y());
gfx::RectF info_data_bounds_f = ToChromeScale(info_data_bounds);
arc::ScaleDeviceFactor(info_data_bounds_f,
active_window->GetToplevelWindow());
return gfx::ToEnclosingRect(info_data_bounds_f);
}
// For the root node, the node bounds is relative to its container View.
views::Widget* widget = views::Widget::GetWidgetForNativeView(active_window); views::Widget* widget = views::Widget::GetWidgetForNativeView(active_window);
DCHECK(widget); DCHECK(widget);
gfx::RectF info_data_bounds_f = gfx::RectF info_data_bounds_f = arc::ToChromeBounds(info_data_bounds, widget);
arc::ToChromeBounds(info_data_bounds, wm_helper, widget);
// TODO(katie): offset_container_id should work and we shouldn't have to
// go into this code path for each node.
DCHECK(widget->widget_delegate()); DCHECK(widget->widget_delegate());
DCHECK(widget->widget_delegate()->GetContentsView()); DCHECK(widget->widget_delegate()->GetContentsView());
const gfx::Rect root_bounds = const gfx::Rect root_bounds =
widget->widget_delegate()->GetContentsView()->GetBoundsInScreen(); widget->widget_delegate()->GetContentsView()->GetBoundsInScreen();
// Bounds of root node is relative to its container, i.e. contents view
// (ShellSurfaceBase).
info_data_bounds_f.Offset(-1 * root_bounds.x(), -1 * root_bounds.y()); info_data_bounds_f.Offset(-1 * root_bounds.x(), -1 * root_bounds.y());
arc::ScaleDeviceFactor(info_data_bounds_f, arc::ScaleDeviceFactor(info_data_bounds_f,
......
...@@ -12,19 +12,17 @@ ...@@ -12,19 +12,17 @@
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
namespace arc { namespace arc {
gfx::RectF ToChromeScale(const gfx::Rect& bounds, exo::WMHelper* wm_helper) { gfx::RectF ToChromeScale(const gfx::Rect& bounds) {
DCHECK(wm_helper); DCHECK(exo::WMHelper::HasInstance());
gfx::RectF bounds_f(bounds); gfx::RectF bounds_f(bounds);
bounds_f.Scale(1.0f / wm_helper->GetDefaultDeviceScaleFactor()); bounds_f.Scale(1.0f /
exo::WMHelper::GetInstance()->GetDefaultDeviceScaleFactor());
return bounds_f; return bounds_f;
} }
gfx::RectF ToChromeBounds(const gfx::Rect& bounds, gfx::RectF ToChromeBounds(const gfx::Rect& bounds, views::Widget* widget) {
exo::WMHelper* wm_helper,
views::Widget* widget) {
DCHECK(wm_helper);
DCHECK(widget); DCHECK(widget);
gfx::RectF chrome_bounds = ToChromeScale(bounds, wm_helper); gfx::RectF chrome_bounds = ToChromeScale(bounds);
// On Android side, content is rendered without considering height of // On Android side, content is rendered without considering height of
// caption bar, e.g. Content is rendered at y:0 instead of y:32 where 32 is // caption bar, e.g. Content is rendered at y:0 instead of y:32 where 32 is
......
...@@ -11,10 +11,6 @@ namespace aura { ...@@ -11,10 +11,6 @@ namespace aura {
class Window; class Window;
} }
namespace exo {
class WMHelper;
}
namespace gfx { namespace gfx {
class RectF; class RectF;
} }
...@@ -26,13 +22,11 @@ class Widget; ...@@ -26,13 +22,11 @@ class Widget;
namespace arc { namespace arc {
// Given ARC pixels, returns DIPs in Chrome OS main display. // Given ARC pixels, returns DIPs in Chrome OS main display.
// This function only scales the bounds. // This function only scales the bounds.
gfx::RectF ToChromeScale(const gfx::Rect& rect, exo::WMHelper* wm_helper); gfx::RectF ToChromeScale(const gfx::Rect& rect);
// Given ARC pixels in screen coordinate, returns DIPs in Chrome OS main // Given ARC pixels in screen coordinate, returns DIPs in Chrome OS main
// display. This function adjusts differences between ARC and Chrome. // display. This function adjusts differences between ARC and Chrome.
gfx::RectF ToChromeBounds(const gfx::Rect& rect, gfx::RectF ToChromeBounds(const gfx::Rect& rect, views::Widget* widget);
exo::WMHelper* wm_helper,
views::Widget* widget);
// Given DIPs in Chrome OS main display, scales it into pixels. // Given DIPs in Chrome OS main display, scales it into pixels.
void ScaleDeviceFactor(gfx::RectF& rect, aura::Window* toplevel_window); void ScaleDeviceFactor(gfx::RectF& rect, aura::Window* toplevel_window);
......
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