[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}

content/browser/accessibility/browser_accessibility_win_unittest.cc

@@ -1168,7 +1168,8 @@ 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;explicit-name:true;", attributes_str);
+  EXPECT_EQ(L"checkable:true;name-from:attribute;explicit-name:true;",
+            attributes_str);
 
   manager.reset();
 }

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -3088,6 +3088,14 @@ 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"));
 }

content/test/content_test_bundle_data.filelist

@@ -4604,6 +4604,10 @@ 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

content/test/data/accessibility/accname/name-combobox-focusable-expected-auralinux.txt

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

content/test/data/accessibility/accname/name-combobox-focusable-expected-win.txt

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

content/test/data/accessibility/html/prohibited-name-expected-auralinux.txt

@@ -0,0 +1,7 @@
+[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'

content/test/data/accessibility/html/prohibited-name-expected-blink.txt

@@ -0,0 +1,12 @@
+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'

content/test/data/accessibility/html/prohibited-name-expected-win.txt

@@ -0,0 +1,7 @@
+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'

content/test/data/accessibility/html/prohibited-name.html

@@ -0,0 +1,9 @@
+<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>

extensions/common/api/automation.idl

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

third_party/blink/renderer/modules/accessibility/ax_node_object.cc

@@ -4838,6 +4838,7 @@ 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:
@@ -4853,6 +4854,7 @@ 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:

third_party/blink/renderer/modules/accessibility/ax_object.cc

@@ -4391,8 +4391,15 @@ 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.
+      // reader users. Exception: if it would be redundant with the inner
+      // contents, then the name is stripped out rather than repaired.
       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 "";
     }
   }

third_party/closure_compiler/externs/automation.js

@@ -459,6 +459,7 @@ chrome.automation.NameFromType = {
   PLACEHOLDER: 'placeholder',
   POPOVER_ATTRIBUTE: 'popoverAttribute',
   PROHIBITED: 'prohibited',
+  PROHIBITED_AND_REDUNDANT: 'prohibitedAndRedundant',
   RELATED_ELEMENT: 'relatedElement',
   TITLE: 'title',
   VALUE: 'value',
@@ -473,6 +474,7 @@ 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',

ui/accessibility/ax_computed_node_data.cc

@@ -599,6 +599,7 @@ 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.

ui/accessibility/ax_enum_util.cc

@@ -2380,6 +2380,8 @@ 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:

ui/accessibility/ax_enums.mojom

@@ -1307,9 +1307,14 @@ 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.
+  // Applies to roles that cannot be named as per the ARIA spec, where the
+  // name may be repaired by moving to the description.
   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,

ui/accessibility/platform/ax_platform_node_base.cc

@@ -1293,10 +1293,58 @@ 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.
-  int name_from;
-  if (GetIntAttribute(ax::mojom::IntAttribute::kNameFrom, &name_from) &&
-      name_from != static_cast<int32_t>(ax::mojom::NameFrom::kContents)) {
+  if (is_explicit_name) {
     AddAttributeToList("explicit-name", "true", attributes);
   }
 

ui/accessibility/platform/inspect/ax_tree_formatter_auralinux.cc

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

ui/views/accessibility/view_ax_platform_node_delegate_win_unittest.cc

@@ -707,14 +707,18 @@ 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"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(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(2, 1),
-            L"hidden:true;explicit-name:true;class:AXVirtualView;");
+            L"hidden:true;name-from:attribute;explicit-name:true;class:"
+            L"AXVirtualView;");
   EXPECT_EQ(get_attributes(2, 2),
-            L"hidden:true;explicit-name:true;class:AXVirtualView;");
+            L"hidden:true;name-from:attribute;explicit-name:true;class:"
+            L"AXVirtualView;");
 }
 
 }  // namespace test