Commit ceb2bcc6 authored by Mohamed Mansour's avatar Mohamed Mansour Committed by Commit Bot

TableView: Re-implement accessibility support for Screen Readers

Currently TableView constructs the virtual accessibility views by
clearing and creating the entire accessibility tree for every
action (Creating, Adding, Removing, Sorting, Adding Columns, etc).
This introduces a few major problems with screen readers.

For example in UIA supported readers, the TableView is not accessible
at all, when the virtual accessibility views are being destroyed and
created, the narrator looses its context causing the focus to shift to
the parent, causing a flicker like effect and the user cannot announce
any cells. For IAccessible2 supported readers, for low end devices, it
takes time to construct all the virtual accessibility views causing
some cells to not narrate.

So instead of destroying and re-creating all the virtual accessibility
views every time there is a change to the table model or the sorting,
it will add, remove and re-arrange virtual accessibility views on an
individual basis.

and other tables in Chromium's UI.

Bug: 1078623
Change-Id: Ie2de53b4460fed87843b6ce81e90a24194b04c2a
AX-Relnotes: UIA screen readers, such as Narrator, announce all information in the Task Manager
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379091
Commit-Queue: Mohamed Mansour <mmansour@microsoft.com>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813489}
parent 37b9946a
...@@ -59,7 +59,16 @@ ViewAccessibility::~ViewAccessibility() = default; ...@@ -59,7 +59,16 @@ ViewAccessibility::~ViewAccessibility() = default;
void ViewAccessibility::AddVirtualChildView( void ViewAccessibility::AddVirtualChildView(
std::unique_ptr<AXVirtualView> virtual_view) { std::unique_ptr<AXVirtualView> virtual_view) {
AddVirtualChildViewAt(std::move(virtual_view), int{virtual_children_.size()});
}
void ViewAccessibility::AddVirtualChildViewAt(
std::unique_ptr<AXVirtualView> virtual_view,
int index) {
DCHECK(virtual_view); DCHECK(virtual_view);
DCHECK_GE(index, 0);
DCHECK_LE(size_t{index}, virtual_children_.size());
if (virtual_view->parent_view() == this) if (virtual_view->parent_view() == this)
return; return;
DCHECK(!virtual_view->parent_view()) << "This |view| already has a View " DCHECK(!virtual_view->parent_view()) << "This |view| already has a View "
...@@ -69,7 +78,8 @@ void ViewAccessibility::AddVirtualChildView( ...@@ -69,7 +78,8 @@ void ViewAccessibility::AddVirtualChildView(
"AXVirtualView parent. Call " "AXVirtualView parent. Call "
"RemoveChildView first."; "RemoveChildView first.";
virtual_view->set_parent_view(this); virtual_view->set_parent_view(this);
virtual_children_.push_back(std::move(virtual_view)); auto insert_iterator = virtual_children_.begin() + index;
virtual_children_.insert(insert_iterator, std::move(virtual_view));
} }
std::unique_ptr<AXVirtualView> ViewAccessibility::RemoveVirtualChildView( std::unique_ptr<AXVirtualView> ViewAccessibility::RemoveVirtualChildView(
......
...@@ -119,6 +119,11 @@ class VIEWS_EXPORT ViewAccessibility { ...@@ -119,6 +119,11 @@ class VIEWS_EXPORT ViewAccessibility {
// virtual children. // virtual children.
void AddVirtualChildView(std::unique_ptr<AXVirtualView> virtual_view); void AddVirtualChildView(std::unique_ptr<AXVirtualView> virtual_view);
// Adds |virtual_view| as a child of this View at an index.
// We take ownership of our virtual children.
void AddVirtualChildViewAt(std::unique_ptr<AXVirtualView> virtual_view,
int index);
// Removes |virtual_view| from this View. The virtual view's parent will // Removes |virtual_view| from this View. The virtual view's parent will
// change to nullptr. Hands ownership back to the caller. // change to nullptr. Hands ownership back to the caller.
std::unique_ptr<AXVirtualView> RemoveVirtualChildView( std::unique_ptr<AXVirtualView> RemoveVirtualChildView(
......
...@@ -434,11 +434,11 @@ class DerivedTestView : public View { ...@@ -434,11 +434,11 @@ class DerivedTestView : public View {
void OnBlur() override { SetVisible(false); } void OnBlur() override { SetVisible(false); }
}; };
using ViewAccessibilityTest = ViewsTestBase; using AXViewTest = ViewsTestBase;
// Check if the destruction of the widget ends successfully if |view|'s // Check if the destruction of the widget ends successfully if |view|'s
// visibility changed during destruction. // visibility changed during destruction.
TEST_F(ViewAccessibilityTest, LayoutCalledInvalidateRootView) { TEST_F(AXViewTest, LayoutCalledInvalidateRootView) {
// TODO(jamescook): Construct a real AutomationManagerAura rather than using // TODO(jamescook): Construct a real AutomationManagerAura rather than using
// this observer to simulate it. // this observer to simulate it.
AXAuraObjCache cache; AXAuraObjCache cache;
......
This diff is collapsed.
...@@ -49,7 +49,7 @@ class TableHeader; ...@@ -49,7 +49,7 @@ class TableHeader;
class TableViewObserver; class TableViewObserver;
class TableViewTestHelper; class TableViewTestHelper;
// The cells in the first column of a table can contain: // The cell's in the first column of a table can contain:
// - only text // - only text
// - a small icon (16x16) and some text // - a small icon (16x16) and some text
// - a check box and some text // - a check box and some text
...@@ -206,10 +206,13 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -206,10 +206,13 @@ class VIEWS_EXPORT TableView : public views::View,
bool GetSortOnPaint() const; bool GetSortOnPaint() const;
void SetSortOnPaint(bool sort_on_paint); void SetSortOnPaint(bool sort_on_paint);
// Returns the proper ax sort direction.
ax::mojom::SortDirection GetFirstSortDescriptorDirection() const;
TableTypes GetTableType() const; TableTypes GetTableType() const;
// Updates the relative bounds of the virtual accessibility children created // Updates the relative bounds of the virtual accessibility children created
// in UpdateVirtualAccessibilityChildren(). This function is public so that // in RebuildVirtualAccessibilityChildren(). This function is public so that
// the table's |header_| can trigger an update when its visible bounds are // the table's |header_| can trigger an update when its visible bounds are
// changed, because its accessibility information is also contained in the // changed, because its accessibility information is also contained in the
// table's virtual accessibility children. // table's virtual accessibility children.
...@@ -248,7 +251,7 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -248,7 +251,7 @@ class VIEWS_EXPORT TableView : public views::View,
struct GroupSortHelper; struct GroupSortHelper;
struct SortHelper; struct SortHelper;
// Used during painting to determine the range of cells that need to be // Used during painting to determine the range of cell's that need to be
// painted. // painted.
// NOTE: the row indices returned by this are in terms of the view and column // NOTE: the row indices returned by this are in terms of the view and column
// indices in terms of |visible_columns_|. // indices in terms of |visible_columns_|.
...@@ -272,9 +275,6 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -272,9 +275,6 @@ class VIEWS_EXPORT TableView : public views::View,
// in a cell. // in a cell.
int GetCellElementSpacing() const; int GetCellElementSpacing() const;
// Invoked when the number of rows changes in some way.
void NumRowsChanged();
// Does the actual sort and updates the mappings (|view_to_model_| and // Does the actual sort and updates the mappings (|view_to_model_| and
// |model_to_view_|) appropriately. If |schedule_paint| is true, // |model_to_view_|) appropriately. If |schedule_paint| is true,
// schedules a paint. This should be true, unless called from // schedules a paint. This should be true, unless called from
...@@ -308,7 +308,7 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -308,7 +308,7 @@ class VIEWS_EXPORT TableView : public views::View,
// Updates the |x| and |width| of each of the columns in |visible_columns_|. // Updates the |x| and |width| of each of the columns in |visible_columns_|.
void UpdateVisibleColumnSizes(); void UpdateVisibleColumnSizes();
// Returns the cells that need to be painted for the specified region. // Returns the cell's that need to be painted for the specified region.
// |bounds| is in terms of |this|. // |bounds| is in terms of |this|.
PaintRegion GetPaintRegion(const gfx::Rect& bounds) const; PaintRegion GetPaintRegion(const gfx::Rect& bounds) const;
...@@ -353,17 +353,17 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -353,17 +353,17 @@ class VIEWS_EXPORT TableView : public views::View,
// Updates a set of accessibility views that expose the visible table contents // Updates a set of accessibility views that expose the visible table contents
// to assistive software. // to assistive software.
void UpdateVirtualAccessibilityChildren(); void RebuildVirtualAccessibilityChildren();
// Clears the set of accessibility views set up in // Clears the set of accessibility views set up in
// UpdateVirtualAccessibilityChildren(). Useful when the model is in the // RebuildVirtualAccessibilityChildren(). Useful when the model is in the
// process of changing but the virtual accessibility children haven't been // process of changing but the virtual accessibility children haven't been
// updated yet, e.g. showing or hiding a column via SetColumnVisibility(). // updated yet, e.g. showing or hiding a column via SetColumnVisibility().
void ClearVirtualAccessibilityChildren(); void ClearVirtualAccessibilityChildren();
// Helper functions used in UpdateVirtualAccessibilityChildrenBounds() for // Helper functions used in UpdateVirtualAccessibilityChildrenBounds() for
// calculating the accessibility bounds for the header and table rows and // calculating the accessibility bounds for the header and table rows and
// cells. // cell's.
gfx::Rect CalculateHeaderRowAccessibilityBounds() const; gfx::Rect CalculateHeaderRowAccessibilityBounds() const;
gfx::Rect CalculateHeaderCellAccessibilityBounds( gfx::Rect CalculateHeaderCellAccessibilityBounds(
const int visible_column_index) const; const int visible_column_index) const;
...@@ -386,6 +386,37 @@ class VIEWS_EXPORT TableView : public views::View, ...@@ -386,6 +386,37 @@ class VIEWS_EXPORT TableView : public views::View,
// |visible_column_index| indexes into |visible_columns_|. // |visible_column_index| indexes into |visible_columns_|.
AXVirtualView* GetVirtualAccessibilityCell(int row, int visible_column_index); AXVirtualView* GetVirtualAccessibilityCell(int row, int visible_column_index);
// Creates a virtual accessibility view that is used to expose information
// about the row at |view_index| to assistive software.
std::unique_ptr<AXVirtualView> CreateRowAccessibilityView(int view_index);
// Creates a virtual accessibility view that is used to expose information
// about the cell at the provided coordinates |row_index| and |column_index|
// to assistive software.
std::unique_ptr<AXVirtualView> CreateCellAccessibilityView(
int row_index,
size_t column_index);
// Creates a virtual accessibility view that is used to expose information
// about this header to assistive software.
std::unique_ptr<AXVirtualView> CreateHeaderAccessibilityView();
// Updates the accessibility data for |ax_row| to match the data in the view
// at |view_index| in the table. Returns false if row data not changed.
bool UpdateVirtualAccessibilityRowData(AXVirtualView* ax_row,
int view_index,
int model_index);
// The accessibility view |ax_row| callback function that populates the
// accessibility data for a table row.
void PopulateAccessibilityRowData(AXVirtualView* ax_row,
ui::AXNodeData* data);
// The accessibility view |ax_cell| callback function that populates the
// accessibility data for a table cell.
void PopulateAccessibilityCellData(AXVirtualView* ax_cell,
ui::AXNodeData* data);
ui::TableModel* model_ = nullptr; ui::TableModel* model_ = nullptr;
std::vector<ui::TableColumn> columns_; std::vector<ui::TableColumn> columns_;
......
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