Skip to content

Commit

Permalink
[UIA] Don't expose the TableProvider for layout tables
Browse files Browse the repository at this point in the history
On UIA, we don't want to expose the ITableProvider for layout tables.
We can't simply rely on the kExposeLayoutTableAsDataTable across
platform code because it needs to stay true on Windows for IA2.

Bug: 1323027
Change-Id: Id4d48e8d15603fbf4e9bd34580a75154e9e8dc0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3630564
Reviewed-by: Daniel Libby <dlibby@microsoft.com>
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1001838}
  • Loading branch information
bebeaudr authored and Chromium LUCI CQ committed May 11, 2022
1 parent 66a32c8 commit bf19c18
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
Document
++Table Grid.ColumnCount=2 Grid.RowCount=1 Table.RowOrColumnMajor='RowMajor'
++++DataItem IsControlElement=false
++++++DataItem Name='1.' GridItem.Column=0 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++Text Name='1.' IsControlElement=false
++++++DataItem Name='2.' GridItem.Column=1 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++Text Name='2.' IsControlElement=false
++Group IsControlElement=false
++++Group IsControlElement=false
++++++Group Name='1.'
++++++++Text Name='1.'
++++++Group Name='2.'
++++++++Text Name='2.'
++Group Name='3. 4.'
++++Table Grid.ColumnCount=2 Grid.RowCount=1 Table.RowOrColumnMajor='RowMajor'
++++++DataItem IsControlElement=false
++++++++DataItem Name='3.' GridItem.Column=0 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++++Text Name='3.' IsControlElement=false
++++++++DataItem Name='4.' GridItem.Column=1 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++++Text Name='4.' IsControlElement=false
++++Group IsControlElement=false
++++++Group IsControlElement=false
++++++++Group Name='3.'
++++++++++Text Name='3.'
++++++++Group Name='4.'
++++++++++Text Name='4.'
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
Document Name='Table example #2'
++Table Grid.ColumnCount=3 Grid.RowCount=3 Table.RowOrColumnMajor='RowMajor'
++++DataItem IsControlElement=false
++++++DataItem Name='1' GridItem.Column=0 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++Text Name='1' IsControlElement=false
++++++DataItem Name='2' GridItem.Column=1 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++Text Name='2' IsControlElement=false
++++++DataItem Name='3' GridItem.Column=2 GridItem.ColumnSpan=1 GridItem.Row=0 GridItem.RowSpan=1
++++++++Text Name='3' IsControlElement=false
++++DataItem IsControlElement=false
++++++DataItem Name='4' GridItem.Column=0 GridItem.ColumnSpan=1 GridItem.Row=1 GridItem.RowSpan=1
++++++++Text Name='4' IsControlElement=false
++++++DataItem Name='5' GridItem.Column=1 GridItem.ColumnSpan=1 GridItem.Row=1 GridItem.RowSpan=1
++++++++Text Name='5' IsControlElement=false
++++++DataItem Name='6' GridItem.Column=2 GridItem.ColumnSpan=1 GridItem.Row=1 GridItem.RowSpan=1
++++++++Text Name='6' IsControlElement=false
++++DataItem IsControlElement=false
++++++DataItem Name='7' GridItem.Column=0 GridItem.ColumnSpan=1 GridItem.Row=2 GridItem.RowSpan=1
++++++++Text Name='7' IsControlElement=false
++++++DataItem Name='8' GridItem.Column=1 GridItem.ColumnSpan=1 GridItem.Row=2 GridItem.RowSpan=1
++++++++Text Name='8' IsControlElement=false
++++++DataItem Name='9' GridItem.Column=2 GridItem.ColumnSpan=1 GridItem.Row=2 GridItem.RowSpan=1
++++++++Text Name='9' IsControlElement=false
++Group IsControlElement=false
++++Group IsControlElement=false
++++++Group Name='1'
++++++++Text Name='1'
++++++Group Name='2'
++++++++Text Name='2'
++++++Group Name='3'
++++++++Text Name='3'
++++Group IsControlElement=false
++++++Group Name='4'
++++++++Text Name='4'
++++++Group Name='5'
++++++++Text Name='5'
++++++Group Name='6'
++++++++Text Name='6'
++++Group IsControlElement=false
++++++Group Name='7'
++++++++Text Name='7'
++++++Group Name='8'
++++++++Text Name='8'
++++++Group Name='9'
++++++++Text Name='9'
14 changes: 14 additions & 0 deletions ui/accessibility/ax_role_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,20 @@ bool IsUIAEmbeddedObject(ax::mojom::Role role) {
}
}

bool IsUIATableLike(ax::mojom::Role role) {
if (role == ax::mojom::Role::kLayoutTable)
return false;

return IsTableLike(role);
}

bool IsUIACellOrTableHeader(ax::mojom::Role role) {
if (role == ax::mojom::Role::kLayoutTableCell)
return false;

return IsCellOrTableHeader(role);
}

bool IsWindow(ax::mojom::Role role) {
switch (role) {
case ax::mojom::Role::kWindow:
Expand Down
7 changes: 7 additions & 0 deletions ui/accessibility/ax_role_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@ AX_BASE_EXPORT bool IsTextField(ax::mojom::Role role);
// objects. See the method definition for more details.
AX_BASE_EXPORT bool IsUIAEmbeddedObject(ax::mojom::Role role);

// Returns false if |role| is a layout table, or whatever `IsTableLike` returns.
AX_BASE_EXPORT bool IsUIATableLike(ax::mojom::Role role);

// Returns false if |role| is a layout table cell, or whatever
// `IsCellOrTableHeader` returns.
AX_BASE_EXPORT bool IsUIACellOrTableHeader(ax::mojom::Role role);

// Returns true if the provided role represents a window.
AX_BASE_EXPORT bool IsWindow(const ax::mojom::Role role);

Expand Down
31 changes: 21 additions & 10 deletions ui/accessibility/platform/ax_platform_node_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1060,15 +1060,15 @@ AXPlatformNodeWin::UIARoleProperties AXPlatformNodeWin::GetUIARoleProperties() {

case ax::mojom::Role::kLayoutTable:
return {UIALocalizationStrategy::kDeferToControlType,
UIA_TableControlTypeId, L"grid"};
UIA_GroupControlTypeId, L"group"};

case ax::mojom::Role::kLayoutTableCell:
return {UIALocalizationStrategy::kDeferToControlType,
UIA_DataItemControlTypeId, L"gridcell"};
UIA_GroupControlTypeId, L"group"};

case ax::mojom::Role::kLayoutTableRow:
return {UIALocalizationStrategy::kDeferToAriaRole,
UIA_DataItemControlTypeId, L"row"};
return {UIALocalizationStrategy::kDeferToAriaRole, UIA_GroupControlTypeId,
L"group"};

case ax::mojom::Role::kLink:
return {UIALocalizationStrategy::kDeferToControlType,
Expand Down Expand Up @@ -7069,7 +7069,7 @@ bool AXPlatformNodeWin::IsUIAControl() const {
// text to be effectively repeated.
auto* ancestor = FromNativeViewAccessible(GetDelegate()->GetParent());
while (ancestor) {
if (IsCellOrTableHeader(ancestor->GetRole()))
if (IsUIACellOrTableHeader(ancestor->GetRole()))
return false;
switch (ancestor->GetRole()) {
case ax::mojom::Role::kListItem:
Expand Down Expand Up @@ -7111,7 +7111,7 @@ bool AXPlatformNodeWin::IsUIAControl() const {
// The control view also includes noninteractive UI items that contribute
// to the logical structure of the UI.
if (IsControl(GetRole()) || ComputeUIALandmarkType() ||
IsTableLike(GetRole()) || IsList(GetRole())) {
IsUIATableLike(GetRole()) || IsList(GetRole())) {
return true;
}
if (IsImage(GetRole())) {
Expand Down Expand Up @@ -7813,13 +7813,13 @@ AXPlatformNodeWin::GetPatternProviderFactoryMethod(PATTERNID pattern_id) {
break;

case UIA_GridPatternId:
if (IsTableLike(GetRole())) {
if (IsUIATableLike(GetRole())) {
return &PatternProvider<IGridProvider>;
}
break;

case UIA_GridItemPatternId:
if (IsCellOrTableHeader(GetRole())) {
if (IsUIACellOrTableHeader(GetRole())) {
return &PatternProvider<IGridItemProvider>;
}
break;
Expand Down Expand Up @@ -7864,7 +7864,12 @@ AXPlatformNodeWin::GetPatternProviderFactoryMethod(PATTERNID pattern_id) {
// headers, we should expose the Table pattern on all table-like roles.
// This will allow clients to detect such constructs as tables and expose
// row/column counts and navigation along with Table semantics.
if (IsTableLike(GetRole()))
//
// On UIA, we don't want to expose the ITableProvider for layout tables
// because it can cause extraneous, confusing announcements for users. We
// initially exposed it, but decided to re-evaluate our decision after
// hearing from users.
if (IsUIATableLike(GetRole()))
return &PatternProvider<ITableProvider>;
break;

Expand All @@ -7875,8 +7880,14 @@ AXPlatformNodeWin::GetPatternProviderFactoryMethod(PATTERNID pattern_id) {
// headers, we should expose the Table pattern on all table-like roles.
// This will allow clients to detect such constructs as tables and expose
// row/column counts and navigation along with Table semantics.
if (IsCellOrTableHeader(GetRole()))
//
// On UIA, we don't want to expose the ITableProvider for layout tables
// because it can cause extraneous, confusing announcements for users. We
// initially exposed it, but decided to re-evaluate our decision after
// hearing from users.
if (IsUIACellOrTableHeader(GetRole())) {
return &PatternProvider<ITableProvider>;
}
break;

case UIA_TextChildPatternId:
Expand Down
2 changes: 1 addition & 1 deletion ui/accessibility/platform/ax_platform_node_win_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5792,7 +5792,7 @@ TEST_F(AXPlatformNodeWinTest, ComputeUIAControlType) {
UIA_ControlTypePropertyId, int{UIA_TableControlTypeId});
EXPECT_UIA_INT_EQ(
QueryInterfaceFromNodeId<IRawElementProviderSimple>(child2.id),
UIA_ControlTypePropertyId, int{UIA_TableControlTypeId});
UIA_ControlTypePropertyId, int{UIA_GroupControlTypeId});
EXPECT_UIA_INT_EQ(
QueryInterfaceFromNodeId<IRawElementProviderSimple>(child3.id),
UIA_ControlTypePropertyId, int{UIA_EditControlTypeId});
Expand Down

0 comments on commit bf19c18

Please sign in to comment.