[ContentChangeObserver] Use aria role as a hint whether a tap should result in a...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Apr 2019 18:57:51 +0000 (18:57 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Apr 2019 18:57:51 +0000 (18:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196988
<rdar://problem/49955328>

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html

* accessibility/AccessibilityObject.h:

Source/WebKit:

Tapping on elements with cretain aria role attributes should trigger synthetic click the same way it does on form control elements.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::isAriaRoleForImmediateClick):
(WebKit::nodeAlwaysRequiresClick):
(WebKit::WebPage::handleSyntheticClick):

LayoutTests:

* fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role-expected.txt: Added.
* fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/accessibility/AccessibilityObject.h
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 89307d9..8af7f82 100644 (file)
@@ -1,3 +1,14 @@
+2019-04-17  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Use aria role as a hint whether a tap should result in a synthetic click
+        https://bugs.webkit.org/show_bug.cgi?id=196988
+        <rdar://problem/49955328>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html: Added.
+
 2019-04-17  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         REGRESSION (r243926): [iOS] Release assertion when computing editor state during an overflow scroll triggered by layout
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role-expected.txt
new file mode 100644 (file)
index 0000000..677e3a3
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html b/LayoutTests/fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html
new file mode 100644 (file)
index 0000000..ad397fb
--- /dev/null
@@ -0,0 +1,57 @@
+<html>
+<head>
+<title>This tests the case when the tap target node has form control aria role.</title>
+<script src="../../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+    visibility: hidden;
+    width: 100px;
+    height: 100px;
+    background-color: green;
+}
+</style>
+<script>
+async function test() {
+    if (!window.testRunner || !testRunner.runUIScript)
+        return;
+    if (window.internals)
+        internals.settings.setContentChangeObserverEnabled(true);
+
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+
+    let rect = tapthis.getBoundingClientRect();
+    let x = rect.left + rect.width / 2;
+    let y = rect.top + rect.height / 2;
+
+    await tapAtPoint(x, y);
+}
+</script>
+</head>
+<body onload="test()">
+<div id=tapthis role="button">PASS if 'clicked' text is shown below.</div>
+<div id=becomesVisible></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mousemove", function( event ) {
+    becomesVisible.style.visibility = "visible";
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
index a457e8c..ce6a674 100644 (file)
@@ -1,3 +1,15 @@
+2019-04-17  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Use aria role as a hint whether a tap should result in a synthetic click
+        https://bugs.webkit.org/show_bug.cgi?id=196988
+        <rdar://problem/49955328>
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/events/touch/ios/content-observation/tap-on-input-looking-div-with-role.html
+
+        * accessibility/AccessibilityObject.h:
+
 2019-04-17  Alex Christensen  <achristensen@webkit.org>
 
         WebSocketHandshake should not know about a Document
index 94cca04..c00557e 100644 (file)
@@ -2514,7 +2514,9 @@ static ARIAReverseRoleMap& reverseAriaRoleMap()
 
 AccessibilityRole AccessibilityObject::ariaRoleToWebCoreRole(const String& value)
 {
-    ASSERT(!value.isEmpty());
+    if (value.isNull() || value.isEmpty())
+        return AccessibilityRole::Unknown;
+
     for (auto roleName : StringView(value).split(' ')) {
         AccessibilityRole role = ariaRoleMap().get<ASCIICaseInsensitiveStringViewHashTranslator>(roleName);
         if (static_cast<int>(role))
index d4f756a..b749368 100644 (file)
@@ -525,8 +525,8 @@ public:
     AccessibilityObject* selectedListItem();
     virtual int layoutCount() const { return 0; }
     virtual double estimatedLoadingProgress() const { return 0; }
-    static bool isARIAControl(AccessibilityRole);
-    static bool isARIAInput(AccessibilityRole);
+    WEBCORE_EXPORT static bool isARIAControl(AccessibilityRole);
+    WEBCORE_EXPORT static bool isARIAInput(AccessibilityRole);
 
     virtual bool supportsARIAOwns() const { return false; }
     bool isActiveDescendantOfFocusedContainer() const;
@@ -755,7 +755,7 @@ public:
     bool isAncestorOfObject(const AccessibilityObject*) const;
     AccessibilityObject* firstAnonymousBlockChild() const;
 
-    static AccessibilityRole ariaRoleToWebCoreRole(const String&);
+    WEBCORE_EXPORT static AccessibilityRole ariaRoleToWebCoreRole(const String&);
     bool hasAttribute(const QualifiedName&) const;
     const AtomicString& getAttribute(const QualifiedName&) const;
     bool hasTagName(const QualifiedName&) const;
index a9498dc..bcee55c 100644 (file)
@@ -1,3 +1,18 @@
+2019-04-17  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Use aria role as a hint whether a tap should result in a synthetic click
+        https://bugs.webkit.org/show_bug.cgi?id=196988
+        <rdar://problem/49955328>
+
+        Reviewed by Simon Fraser.
+
+        Tapping on elements with cretain aria role attributes should trigger synthetic click the same way it does on form control elements. 
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::isAriaRoleForImmediateClick):
+        (WebKit::nodeAlwaysRequiresClick):
+        (WebKit::WebPage::handleSyntheticClick):
+
 2019-04-17  Alex Christensen  <achristensen@webkit.org>
 
         WKRetainPtr's adoption constructor should be private
index 90d7b73..86ae4ca 100644 (file)
@@ -566,6 +566,18 @@ static void dispatchSyntheticMouseMove(Frame& mainFrame, const WebCore::FloatPoi
     mainFrame.eventHandler().dispatchSyntheticMouseMove(mouseEvent);
 }
 
+static bool nodeAlwaysTriggersClick(const Node& targetNode)
+{
+    if (!is<Element>(targetNode))
+        return false;
+
+    if (is<HTMLFormControlElement>(targetNode))
+        return true;
+
+    auto ariaRole = AccessibilityObject::ariaRoleToWebCoreRole(downcast<Element>(targetNode).getAttribute(HTMLNames::roleAttr));
+    return AccessibilityObject::isARIAControl(ariaRole) || AccessibilityObject::isARIAInput(ariaRole);
+}
+
 void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::FloatPoint& location, OptionSet<WebEvent::Modifier> modifiers)
 {
     if (!nodeRespondingToClick.document().settings().contentChangeObserverEnabled()) {
@@ -587,8 +599,9 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
 
     auto& contentChangeObserver = respondingDocument.contentChangeObserver();
     auto observedContentChange = contentChangeObserver.observedContentChange();
+    auto targetNodeTriggersClick = nodeAlwaysTriggersClick(nodeRespondingToClick);
 
-    auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || is<HTMLFormControlElement>(nodeRespondingToClick));
+    auto continueContentObservation = !(observedContentChange == WKContentVisibilityChange || targetNodeTriggersClick);
     if (continueContentObservation) {
         // Wait for callback to completePendingSyntheticClickForContentChangeObserver() to decide whether to send the click event.
         const Seconds observationDuration = 32_ms;
@@ -600,11 +613,11 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
         return;
     }
 
-    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange] {
+    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(nodeRespondingToClick), location, modifiers, observedContentChange, targetNodeTriggersClick] {
         if (protectedThis->m_isClosed || !protectedThis->corePage())
             return;
 
-        auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange && !is<HTMLFormControlElement>(targetNode);
+        auto shouldStayAtHoverState = observedContentChange == WKContentVisibilityChange && !targetNodeTriggersClick;
         if (shouldStayAtHoverState) {
             // The move event caused new contents to appear. Don't send synthetic click event, but just ensure that the mouse is on the most recent content.
             dispatchSyntheticMouseMove(protectedThis->corePage()->mainFrame(), location, modifiers);