0

Revert "[A11y] When repairing prohibited name, avoid redundant description"

This reverts commit 6946fa2839.

Reason for revert: All/DumpAccessibilityTreeTest.AccessibilityProhibitedName/blink is consistently failing on Mac13 Tests: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac13%20Tests/18046/overview

Original change's description:
> [A11y] When repairing prohibited name, avoid redundant description
>
> Do not expose the prohibited name as a description if it would be
> redundant with the inner contents of the object.
>
> Also, implement ATK/IA2 name-from object attribute so that ATs can
> make their own determination as to the value of the repaired name.
> For more info about name-from, see:
> https://docs.google.com/document/d/1-zjGmiOtjrHeBjXVGkhCcK2ya4Xq_k3yerHqSi_3qsY/edit#heading=h.benybc8eru3u
>
> Bug: 350528330
> Change-Id: Ibe2d4ccb777c24f0b5a471b8b45848051add0004
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3900708
> Reviewed-by: Tim <tjudkins@chromium.org>
> Reviewed-by: Brendon Tiszka <tiszka@chromium.org>
> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
> Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
> Reviewed-by: Jacques Newman <janewman@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#1338011}

Bug: 350528330
Change-Id: Ia53be82170c486b840e5146f1de20c00d830cdbb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5766619
Owners-Override: Marijn Kruisselbrink <mek@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1338060}
This commit is contained in:
Marijn Kruisselbrink
2024-08-06 20:05:25 +00:00
committed by Chromium LUCI CQ
parent ab20740353
commit baf03caed1
19 changed files with 14 additions and 135 deletions

@ -1168,8 +1168,7 @@ TEST_F(BrowserAccessibilityWinTest, TestIA2Attributes) {
EXPECT_EQ(S_OK, hr);
EXPECT_NE(nullptr, attributes.Get());
attributes_str = std::wstring(attributes.Get(), attributes.Length());
EXPECT_EQ(L"checkable:true;name-from:attribute;explicit-name:true;",
attributes_str);
EXPECT_EQ(L"checkable:true;explicit-name:true;", attributes_str);
manager.reset();
}

@ -3088,14 +3088,6 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityPermission) {
RunHtmlTest(FILE_PATH_LITERAL("permission.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityProhibitedName) {
// Explicitly enable feature that repairs accessible names on roles where it
// prohibited, moving to description.
base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
switches::kEnableBlinkFeatures, "AccessibilityProhibitedNames");
RunHtmlTest(FILE_PATH_LITERAL("prohibited-name.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityPopoverApi) {
RunHtmlTest(FILE_PATH_LITERAL("popover-api.html"));
}

@ -4604,10 +4604,6 @@ data/accessibility/html/progress-expected-mac.txt
data/accessibility/html/progress-expected-uia-win.txt
data/accessibility/html/progress-expected-win.txt
data/accessibility/html/progress.html
data/accessibility/html/prohibited-name-expected-auralinux.txt
data/accessibility/html/prohibited-name-expected-blink.txt
data/accessibility/html/prohibited-name-expected-win.txt
data/accessibility/html/prohibited-name.html
data/accessibility/html/q-expected-android-assist-data.txt
data/accessibility/html/q-expected-android-external.txt
data/accessibility/html/q-expected-android.txt

@ -1 +1 @@
[combo box] name='Choose your language.' name-from:tooltip
[combo box] name='Choose your language.'

@ -1 +1 @@
ROLE_SYSTEM_COMBOBOX name='Choose your language.' FOCUSABLE HASPOPUP name-from:tooltip
ROLE_SYSTEM_COMBOBOX name='Choose your language.' FOCUSABLE HASPOPUP

@ -1,7 +0,0 @@
[document web]
++[section] description='Illegal aria-label' description-from:prohibited-name-repair
++++[static] name='Illegal aria-label moves to description'
++[section]
++++[static] name='Redundant aria-label ignored'
++[section] description='Manual description' description:Manual description description-from:aria-description
++++[static] name='Some inner content'

@ -1,12 +0,0 @@
rootWebArea
++genericContainer ignored
++++genericContainer ignored
++++++genericContainer description='Illegal aria-label' nameFrom=prohibited descriptionFrom=prohibitedNameRepair
++++++++staticText name='Illegal aria-label moves to description'
++++++++++inlineTextBox name='Illegal aria-label moves to description'
++++++genericContainer
++++++++staticText name='Redundant aria-label ignored'
++++++++++inlineTextBox name='Redundant aria-label ignored'
++++++genericContainer description='Manual description' nameFrom=prohibited descriptionFrom=ariaDescription
++++++++staticText name='Some inner content'
++++++++++inlineTextBox name='Some inner content'

@ -1,7 +0,0 @@
ROLE_SYSTEM_DOCUMENT READONLY FOCUSABLE
++IA2_ROLE_SECTION description-from:prohibited-name-repair description='Illegal aria-label'
++++ROLE_SYSTEM_STATICTEXT name='Illegal aria-label moves to description'
++IA2_ROLE_SECTION
++++ROLE_SYSTEM_STATICTEXT name='Redundant aria-label ignored'
++IA2_ROLE_SECTION description:Manual description description-from:aria-description description='Manual description'
++++ROLE_SYSTEM_STATICTEXT name='Some inner content'

@ -1,9 +0,0 @@
<div aria-label="Illegal aria-label">
Illegal aria-label moves to description
</div>
<div aria-label="Redundant aria-label ignored">
Redundant aria-label ignored
</div>
<div aria-label="Nowhere for aria-label to go" aria-description="Manual description">
Some inner content
</div>

@ -478,7 +478,6 @@
placeholder,
popoverAttribute,
prohibited,
prohibitedAndRedundant,
relatedElement,
title,
value

@ -4838,7 +4838,6 @@ static bool ShouldInsertSpaceBetweenObjectsIfNeeded(
case ax::mojom::blink::NameFrom::kAttributeExplicitlyEmpty:
case ax::mojom::blink::NameFrom::kContents:
case ax::mojom::blink::NameFrom::kProhibited:
case ax::mojom::blink::NameFrom::kProhibitedAndRedundant:
break;
case ax::mojom::blink::NameFrom::kAttribute:
case ax::mojom::blink::NameFrom::kCaption:
@ -4854,7 +4853,6 @@ static bool ShouldInsertSpaceBetweenObjectsIfNeeded(
case ax::mojom::blink::NameFrom::kAttributeExplicitlyEmpty:
case ax::mojom::blink::NameFrom::kContents:
case ax::mojom::blink::NameFrom::kProhibited:
case ax::mojom::blink::NameFrom::kProhibitedAndRedundant:
break;
case ax::mojom::blink::NameFrom::kAttribute:
case ax::mojom::blink::NameFrom::kCaption:

@ -4391,15 +4391,8 @@ String AXObject::SimplifyName(const String& str,
if (RuntimeEnabledFeatures::AccessibilityProhibitedNamesEnabled()) {
// Prohibited names are repaired by moving them to the description field,
// where they will not override the contents of the element for screen
// reader users. Exception: if it would be redundant with the inner
// contents, then the name is stripped out rather than repaired.
// reader users.
name_from = ax::mojom::blink::NameFrom::kProhibited;
// If already redundant with inner text, do not repair to description
if (name_from == ax::mojom::blink::NameFrom::kContents ||
simplified ==
GetElement()->GetInnerTextWithoutUpdate().StripWhiteSpace()) {
name_from = ax::mojom::blink::NameFrom::kProhibitedAndRedundant;
}
return "";
}
}

@ -459,7 +459,6 @@ chrome.automation.NameFromType = {
PLACEHOLDER: 'placeholder',
POPOVER_ATTRIBUTE: 'popoverAttribute',
PROHIBITED: 'prohibited',
PROHIBITED_AND_REDUNDANT: 'prohibitedAndRedundant',
RELATED_ELEMENT: 'relatedElement',
TITLE: 'title',
VALUE: 'value',
@ -474,7 +473,6 @@ chrome.automation.DescriptionFromType = {
ATTRIBUTE_EXPLICITLY_EMPTY: 'attributeExplicitlyEmpty',
BUTTON_LABEL: 'buttonLabel',
POPOVER_ATTRIBUTE: 'popoverAttribute',
PROHIBITED_NAME_REPAIR: 'prohibitedNameRepair',
RELATED_ELEMENT: 'relatedElement',
RUBY_ANNOTATION: 'rubyAnnotation',
SUMMARY: 'summary',

@ -599,7 +599,6 @@ std::string AXComputedNodeData::ComputeTextContentUTF8() const {
// If kProhibited is set, that means we calculated a name in Blink and
// are deliberately not exposing it.
case ax::mojom::NameFrom::kProhibited:
case ax::mojom::NameFrom::kProhibitedAndRedundant:
case ax::mojom::NameFrom::kRelatedElement:
// The accessible name is not displayed directly inside the node but is
// visible via e.g. a tooltip.

@ -2380,8 +2380,6 @@ const char* ToString(ax::mojom::NameFrom name_from) {
return "placeholder";
case ax::mojom::NameFrom::kProhibited:
return "prohibited";
case ax::mojom::NameFrom::kProhibitedAndRedundant:
return "prohibitedAndRedundant";
case ax::mojom::NameFrom::kRelatedElement:
return "relatedElement";
case ax::mojom::NameFrom::kTitle:

@ -1307,14 +1307,9 @@ enum NameFrom {
// the figcaption element in a figure, or another View present in the UI.
kRelatedElement,
// Applies to roles that cannot be named as per the ARIA spec, where the
// name may be repaired by moving to the description.
// Applies to roles that cannot be named as per the ARIA spec.
kProhibited,
// Applies to roles that cannot be named as per the ARIA spec, where the
// name would have been redundant with the inner text.
kProhibitedAndRedundant,
// The name comes from the title attribute (HTML), the title element (SVG),
// or a View's tooltip.
kTitle,

@ -1293,58 +1293,10 @@ void AXPlatformNodeBase::ComputeAttributes(PlatformAttributeList* attributes) {
AddAttributeToList(ax::mojom::BoolAttribute::kContainerLiveBusy,
"container-busy", attributes);
// Expose name-from.
ax::mojom::NameFrom name_from = GetNameFrom();
std::string from;
bool is_explicit_name = true;
switch (static_cast<ax::mojom::NameFrom>(name_from)) {
case ax::mojom::NameFrom::kAttribute:
from = "attribute";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kCaption:
from = "caption";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kContents:
is_explicit_name = false;
from = "contents";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kPlaceholder:
from = "placeholder";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kProhibited:
case ax::mojom::NameFrom::kProhibitedAndRedundant:
is_explicit_name = false;
from = "prohibited";
DCHECK(GetName().empty());
break;
case ax::mojom::NameFrom::kRelatedElement:
from = "related-element";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kPopoverAttribute:
case ax::mojom::NameFrom::kTitle:
from = "tooltip";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kValue:
from = "value";
DCHECK(!GetName().empty());
break;
case ax::mojom::NameFrom::kAttributeExplicitlyEmpty:
break;
case ax::mojom::NameFrom::kNone:
is_explicit_name = false;
break; // Not exposed.
}
if (!from.empty()) {
AddAttributeToList("name-from", from, attributes);
}
// Expose the non-standard explicit-name IA2 attribute.
if (is_explicit_name) {
int name_from;
if (GetIntAttribute(ax::mojom::IntAttribute::kNameFrom, &name_from) &&
name_from != static_cast<int32_t>(ax::mojom::NameFrom::kContents)) {
AddAttributeToList("explicit-name", "true", attributes);
}

@ -647,7 +647,6 @@ const char* const ATK_OBJECT_ATTRIBUTES[] = {
"level",
"link-target",
"live",
"name-from",
"placeholder",
"posinset",
"relevant",

@ -707,18 +707,14 @@ TEST_F(ViewAXPlatformNodeDelegateWinTableTest, TableCellAttributes) {
// These strings should NOT contain rowindex or colindex, since those
// imply an ARIA override.
EXPECT_EQ(
get_attributes(1, 1),
L"name-from:attribute;explicit-name:true;sort:none;class:AXVirtualView;");
EXPECT_EQ(
get_attributes(1, 2),
L"name-from:attribute;explicit-name:true;sort:none;class:AXVirtualView;");
EXPECT_EQ(get_attributes(1, 1),
L"explicit-name:true;sort:none;class:AXVirtualView;");
EXPECT_EQ(get_attributes(1, 2),
L"explicit-name:true;sort:none;class:AXVirtualView;");
EXPECT_EQ(get_attributes(2, 1),
L"hidden:true;name-from:attribute;explicit-name:true;class:"
L"AXVirtualView;");
L"hidden:true;explicit-name:true;class:AXVirtualView;");
EXPECT_EQ(get_attributes(2, 2),
L"hidden:true;name-from:attribute;explicit-name:true;class:"
L"AXVirtualView;");
L"hidden:true;explicit-name:true;class:AXVirtualView;");
}
} // namespace test