Attachment 372628 Details for Bug 199107 – Patch

[patch] Patch

bug-199107-20190621101009.patch (text/plain), 13.30 KB, created by Andres Gonzalez on 2019-06-21 07:10:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andres Gonzalez

Created: 2019-06-21 07:10:10 PDT

Size: 13.30 KB

patch

obsolete

>Subversion Revision: 246586
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c571b58a80f3ebdd94d7bc2760647ccd15dac75f..76ee6a81ea19b40a3b196f7524f9f8e28ef3b20b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-06-21  Andres Gonzalez  <andresg_22@apple.com>
>+
>+        Accessibility objects contained in links should honor the aria-haspopup attribute in the ancestor link.
>+        https://bugs.webkit.org/show_bug.cgi?id=199107
>+        <rdar://problem/43663611>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: accessibility/ios-simulator/button-with-aria-haspopup-role.html
>+               accessibility/ios-simulator/element-haspopup.html
>+
>+        If an accessibility object is contained within an <a>, we should check
>+        the aria-haspopup attribute on the ancestor <a> element. This was done
>+        before in the iOS WebKit accessibility bunddle override.
>+        * accessibility/AccessibilityRenderObject.cpp:
>+        (WebCore::AccessibilityRenderObject::hasPopup const):
>+
> 2019-06-18  Saam Barati  <sbarati@apple.com>
> 
>         [WHLSL] Support matrices
>diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>index edbdb81ee412eaf7a3a2e41ea30dfbf8969ee9a4..a1df8e3d78cf174cd24e33fdabfe991f54cb3872 100644
>--- a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>@@ -1029,9 +1029,21 @@ bool AccessibilityRenderObject::hasTextAlternative() const
>     // override the "label" element association.
>     return ariaAccessibilityDescription().length();
> }
>-    
>+
> bool AccessibilityRenderObject::hasPopup() const
> {
>+    // If this has a link ancestor, check for the aria-haspopup attribute in the ancestor.
>+    Node* node = this->node();
>+    while (node && !is<HTMLAnchorElement>(node)) {
>+        node = node->parentNode();
>+
>+        if (node && is<HTMLAnchorElement>(node)) {
>+            AXObjectCache* objCache = axObjectCache();
>+            if (objCache)
>+                return !equalLettersIgnoringASCIICase(objCache->getOrCreate(node)->hasPopupValue(), "false");
>+        }
>+    }
>+
>     return !equalLettersIgnoringASCIICase(hasPopupValue(), "false");
> }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 6c8bb2fab96e79db756f545f7d59eefd0ed9986a..8d65a9169a98f27d8d3b227b3eedc8819100c743 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-21  Andres Gonzalez  <andresg_22@apple.com>
>+
>+        Accessibility objects contained in links should honor the aria-haspopup attribute in the ancestor link.
>+        https://bugs.webkit.org/show_bug.cgi?id=199107
>+        <rdar://problem/43663611>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        iOS implementation for AccessibilityUIElement::hasPopup.
>+        * WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
>+        (WTR::AccessibilityUIElement::hasPopup const):
>+
> 2019-06-18  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] JSLock should be WebThread aware
>diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
>index c60640e375f30cd250dff8fc4ef870b37c68804e..f34deed472cf6b22cc8b33f9c7d17fc9e8e837cb 100644
>--- a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
>+++ b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
>@@ -86,6 +86,7 @@ - (BOOL)_accessibilityIsStrongPasswordField;
> - (CGRect)accessibilityVisibleContentRect;
> - (NSString *)accessibilityTextualContext;
> - (NSString *)accessibilityRoleDescription;
>+- (BOOL)accessibilityHasPopup;
> - (NSString *)accessibilityColorStringValue;
> 
> // TextMarker related
>@@ -1054,7 +1055,7 @@ bool AccessibilityUIElement::isMultiLine() const
> 
> bool AccessibilityUIElement::hasPopup() const
> {
>-    return false;
>+    return [m_element accessibilityHasPopup];
> }
> 
> void AccessibilityUIElement::takeFocus()
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 46d15ac6d5f0590f8f9fc336c108cd84a56b0bbf..c65f01f96531101f0a8e67a9aa566bd877cc04dc 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-06-21  Andres Gonzalez  <andresg_22@apple.com>
>+
>+        Accessibility objects contained in links should honor the aria-haspopup attribute in the ancestor link.
>+        https://bugs.webkit.org/show_bug.cgi?id=199107
>+        <rdar://problem/43663611>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/element-haspopup-expected.txt:
>+        * accessibility/element-haspopup.html: Added cases for a <p> element contained within <a>.
>+        * accessibility/ios-simulator/button-with-aria-haspopup-role-expected.txt: Added.
>+        * accessibility/ios-simulator/button-with-aria-haspopup-role.html: Added. Existed for MacOS and now also for iOS.
>+        * accessibility/ios-simulator/element-haspopup-expected.txt: Copied from LayoutTests/accessibility/element-haspopup-expected.txt.
>+        * accessibility/ios-simulator/element-haspopup.html: Added.
>+
> 2019-06-19  Cathie Chen  <cathiechen@igalia.com>
> 
>         resize-observer/element-leak.html is a flaky failure
>diff --git a/LayoutTests/accessibility/element-haspopup-expected.txt b/LayoutTests/accessibility/element-haspopup-expected.txt
>index c7b511c471da67df654ec4be09f7cd92aef7e19e..0ccc44f681203f07db361e84389e34eb7300b191 100644
>--- a/LayoutTests/accessibility/element-haspopup-expected.txt
>+++ b/LayoutTests/accessibility/element-haspopup-expected.txt
>@@ -1,4 +1,10 @@
> Link 1 Link 2
>+Paragraph 1.
>+
>+Paragraph 2.
>+
>+Paragraph 3.
>+
> This tests that an element will report that it has a popup correctly.
> 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>@@ -6,6 +12,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
> 
> PASS accessibilityController.focusedElement.hasPopup is true
> PASS accessibilityController.focusedElement.hasPopup is false
>+PASS p1.hasPopup is true
>+PASS p2.hasPopup is false
>+PASS p3.hasPopup is false
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>diff --git a/LayoutTests/accessibility/element-haspopup.html b/LayoutTests/accessibility/element-haspopup.html
>index 50c48cae24b9d6b4d8f46b86e2cd765f6566a3c3..2cf0c317b1cf35c189c49adf8e74f487a4850264 100644
>--- a/LayoutTests/accessibility/element-haspopup.html
>+++ b/LayoutTests/accessibility/element-haspopup.html
>@@ -8,20 +8,39 @@
> <a href="#" aria-haspopup="true" tabindex="0" id="link1">Link 1</a>
> <a href="#" aria-haspopup="false" tabindex="0" id="link2">Link 2</a>
> 
>+<a href="#" aria-haspopup="true" tabindex="0" id="link3">
>+    <p id="paragraph1">Paragraph 1.</p>
>+</a>
>+
>+<a href="#" aria-haspopup="false" tabindex="0" id="link4">
>+    <p id="paragraph2">Paragraph 2.</p>
>+</a>
>+
>+<a href="#" tabindex="0" id="link5">
>+    <p id="paragraph3">Paragraph 3.</p>
>+</a>
>+
> <p id="description"></p>
> <div id="console"></div>
> 
> <script>
>-
>     description("This tests that an element will report that it has a popup correctly.");
> 
>     if (window.accessibilityController) {
>-
>         document.getElementById("link1").focus();
>         shouldBe("accessibilityController.focusedElement.hasPopup", "true");
> 
>         document.getElementById("link2").focus();
>         shouldBe("accessibilityController.focusedElement.hasPopup", "false");
>+
>+        var p1 = accessibilityController.accessibleElementById("paragraph1");
>+        shouldBe("p1.hasPopup", "true");
>+
>+        var p2 = accessibilityController.accessibleElementById("paragraph2");
>+        shouldBe("p2.hasPopup", "false");
>+
>+        var p3 = accessibilityController.accessibleElementById("paragraph3");
>+        shouldBe("p3.hasPopup", "false");
>     }
> 
> </script>
>diff --git a/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role-expected.txt b/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..686ccde197b05141637a1b04552815a584766fd8
>--- /dev/null
>+++ b/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role-expected.txt
>@@ -0,0 +1,19 @@
>+This tests the platform role exposed for buttons with aria-haspopup
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+test1 AXTraits: 8256 for aria-haspopup=(null)
>+test2 AXTraits: 32832 for aria-haspopup='true'
>+test3 AXTraits: 8256 for aria-haspopup='false'
>+test4 AXTraits: 32832 for aria-haspopup='dialog'
>+test5 AXTraits: 32832 for aria-haspopup='grid'
>+test6 AXTraits: 32832 for aria-haspopup='listbox'
>+test7 AXTraits: 32832 for aria-haspopup='menu'
>+test8 AXTraits: 32832 for aria-haspopup='tree'
>+test9 AXTraits: 8256 for aria-haspopup='foo'
>+test10 AXTraits: 8256 for aria-haspopup=''
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role.html b/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e3b0568cef5db32bae82e46eae92b328296abb19
>--- /dev/null
>+++ b/LayoutTests/accessibility/ios-simulator/button-with-aria-haspopup-role.html
>@@ -0,0 +1,36 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../../resources/js-test-pre.js"></script>
>+</head>
>+<body id="body">
>+<div id="content">
>+  <div id="test1" role="button">X</div>
>+  <div id="test2" role="button" aria-haspopup="true">X</div>
>+  <div id="test3" role="button" aria-haspopup="false">X</div>
>+  <div id="test4" role="button" aria-haspopup="dialog">X</div>
>+  <div id="test5" role="button" aria-haspopup="grid">X</div>
>+  <div id="test6" role="button" aria-haspopup="listbox">X</div>
>+  <div id="test7" role="button" aria-haspopup="menu">X</div>
>+  <div id="test8" role="button" aria-haspopup="tree">X</div>
>+  <div id="test9" role="button" aria-haspopup="foo">X</div>
>+  <div id="test10" role="button" aria-haspopup="">X</div>
>+</div>
>+<p id="description"></p>
>+<div id="console"></div>
>+<script>
>+    description("This tests the platform role exposed for buttons with aria-haspopup");
>+    if (window.accessibilityController) {
>+        for (var i = 1; i <= 10; i++) {
>+            var element = document.getElementById("test" + i)
>+            var popup = element.hasAttribute("aria-haspopup") ? "'" + element.getAttribute("aria-haspopup") + "'" : "(null)";
>+            var axElement = accessibilityController.accessibleElementById("test" + i);
>+            debug("test" + i + " " + axElement.traits + " for aria-haspopup=" + popup)
>+        }
>+
>+        document.getElementById("content").style.visibility = "hidden";
>+    }
>+</script>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/accessibility/ios-simulator/element-haspopup-expected.txt b/LayoutTests/accessibility/ios-simulator/element-haspopup-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..0ccc44f681203f07db361e84389e34eb7300b191
>--- /dev/null
>+++ b/LayoutTests/accessibility/ios-simulator/element-haspopup-expected.txt
>@@ -0,0 +1,21 @@
>+Link 1 Link 2
>+Paragraph 1.
>+
>+Paragraph 2.
>+
>+Paragraph 3.
>+
>+This tests that an element will report that it has a popup correctly.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS accessibilityController.focusedElement.hasPopup is true
>+PASS accessibilityController.focusedElement.hasPopup is false
>+PASS p1.hasPopup is true
>+PASS p2.hasPopup is false
>+PASS p3.hasPopup is false
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/accessibility/ios-simulator/element-haspopup.html b/LayoutTests/accessibility/ios-simulator/element-haspopup.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2c008b970cf77f39fc9a351ff8f2d5a344729c19
>--- /dev/null
>+++ b/LayoutTests/accessibility/ios-simulator/element-haspopup.html
>@@ -0,0 +1,50 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../../resources/js-test-pre.js"></script>
>+</head>
>+<body id="body">
>+
>+<a href="#" aria-haspopup="true" tabindex="0" id="link1">Link 1</a>
>+<a href="#" aria-haspopup="false" tabindex="0" id="link2">Link 2</a>
>+
>+<a href="#" aria-haspopup="true" tabindex="0" id="link3">
>+    <p id="paragraph1">Paragraph 1.</p>
>+</a>
>+
>+<a href="#" aria-haspopup="false" tabindex="0" id="link4">
>+    <p id="paragraph2">Paragraph 2.</p>
>+</a>
>+
>+<a href="#" tabindex="0" id="link5">
>+    <p id="paragraph3">Paragraph 3.</p>
>+</a>
>+
>+<p id="description"></p>
>+<div id="console"></div>
>+
>+<script>
>+    description("This tests that an element will report that it has a popup correctly.");
>+
>+    if (window.accessibilityController) {
>+        document.getElementById("link1").focus();
>+        shouldBe("accessibilityController.focusedElement.hasPopup", "true");
>+
>+        document.getElementById("link2").focus();
>+        shouldBe("accessibilityController.focusedElement.hasPopup", "false");
>+
>+        var p1 = accessibilityController.accessibleElementById("paragraph1");
>+        shouldBe("p1.hasPopup", "true");
>+
>+        var p2 = accessibilityController.accessibleElementById("paragraph2");
>+        shouldBe("p2.hasPopup", "false");
>+
>+        var p3 = accessibilityController.accessibleElementById("paragraph3");
>+        shouldBe("p3.hasPopup", "false");
>+    }
>+
>+</script>
>+
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199107: 372628 | 372644