AX: ARIA "region" role which lacks an accessible name should not be treated as a...
authorjdiggs@igalia.com <jdiggs@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Apr 2017 02:07:21 +0000 (02:07 +0000)
committerjdiggs@igalia.com <jdiggs@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Apr 2017 02:07:21 +0000 (02:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171180

Reviewed by Chris Fleizach.

Source/WebCore:

Remove mapping of LandmarkRegionRole for regions which lack an accessible name.
Doing so had a side effect of causing a number of DPub ARIA roles to stop being
mapped as ARIA landmarks. This is due to our internal role mappings, namely
treating the DPub ARIA landmark roles as if they were regions. Because DPub's
landmarks do not subclass region, and do not have the same name-from-author
requirement as region, create a new LandmarkDocRegionRole AccessibilityRole and
map DPub ARIA's generic landmarks to it.

No new tests because we already have sufficient coverage. Several existing
tests were given additional test cases to cover named and unnamed regions,
and test expectations updated accordingly.

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::determineAriaRoleAttribute):
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::accessibleNameDerivesFromContent):
(WebCore::AccessibilityObject::isLandmark):
(WebCore::initializeRoleMap):
(WebCore::AccessibilityObject::computedRoleString):
* accessibility/AccessibilityObject.h:
* accessibility/atk/WebKitAccessibleWrapperAtk.cpp:
(atkRole):
* accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
(-[WebAccessibilityObjectWrapper _accessibilityIsLandmarkRole:]):
(-[WebAccessibilityObjectWrapper determineIsAccessibilityElement]):
* accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase ariaLandmarkRoleDescription]):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(createAccessibilityRoleMap):
(-[WebAccessibilityObjectWrapper subrole]):

LayoutTests:

Add new test cases to several tests so that we have coverage for both named and
unnamed regions, which now have different mappings. In aria-namefrom-author.html,
the region role was replaced with the table role because the test requires elements
which have mapped ARIA roles and lack an author-provided name. A region which lacks
an author-provided name is no longer mapped as an ARIA role as per the spec. Table
was introduced in ARIA 1.1 and serves the same purpose with respect to what is
being tested.

* accessibility/aria-namefrom-author.html:
* accessibility/gtk/xml-roles-exposed-expected.txt:
* accessibility/gtk/xml-roles-exposed.html:
* accessibility/mac/aria-grouping-roles.html:
* accessibility/roles-computedRoleString.html:
* accessibility/roles-exposed.html:
* platform/gtk/accessibility/roles-exposed-expected.txt:
* platform/mac/accessibility/roles-computedRoleString-expected.txt:
* platform/mac/accessibility/roles-exposed-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@215968 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
LayoutTests/ChangeLog
LayoutTests/accessibility/aria-namefrom-author.html
LayoutTests/accessibility/gtk/xml-roles-exposed-expected.txt
LayoutTests/accessibility/gtk/xml-roles-exposed.html
LayoutTests/accessibility/mac/aria-grouping-roles.html
LayoutTests/accessibility/roles-computedRoleString.html
LayoutTests/accessibility/roles-exposed.html
LayoutTests/platform/gtk/accessibility/roles-exposed-expected.txt
LayoutTests/platform/mac/accessibility/roles-computedRoleString-expected.txt
LayoutTests/platform/mac/accessibility/roles-exposed-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/accessibility/AccessibilityObject.h
Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp
Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.mm
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

index b4a967d..e3b37d2 100644 (file)
@@ -1,3 +1,28 @@
+2017-04-28  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        AX: ARIA "region" role which lacks an accessible name should not be treated as a landmark
+        https://bugs.webkit.org/show_bug.cgi?id=171180
+
+        Reviewed by Chris Fleizach.
+
+        Add new test cases to several tests so that we have coverage for both named and
+        unnamed regions, which now have different mappings. In aria-namefrom-author.html,
+        the region role was replaced with the table role because the test requires elements
+        which have mapped ARIA roles and lack an author-provided name. A region which lacks
+        an author-provided name is no longer mapped as an ARIA role as per the spec. Table
+        was introduced in ARIA 1.1 and serves the same purpose with respect to what is
+        being tested.
+
+        * accessibility/aria-namefrom-author.html:
+        * accessibility/gtk/xml-roles-exposed-expected.txt:
+        * accessibility/gtk/xml-roles-exposed.html:
+        * accessibility/mac/aria-grouping-roles.html:
+        * accessibility/roles-computedRoleString.html:
+        * accessibility/roles-exposed.html:
+        * platform/gtk/accessibility/roles-exposed-expected.txt:
+        * platform/mac/accessibility/roles-computedRoleString-expected.txt:
+        * platform/mac/accessibility/roles-exposed-expected.txt:
+
 2017-04-28  Joseph Pecoraro  <pecoraro@apple.com>
 
         LayoutTests/js/dom/Promise-static-all/race.html are flakey - Unhandled Promise Rejection messages
index 4ac0b2a..9bf62f6 100644 (file)
@@ -21,7 +21,7 @@
 <div id="test11" class="test" role="article">article</div>
 <div id="test12" class="test" role="math">math</div>
 <div id="test13" class="test" role="note">note</div>
-<div id="test14" class="test" role="region">region</div>
+<div id="test14" class="test" role="table">table</div>
 <div id="test15" class="test" role="form">form</div>
 <div id="test16" class="test" role="grid">grid</div>
 <div id="test17" class="test" role="group">group</div>
index 91abc2c..7db007d 100644 (file)
@@ -1,4 +1,3 @@
-
 Verify exposure of ARIA role values via the 'xml-roles' object attribute.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -542,7 +541,7 @@ AXParent: AXWebArea
 AXChildren: 0
 AXPosition: { 0.000000, 0.000000 }
 AXSize: { 0.000000, 0.000000 }
-AXTitle: 
+AXTitle: X
 AXDescription: 
 AXValue: 
 AXFocusable: 0
@@ -556,6 +555,25 @@ AXRequired: 0
 AXChecked: 0
 AXPlatformAttributes: computed-role:region, xml-roles:region, tag:div, toolkit:WebKitGtk
 ------------
+AXRole: AXSection
+AXParent: AXWebArea
+AXChildren: 0
+AXPosition: { 8.000000, 8.000000 }
+AXSize: { 784.000000, 17.000000 }
+AXTitle: 
+AXDescription: 
+AXValue: X
+AXFocusable: 0
+AXFocused: 0
+AXSelectable: 0
+AXSelected: 0
+AXMultiSelectable: 0
+AXEnabled: 1
+AXExpanded: 0
+AXRequired: 0
+AXChecked: 0
+AXPlatformAttributes: xml-roles:region, tag:div, toolkit:WebKitGtk
+------------
 AXRole: AXScrollBar
 AXParent: AXWebArea
 AXChildren: 0
@@ -1532,7 +1550,7 @@ AXPlatformAttributes: computed-role:navigation, xml-roles:doc-toc, tag:div, tool
 AXRole: AXParagraph
 AXParent: AXWebArea
 AXChildren: 2
-AXPosition: { 8.000000, 8.000000 }
+AXPosition: { 8.000000, 41.000000 }
 AXSize: { 784.000000, 50.000000 }
 AXTitle: 
 AXDescription: 
index 30bd9c7..6dc5bf9 100644 (file)
@@ -34,7 +34,8 @@
  <div role="presentation"></div>
  <div role="progressbar"></div>
  <div role="radio"></div>
- <div role="region"></div>
+ <div role="region" aria-label="X"></div>
+ <div role="region">X</div>
  <div role="scrollbar"></div>
  <div role="search"></div>
  <div role="searchbox"></div>
@@ -94,6 +95,7 @@ description("Verify exposure of ARIA role values via the 'xml-roles' object attr
 if (window.accessibilityController) {
     document.getElementById("body").focus();
     debug(accessibilityController.focusedElement.attributesOfChildren());
+    document.getElementById("test").style.visibility = "hidden";
 }
 </script>
 <script src="../../resources/js-test-post.js"></script>
index 2f4e175..3fa5460 100644 (file)
@@ -16,7 +16,7 @@
 <div role="marquee">marquee role</div>
 <div role="note">note role</div>
 <div role="navigation">navigation role</div>
-<div role="region">region role</div>
+<div role="region" aria-label="region">region role</div>
 <div role="search">search role</div>
 <div role="status">status role</div>
 <div role="tooltip">tooltip role</div>
index c8c098f..8e147d7 100644 (file)
 <div role="radiogroup"               data-role="radiogroup" class="ex">
     <div role="radio"                data-role="radio" class="ex">X</div>
 </div>
-<div role="region"                   data-role="region" class="ex">X</div>
+<div role="region"                   class="ex" data-role="" data-note=":not([aria-label]:not([aria-labelledby])">X</div>
+<div role="region"                   class="ex" data-role="region" aria-label="x" data-note="[aria-label]">X</div>
+<div role="region"                   class="ex" data-role="region" aria-labelledby="region-label" data-note="[aria-labelledby]">
+    <h2 id="region-label">X</h2>
+</div>
+
 <div role="scrollbar"                data-role="scrollbar" class="ex">X</div>
 <div role="search"                   data-role="search" class="ex">X</div>
 <div role="separator"                data-role="separator" class="ex">X</div>
index 631a0a3..2a98508 100644 (file)
 <div role="radiogroup"               data-platform="atk,mac" class="ex">
     <div role="radio"                data-platform="atk,mac" class="ex">X</div>
 </div>
-<div role="region"                   data-platform="atk,mac" class="ex">X</div>
+<div role="region"                   data-platform="atk,mac" class="ex" data-note=":not([aria-label]:not([aria-labelledby])">X</div>
+<div role="region"                   data-platform="atk,mac" class="ex" aria-label="x" data-note="[aria-label]">X</div>
+<div role="region"                   data-platform="atk,mac" class="ex" aria-labelledby="region-label" data-note="[aria-labelledby]">
+    <h2 id="region-label">X</h2>
+</div>
 <div role="scrollbar"                data-platform="atk,mac" class="ex">X</div>
 <div role="search"                   data-platform="atk,mac" class="ex">X</div>
 <div role="searchbox"                data-platform="atk,mac" class="ex">X</div>
index 7b43d14..f205739 100644 (file)
@@ -745,7 +745,13 @@ div[role=radiogroup]
 div[role=radio]
       AXRole: AXRadioButton
       
-div[role=region]
+div[role=region]:not([aria-label]:not([aria-labelledby])
+      AXRole: AXSection
+      
+div[role=region][aria-label]
+      AXRole: AXLandmarkRegion
+      
+div[role=region][aria-labelledby]
       AXRole: AXLandmarkRegion
       
 div[role=scrollbar]
index 44e2942..f7a3de8 100644 (file)
@@ -139,7 +139,9 @@ PASS: div[role="note"] -> note.
 PASS: div[role="progressbar"] -> progressbar. 
 PASS: div[role="radiogroup"] -> radiogroup. 
 PASS: div[role="radio"] -> radio. 
-PASS: div[role="region"] -> region. 
+PASS: div[role="region"]:not([aria-label]:not([aria-labelledby]) -> . 
+PASS: div[role="region"][aria-label] -> region. 
+PASS: div[role="region"][aria-labelledby] -> region. 
 PASS: div[role="scrollbar"] -> scrollbar. 
 PASS: div[role="search"] -> search. 
 PASS: div[role="separator"] -> separator. 
index 6271138..258d4ab 100644 (file)
@@ -1244,7 +1244,17 @@ div[role=radio]
       AXSubrole: 
       AXRoleDescription: radio button
       
-div[role=region]
+div[role=region]:not([aria-label]:not([aria-labelledby])
+      AXRole: AXGroup
+      AXSubrole: 
+      AXRoleDescription: group
+      
+div[role=region][aria-label]
+      AXRole: AXGroup
+      AXSubrole: AXLandmarkRegion
+      AXRoleDescription: region
+      
+div[role=region][aria-labelledby]
       AXRole: AXGroup
       AXSubrole: AXLandmarkRegion
       AXRoleDescription: region
index 0a00e22..0f177e8 100644 (file)
@@ -1,3 +1,41 @@
+2017-04-28  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        AX: ARIA "region" role which lacks an accessible name should not be treated as a landmark
+        https://bugs.webkit.org/show_bug.cgi?id=171180
+
+        Reviewed by Chris Fleizach.
+
+        Remove mapping of LandmarkRegionRole for regions which lack an accessible name.
+        Doing so had a side effect of causing a number of DPub ARIA roles to stop being
+        mapped as ARIA landmarks. This is due to our internal role mappings, namely
+        treating the DPub ARIA landmark roles as if they were regions. Because DPub's
+        landmarks do not subclass region, and do not have the same name-from-author
+        requirement as region, create a new LandmarkDocRegionRole AccessibilityRole and
+        map DPub ARIA's generic landmarks to it.
+
+        No new tests because we already have sufficient coverage. Several existing
+        tests were given additional test cases to cover named and unnamed regions,
+        and test expectations updated accordingly.
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::determineAriaRoleAttribute):
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::accessibleNameDerivesFromContent):
+        (WebCore::AccessibilityObject::isLandmark):
+        (WebCore::initializeRoleMap):
+        (WebCore::AccessibilityObject::computedRoleString):
+        * accessibility/AccessibilityObject.h:
+        * accessibility/atk/WebKitAccessibleWrapperAtk.cpp:
+        (atkRole):
+        * accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
+        (-[WebAccessibilityObjectWrapper _accessibilityIsLandmarkRole:]):
+        (-[WebAccessibilityObjectWrapper determineIsAccessibilityElement]):
+        * accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
+        (-[WebAccessibilityObjectWrapperBase ariaLandmarkRoleDescription]):
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (createAccessibilityRoleMap):
+        (-[WebAccessibilityObjectWrapper subrole]):
+
 2017-04-28  Simon Fraser  <simon.fraser@apple.com>
 
         Enhance showLayerTree() to show fragments
index e1e71a8..66b83e7 100644 (file)
@@ -2121,6 +2121,13 @@ AccessibilityRole AccessibilityNodeObject::determineAriaRoleAttribute() const
     if (role == PresentationalRole && supportsARIAAttributes())
         role = UnknownRole;
     
+    // The ARIA spec states, "Authors must give each element with role region a brief label that
+    // describes the purpose of the content in the region." The Core AAM states, "Special case:
+    // if the region does not have an accessible name, do not expose the element as a landmark.
+    // Use the native host language role of the element instead."
+    if (role == LandmarkRegionRole && !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr))
+        role = UnknownRole;
+
     if (role)
         return role;
 
index 409d27e..d2bcf3b 100644 (file)
@@ -321,6 +321,7 @@ bool AccessibilityObject::accessibleNameDerivesFromContent() const
     case DocumentMathRole:
     case DocumentNoteRole:
     case LandmarkRegionRole:
+    case LandmarkDocRegionRole:
     case FormRole:
     case GridRole:
     case GroupRole:
@@ -414,6 +415,7 @@ bool AccessibilityObject::isLandmark() const
     return role == LandmarkBannerRole
         || role == LandmarkComplementaryRole
         || role == LandmarkContentInfoRole
+        || role == LandmarkDocRegionRole
         || role == LandmarkMainRole
         || role == LandmarkNavigationRole
         || role == LandmarkRegionRole
@@ -2145,39 +2147,39 @@ static void initializeRoleMap()
         // The 'doc-*' roles are defined the ARIA DPUB mobile: https://www.w3.org/TR/dpub-aam-1.0/ 
         // Editor's draft is currently at https://rawgit.com/w3c/aria/master/dpub-aam/dpub-aam.html 
         { "doc-abstract", ApplicationTextGroupRole },
-        { "doc-acknowledgments", LandmarkRegionRole },
-        { "doc-afterword", LandmarkRegionRole },
-        { "doc-appendix", LandmarkRegionRole },
+        { "doc-acknowledgments", LandmarkDocRegionRole },
+        { "doc-afterword", LandmarkDocRegionRole },
+        { "doc-appendix", LandmarkDocRegionRole },
         { "doc-backlink", WebCoreLinkRole },
         { "doc-biblioentry", ListItemRole },
-        { "doc-bibliography", LandmarkRegionRole },
+        { "doc-bibliography", LandmarkDocRegionRole },
         { "doc-biblioref", WebCoreLinkRole },
-        { "doc-chapter", LandmarkRegionRole },
+        { "doc-chapter", LandmarkDocRegionRole },
         { "doc-colophon", ApplicationTextGroupRole },
-        { "doc-conclusion", LandmarkRegionRole },
+        { "doc-conclusion", LandmarkDocRegionRole },
         { "doc-cover", ImageRole },
         { "doc-credit", ApplicationTextGroupRole },
-        { "doc-credits", LandmarkRegionRole },
+        { "doc-credits", LandmarkDocRegionRole },
         { "doc-dedication", ApplicationTextGroupRole },
         { "doc-endnote", ListItemRole },
-        { "doc-endnotes", LandmarkRegionRole },
+        { "doc-endnotes", LandmarkDocRegionRole },
         { "doc-epigraph", ApplicationTextGroupRole },
-        { "doc-epilogue", LandmarkRegionRole },
-        { "doc-errata", LandmarkRegionRole },
+        { "doc-epilogue", LandmarkDocRegionRole },
+        { "doc-errata", LandmarkDocRegionRole },
         { "doc-example", ApplicationTextGroupRole },
         { "doc-footnote", ApplicationTextGroupRole },
-        { "doc-foreword", LandmarkRegionRole },
-        { "doc-glossary", LandmarkRegionRole },
+        { "doc-foreword", LandmarkDocRegionRole },
+        { "doc-glossary", LandmarkDocRegionRole },
         { "doc-glossref", WebCoreLinkRole },
         { "doc-index", LandmarkNavigationRole },
-        { "doc-introduction", LandmarkRegionRole },
+        { "doc-introduction", LandmarkDocRegionRole },
         { "doc-noteref", WebCoreLinkRole },
         { "doc-notice", DocumentNoteRole },
         { "doc-pagebreak", SplitterRole },
         { "doc-pagelist", LandmarkNavigationRole },
-        { "doc-part", LandmarkRegionRole },
-        { "doc-preface", LandmarkRegionRole },
-        { "doc-prologue", LandmarkRegionRole },
+        { "doc-part", LandmarkDocRegionRole },
+        { "doc-preface", LandmarkDocRegionRole },
+        { "doc-prologue", LandmarkDocRegionRole },
         { "doc-pullquote", ApplicationTextGroupRole },
         { "doc-qna", ApplicationTextGroupRole },
         { "doc-subtitle", HeadingRole },
@@ -2298,6 +2300,9 @@ String AccessibilityObject::computedRoleString() const
     if (role == PopUpButtonRole || role == ToggleButtonRole)
         return reverseAriaRoleMap().get(ButtonRole);
 
+    if (role == LandmarkDocRegionRole)
+        return reverseAriaRoleMap().get(LandmarkRegionRole);
+
     return reverseAriaRoleMap().get(role);
 }
 
index dd3f553..4058736 100644 (file)
@@ -147,6 +147,7 @@ enum AccessibilityRole {
     LandmarkBannerRole,
     LandmarkComplementaryRole,
     LandmarkContentInfoRole,
+    LandmarkDocRegionRole,
     LandmarkMainRole,
     LandmarkNavigationRole,
     LandmarkRegionRole,
index 9a8952d..45a27df 100644 (file)
@@ -670,6 +670,7 @@ static AtkRole atkRole(AccessibilityObject* coreObject)
     case LandmarkBannerRole:
     case LandmarkComplementaryRole:
     case LandmarkContentInfoRole:
+    case LandmarkDocRegionRole:
     case LandmarkMainRole:
     case LandmarkNavigationRole:
     case LandmarkRegionRole:
index a66369d..2d2892f 100644 (file)
@@ -521,6 +521,7 @@ static AccessibilityObjectWrapper* AccessibilityUnignoredAncestor(AccessibilityO
     case LandmarkBannerRole:
     case LandmarkComplementaryRole:
     case LandmarkContentInfoRole:
+    case LandmarkDocRegionRole:
     case LandmarkMainRole:
     case LandmarkNavigationRole:
     case LandmarkRegionRole:
@@ -882,6 +883,7 @@ static AccessibilityObjectWrapper* AccessibilityUnignoredAncestor(AccessibilityO
         case LandmarkBannerRole:
         case LandmarkComplementaryRole:
         case LandmarkContentInfoRole:
+        case LandmarkDocRegionRole:
         case LandmarkMainRole:
         case LandmarkNavigationRole:
         case LandmarkRegionRole:
index 7a531f0..dc0ad1b 100644 (file)
@@ -509,6 +509,7 @@ static void convertPathToScreenSpaceFunction(PathConversionInfo& conversion, con
         return AXARIAContentGroupText(@"ARIALandmarkMain");
     case LandmarkNavigationRole:
         return AXARIAContentGroupText(@"ARIALandmarkNavigation");
+    case LandmarkDocRegionRole:
     case LandmarkRegionRole:
         return AXARIAContentGroupText(@"ARIALandmarkRegion");
     case LandmarkSearchRole:
index ed4ea37..f842e86 100644 (file)
@@ -1876,6 +1876,7 @@ static const AccessibilityRoleMap& createAccessibilityRoleMap()
         { WebApplicationRole, NSAccessibilityGroupRole },
         { LandmarkBannerRole, NSAccessibilityGroupRole },
         { LandmarkComplementaryRole, NSAccessibilityGroupRole },
+        { LandmarkDocRegionRole, NSAccessibilityGroupRole },
         { LandmarkContentInfoRole, NSAccessibilityGroupRole },
         { LandmarkMainRole, NSAccessibilityGroupRole },
         { LandmarkNavigationRole, NSAccessibilityGroupRole },
@@ -2033,6 +2034,7 @@ static NSString* roleValueToNSString(AccessibilityRole value)
             return @"AXLandmarkMain";
         case LandmarkNavigationRole:
             return @"AXLandmarkNavigation";
+        case LandmarkDocRegionRole:
         case LandmarkRegionRole:
             return @"AXLandmarkRegion";
         case LandmarkSearchRole: