Attachment 362827 Details for Bug 190138 – patch

[patch] patch

patch (text/plain), 19.20 KB, created by chris fleizach on 2019-02-23 09:57:26 PST

(hide)

Description:

Filename:

MIME Type:

Creator: chris fleizach

Created: 2019-02-23 09:57:26 PST

Size: 19.20 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 241989)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2019-02-23  Chris Fleizach  <cfleizach@apple.com>
>+
>+        AX: <footer> HTML5 tag not reading as ARIA Landmark to VoiceOver
>+        https://bugs.webkit.org/show_bug.cgi?id=190138
>+        <rdar://problem/44907695>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make sure that footer elements use the right role depending on their context.
>+        If scoped to body, they become contentinfo. Otherwise they are just delineated by
>+        a footer subrole.
>+
>+        * accessibility/AccessibilityObject.cpp:
>+        (WebCore::AccessibilityObject::isLandmark const):
>+        * accessibility/AccessibilityRenderObject.cpp:
>+        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
>+        (WebCore::AccessibilityRenderObject::isDescendantOfElementType const):
>+        (WebCore::AccessibilityRenderObject::determineAccessibilityRole):
>+        * accessibility/AccessibilityRenderObject.h:
>+        * accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
>+        (-[WebAccessibilityObjectWrapper _accessibilityIsLandmarkRole:]):
>+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
>+        (-[WebAccessibilityObjectWrapper subrole]):
>+
> 2019-02-23  chris fleizach  <cfleizach@apple.com>
> 
>         AX: WebKit is incorrectly mapping the <meter> element to progressbar
>Index: Source/WebCore/accessibility/AccessibilityObject.cpp
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityObject.cpp	(revision 241863)
>+++ Source/WebCore/accessibility/AccessibilityObject.cpp	(working copy)
>@@ -408,16 +408,19 @@
> 
> bool AccessibilityObject::isLandmark() const
> {
>-    AccessibilityRole role = roleValue();
>-    
>-    return role == AccessibilityRole::LandmarkBanner
>-        || role == AccessibilityRole::LandmarkComplementary
>-        || role == AccessibilityRole::LandmarkContentInfo
>-        || role == AccessibilityRole::LandmarkDocRegion
>-        || role == AccessibilityRole::LandmarkMain
>-        || role == AccessibilityRole::LandmarkNavigation
>-        || role == AccessibilityRole::LandmarkRegion
>-        || role == AccessibilityRole::LandmarkSearch;
>+    switch (roleValue()) {
>+    case AccessibilityRole::LandmarkBanner:
>+    case AccessibilityRole::LandmarkComplementary:
>+    case AccessibilityRole::LandmarkContentInfo:
>+    case AccessibilityRole::LandmarkDocRegion:
>+    case AccessibilityRole::LandmarkMain:
>+    case AccessibilityRole::LandmarkNavigation:
>+    case AccessibilityRole::LandmarkRegion:
>+    case AccessibilityRole::LandmarkSearch:
>+        return true;
>+    default:
>+        return false;
>+    }
> }
> 
> bool AccessibilityObject::hasMisspelling() const
>Index: Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityRenderObject.cpp	(revision 241863)
>+++ Source/WebCore/accessibility/AccessibilityRenderObject.cpp	(working copy)
>@@ -1273,6 +1273,7 @@
>     case AccessibilityRole::DescriptionListDetail:
>     case AccessibilityRole::Details:
>     case AccessibilityRole::DocumentArticle:
>+    case AccessibilityRole::Footer:
>     case AccessibilityRole::LandmarkRegion:
>     case AccessibilityRole::ListItem:
>     case AccessibilityRole::Time:
>@@ -2627,6 +2628,15 @@
>     return nullptr;
> }
> 
>+bool AccessibilityRenderObject::isDescendantOfElementType(const HashSet<QualifiedName>& tagNames) const
>+{
>+    for (auto& ancestor : ancestorsOfType<RenderElement>(*m_renderer)) {
>+        if (ancestor.element() && tagNames.contains(ancestor.element()->tagQName()))
>+            return true;
>+    }
>+    return false;
>+}
>+
> bool AccessibilityRenderObject::isDescendantOfElementType(const QualifiedName& tagName) const
> {
>     for (auto& ancestor : ancestorsOfType<RenderElement>(*m_renderer)) {
>@@ -2852,10 +2862,16 @@
> 
>     // There should only be one banner/contentInfo per page. If header/footer are being used within an article or section
>     // then it should not be exposed as whole page's banner/contentInfo
>-    if (node && node->hasTagName(headerTag) && !isDescendantOfElementType(articleTag) && !isDescendantOfElementType(sectionTag))
>+    if (node && node->hasTagName(headerTag) && !isDescendantOfElementType({ articleTag, sectionTag }))
>         return AccessibilityRole::LandmarkBanner;
>-    if (node && node->hasTagName(footerTag) && !isDescendantOfElementType(articleTag) && !isDescendantOfElementType(sectionTag))
>+
>+    // http://webkit.org/b/190138 Footers should become contentInfo's if scoped to body (and consequently become a landmark).
>+    // It should remain a footer if scoped to main, sectioning elements (article, section) or root sectioning element (blockquote, details, dialog, fieldset, figure, td).
>+    if (node && node->hasTagName(footerTag)) {
>+        if (!isDescendantOfElementType({ articleTag, sectionTag, mainTag, blockquoteTag, detailsTag, fieldsetTag, figureTag, tdTag }))
>+            return AccessibilityRole::LandmarkContentInfo;
>         return AccessibilityRole::Footer;
>+    }
>     
>     // menu tags with toolbar type should have Toolbar role.
>     if (node && node->hasTagName(menuTag) && equalLettersIgnoringASCIICase(getAttribute(typeAttr), "toolbar"))
>Index: Source/WebCore/accessibility/AccessibilityRenderObject.h
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityRenderObject.h	(revision 241863)
>+++ Source/WebCore/accessibility/AccessibilityRenderObject.h	(working copy)
>@@ -244,7 +244,8 @@
>     bool renderObjectIsObservable(RenderObject&) const;
>     RenderObject* renderParentObject() const;
>     bool isDescendantOfElementType(const QualifiedName& tagName) const;
>-    
>+    bool isDescendantOfElementType(const HashSet<QualifiedName>&) const;
>+
>     bool isSVGImage() const;
>     void detachRemoteSVGRoot();
>     enum CreationChoice { Create, Retrieve };
>Index: Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm
>===================================================================
>--- Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm	(revision 241863)
>+++ Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm	(working copy)
>@@ -526,7 +526,6 @@
>     case AccessibilityRole::Document:
>     case AccessibilityRole::DocumentArticle:
>     case AccessibilityRole::DocumentNote:
>-    case AccessibilityRole::Footer:
>     case AccessibilityRole::LandmarkBanner:
>     case AccessibilityRole::LandmarkComplementary:
>     case AccessibilityRole::LandmarkContentInfo:
>Index: Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
>===================================================================
>--- Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	(revision 241863)
>+++ Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	(working copy)
>@@ -2053,7 +2053,9 @@
>         return NSAccessibilityContentSeparatorSubrole;
>     if (role == AccessibilityRole::ToggleButton)
>         return NSAccessibilityToggleSubrole;
>-    
>+    if (role == AccessibilityRole::Footer)
>+        return @"AXFooter";
>+
>     if (is<AccessibilitySpinButtonPart>(*m_object)) {
>         if (downcast<AccessibilitySpinButtonPart>(*m_object).isIncrementor())
>             return NSAccessibilityIncrementArrowSubrole;
>@@ -2085,8 +2087,6 @@
>         return @"AXLandmarkBanner";
>     case AccessibilityRole::LandmarkComplementary:
>         return @"AXLandmarkComplementary";
>-    // Footer roles should appear as content info types.
>-    case AccessibilityRole::Footer:
>     case AccessibilityRole::LandmarkContentInfo:
>         return @"AXLandmarkContentInfo";
>     case AccessibilityRole::LandmarkMain:
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 241863)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2019-02-23  Chris Fleizach  <cfleizach@apple.com>
>+
>+        AX: <footer> HTML5 tag not reading as ARIA Landmark to VoiceOver
>+        https://bugs.webkit.org/show_bug.cgi?id=190138
>+        <rdar://problem/44907695>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/mac/footer-expected.txt:
>+        * accessibility/mac/footer-roledescription-expected.txt:
>+        * accessibility/mac/footer-roledescription.html:
>+        * accessibility/mac/footer.html:
>+        * accessibility/roles-computedRoleString-expected.txt:
>+        * accessibility/roles-computedRoleString.html:
>+        * platform/gtk/accessibility/roles-computedRoleString-expected.txt:
>+        * platform/mac-wk2/accessibility/roles-exposed-expected.txt:
>+        * platform/mac/accessibility/roles-computedRoleString-expected.txt:
>+        * platform/mac/accessibility/roles-exposed-expected.txt:
>+
> 2019-02-20  Antti Koivisto  <antti@apple.com>
> 
>         Make programmatic frame scrolling work on iOS
>Index: LayoutTests/accessibility/mac/footer-expected.txt
>===================================================================
>--- LayoutTests/accessibility/mac/footer-expected.txt	(revision 241863)
>+++ LayoutTests/accessibility/mac/footer-expected.txt	(working copy)
>@@ -1,9 +1,9 @@
> PASS accessibilityController.focusedElement.role is "AXRole: AXGroup"
>+PASS accessibilityController.focusedElement.roleDescription is "AXRoleDescription: content information"
>+PASS accessibilityController.focusedElement.role is "AXRole: AXGroup"
> PASS accessibilityController.focusedElement.roleDescription is "AXRoleDescription: footer"
> PASS accessibilityController.focusedElement.role is "AXRole: AXGroup"
>-PASS accessibilityController.focusedElement.roleDescription is "AXRoleDescription: group"
>-PASS accessibilityController.focusedElement.role is "AXRole: AXGroup"
>-PASS accessibilityController.focusedElement.roleDescription is "AXRoleDescription: group"
>+PASS accessibilityController.focusedElement.roleDescription is "AXRoleDescription: footer"
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>Index: LayoutTests/accessibility/mac/footer-roledescription-expected.txt
>===================================================================
>--- LayoutTests/accessibility/mac/footer-roledescription-expected.txt	(revision 241863)
>+++ LayoutTests/accessibility/mac/footer-roledescription-expected.txt	(working copy)
>@@ -1,14 +1,23 @@
>-footer
>-content info
>-This tests that a footer element has a different role description from a contentinfo
>+This tests that a footer element and content info has the right role at the right time.
> 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> 
> 
>-PASS footer.role is contentInfo.role
>-PASS footer.subrole is contentInfo.subrole
>-PASS footer.roleDescription is 'AXRoleDescription: footer'
>-PASS contentInfo.roleDescription is 'AXRoleDescription: content information'
>+PASS footer1.role is 'AXRole: AXGroup'
>+PASS footer1.subrole is 'AXSubrole: AXLandmarkContentInfo'
>+PASS footer1.roleDescription is 'AXRoleDescription: content information'
>+PASS contentinfo.role is 'AXRole: AXGroup'
>+PASS contentinfo.subrole is 'AXSubrole: AXLandmarkContentInfo'
>+PASS contentinfo.roleDescription is 'AXRoleDescription: content information'
>+PASS footer2.role is 'AXRole: AXGroup'
>+PASS footer2.subrole is 'AXSubrole: AXFooter'
>+PASS footer2.roleDescription is 'AXRoleDescription: footer'
>+PASS footer3.role is 'AXRole: AXGroup'
>+PASS footer3.subrole is 'AXSubrole: AXFooter'
>+PASS footer3.roleDescription is 'AXRoleDescription: footer'
>+PASS footer4.role is 'AXRole: AXGroup'
>+PASS footer4.subrole is 'AXSubrole: AXFooter'
>+PASS footer4.roleDescription is 'AXRoleDescription: footer'
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>Index: LayoutTests/accessibility/mac/footer-roledescription.html
>===================================================================
>--- LayoutTests/accessibility/mac/footer-roledescription.html	(revision 241863)
>+++ LayoutTests/accessibility/mac/footer-roledescription.html	(working copy)
>@@ -5,29 +5,69 @@
> </head>
> <body id="body">
> 
>-<footer>footer</footer>
>-<div role="contentinfo">content info</div>
>+<div id="content">
>+
>+<footer id="footer1">footer that is contentinfo</footer>
>+
>+<div role="contentinfo" id="contentinfo">content info</div>
>+
>+<main>
>+  <footer id="footer2">footer that is not contentinfo</footer>
>+</main>
>+
>+<blockquote>
>+  <footer id="footer3">footer that is not contentinfo</footer>
>+</blockquote>
>+
>+<fieldset>
>+  <footer id="footer3">footer that is not contentinfo</footer>
>+</fieldset>
>+
>+<details open>
>+  <footer id="footer4">footer that is not contentinfo</footer>
>+</details>
>+
>+</div>
>+
> <p id="description"></p>
> <div id="console"></div>
> 
> <script>
> 
>-    description("This tests that a footer element has a different role description from a contentinfo");
>+    description("This tests that a footer element and content info has the right role at the right time.");
> 
>     if (window.accessibilityController) {
> 
>-          // this text field should be required.
>-          document.getElementById("body").focus();
>-          var obj = accessibilityController.focusedElement;
>+          // Scoped to body, becomes a content info.
>+          var footer1 = accessibilityController.accessibleElementById("footer1");
>+          var contentinfo = accessibilityController.accessibleElementById("contentinfo");
> 
>-          var footer = obj.childAtIndex(0);
>-          var contentInfo = obj.childAtIndex(1);
>+          // Scoped to other element types, not a content info.
>+          var footer2 = accessibilityController.accessibleElementById("footer2");
>+          var footer3 = accessibilityController.accessibleElementById("footer3");
>+          var footer4 = accessibilityController.accessibleElementById("footer4");
>          
>-          shouldBe("footer.role", "contentInfo.role");
>-          shouldBe("footer.subrole", "contentInfo.subrole");
>-          shouldBe("footer.roleDescription", "'AXRoleDescription: footer'");
>-          shouldBe("contentInfo.roleDescription", "'AXRoleDescription: content information'");
>+          shouldBe("footer1.role", "'AXRole: AXGroup'");
>+          shouldBe("footer1.subrole", "'AXSubrole: AXLandmarkContentInfo'");
>+          shouldBe("footer1.roleDescription", "'AXRoleDescription: content information'");
> 
>+          shouldBe("contentinfo.role", "'AXRole: AXGroup'");
>+          shouldBe("contentinfo.subrole", "'AXSubrole: AXLandmarkContentInfo'");
>+          shouldBe("contentinfo.roleDescription", "'AXRoleDescription: content information'");
>+
>+          shouldBe("footer2.role", "'AXRole: AXGroup'");
>+          shouldBe("footer2.subrole", "'AXSubrole: AXFooter'");
>+          shouldBe("footer2.roleDescription", "'AXRoleDescription: footer'");
>+
>+          shouldBe("footer3.role", "'AXRole: AXGroup'");
>+          shouldBe("footer3.subrole", "'AXSubrole: AXFooter'");
>+          shouldBe("footer3.roleDescription", "'AXRoleDescription: footer'");
>+
>+          shouldBe("footer4.role", "'AXRole: AXGroup'");
>+          shouldBe("footer4.subrole", "'AXSubrole: AXFooter'");
>+          shouldBe("footer4.roleDescription", "'AXRoleDescription: footer'");
>+
>+          document.getElementById("content").style.visibility = "hidden";
>     }
> 
> </script>
>Index: LayoutTests/accessibility/mac/footer.html
>===================================================================
>--- LayoutTests/accessibility/mac/footer.html	(revision 241863)
>+++ LayoutTests/accessibility/mac/footer.html	(working copy)
>@@ -27,9 +27,9 @@
>             shouldBeEqualToString('accessibilityController.focusedElement.roleDescription', "AXRoleDescription: " + expectedDescription);
>         }
> 
>-        expectRole('AXGroup', 'footer', '#footer01');
>-        expectRole('AXGroup', 'group', '#footer02');
>-        expectRole('AXGroup', 'group', '#footer03');
>+        expectRole('AXGroup', 'content information', '#footer01');
>+        expectRole('AXGroup', 'footer', '#footer02');
>+        expectRole('AXGroup', 'footer', '#footer03');
>     </script>
> <script src='../../resources/js-test-post.js'></script>
> </body>
>Index: LayoutTests/accessibility/roles-computedRoleString-expected.txt
>===================================================================
>--- LayoutTests/accessibility/roles-computedRoleString-expected.txt	(revision 241863)
>+++ LayoutTests/accessibility/roles-computedRoleString-expected.txt	(working copy)
>@@ -9,7 +9,7 @@
> PASS: button -> button. 
> PASS: dfn -> definition. 
> PASS: dl -> . 
>-PASS: footer -> . 
>+PASS: footer -> contentinfo. 
> PASS: form -> form. 
> PASS: header -> banner. 
> PASS: h1 -> heading. 
>Index: LayoutTests/accessibility/roles-computedRoleString.html
>===================================================================
>--- LayoutTests/accessibility/roles-computedRoleString.html	(revision 241863)
>+++ LayoutTests/accessibility/roles-computedRoleString.html	(working copy)
>@@ -17,7 +17,7 @@
>     <dt>X</dt>
>     <dd>X</dd>
> </dl>
>-<footer data-role="" data-platform="atk,mac" class="ex">X</footer>
>+<footer data-role="contentinfo" data-platform="atk,mac" class="ex">X</footer>
> <form data-role="form" data-platform="atk,mac" class="ex">X</form>
> <header data-role="banner" data-platform="atk,mac" class="ex">X</header>
> <h1 data-role="heading" data-platform="atk,mac" class="ex">X</h1>
>Index: LayoutTests/platform/gtk/accessibility/roles-computedRoleString-expected.txt
>===================================================================
>--- LayoutTests/platform/gtk/accessibility/roles-computedRoleString-expected.txt	(revision 241863)
>+++ LayoutTests/platform/gtk/accessibility/roles-computedRoleString-expected.txt	(working copy)
>@@ -9,7 +9,7 @@
> PASS: button -> button. 
> PASS: dfn -> definition. 
> PASS: dl -> . 
>-PASS: footer -> . 
>+PASS: footer -> contentinfo. 
> PASS: form -> form. 
> PASS: header -> banner. 
> PASS: h1 -> heading. 
>Index: LayoutTests/platform/mac/accessibility/roles-computedRoleString-expected.txt
>===================================================================
>--- LayoutTests/platform/mac/accessibility/roles-computedRoleString-expected.txt	(revision 241863)
>+++ LayoutTests/platform/mac/accessibility/roles-computedRoleString-expected.txt	(working copy)
>@@ -9,7 +9,7 @@
> PASS: button -> button. 
> PASS: dfn -> definition. 
> PASS: dl -> . 
>-PASS: footer -> . 
>+PASS: footer -> contentinfo. 
> PASS: form -> form. 
> PASS: header -> banner. 
> PASS: h1 -> heading. 
>Index: LayoutTests/platform/mac/accessibility/roles-exposed-expected.txt
>===================================================================
>--- LayoutTests/platform/mac/accessibility/roles-exposed-expected.txt	(revision 241863)
>+++ LayoutTests/platform/mac/accessibility/roles-exposed-expected.txt	(working copy)
>@@ -132,7 +132,7 @@
> footer
>       AXRole: AXGroup
>       AXSubrole: AXLandmarkContentInfo
>-      AXRoleDescription: footer
>+      AXRoleDescription: content information 
>       
> form
>       AXRole: AXGroup
>Index: LayoutTests/platform/mac-wk2/accessibility/roles-exposed-expected.txt
>===================================================================
>--- LayoutTests/platform/mac-wk2/accessibility/roles-exposed-expected.txt	(revision 241863)
>+++ LayoutTests/platform/mac-wk2/accessibility/roles-exposed-expected.txt	(working copy)
>@@ -132,7 +132,7 @@
> footer
>       AXRole: AXGroup
>       AXSubrole: AXLandmarkContentInfo
>-      AXRoleDescription: footer
>+      AXRoleDescription: content information
>       
> form
>       AXRole: AXGroup

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 190138: 351243 | 362043 | 362827 | 362828 | 362830 | 362831 | 362886