iOS: An element with tabindex is not focusable unless there is no mouse event handler
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 21:57:37 +0000 (21:57 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Dec 2016 21:57:37 +0000 (21:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165843

Reviewed by Antti Koivisto.

Source/WebCore:

The bug was caused by ancestorRespondingToClickEvents not checking the precense of tabindex attribute.
Check that condition along with event listeners.

Test: fast/events/focusing-element-with-tabindex-by-tap-or-click.html

* page/ios/FrameIOS.mm:
(WebCore::ancestorRespondingToClickEvents):

Tools:

Add testRunner.isWebKit2 which is always true in WebKitTestRunner.
Without this, it's really hard to reliably differentiate DumpRenderTree and WebKitTestRunner,
and DumpRenderTree's runUIScript would hit an assertion :(

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.h:
(WTR::TestRunner::isWebKit2):

LayoutTests:

Added a regression test for focusing an element with just tabindex using UIHelper.

Also fixed UIHelper to work in iOS DumpRenderTree which was hitting an assertion
by explicitly checking testRunner.isWebKit2. Prior to fixing this, it was hitting
an assertion in RunLoop::main() which was asserting that there is a runloop,
which doesn't exist in DumpRenderTree.

* fast/events/focusing-element-with-tabindex-by-tap-or-click-expected.txt: Added.
* fast/events/focusing-element-with-tabindex-by-tap-or-click.html: Added.
* platform/ios-simulator-wk2/TestExpectations:
* resources/ui-helper.js:
(window.UIHelper.isWebKit2):
(window.UIHelper.wait): Added the support for js-test.js / js-test-pre.js style tests.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click.html [new file with mode: 0644]
LayoutTests/platform/ios-simulator-wk2/TestExpectations
LayoutTests/resources/ui-helper.js
Source/WebCore/ChangeLog
Source/WebCore/page/ios/FrameIOS.mm
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h

index 0782f4e..4252cc7 100644 (file)
@@ -1,3 +1,24 @@
+2016-12-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        iOS: An element with tabindex is not focusable unless there is no mouse event handler
+        https://bugs.webkit.org/show_bug.cgi?id=165843
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test for focusing an element with just tabindex using UIHelper.
+
+        Also fixed UIHelper to work in iOS DumpRenderTree which was hitting an assertion
+        by explicitly checking testRunner.isWebKit2. Prior to fixing this, it was hitting
+        an assertion in RunLoop::main() which was asserting that there is a runloop,
+        which doesn't exist in DumpRenderTree.
+
+        * fast/events/focusing-element-with-tabindex-by-tap-or-click-expected.txt: Added.
+        * fast/events/focusing-element-with-tabindex-by-tap-or-click.html: Added.
+        * platform/ios-simulator-wk2/TestExpectations:
+        * resources/ui-helper.js:
+        (window.UIHelper.isWebKit2):
+        (window.UIHelper.wait): Added the support for js-test.js / js-test-pre.js style tests.
+
 2016-12-14  Dave Hyatt  <hyatt@apple.com>
 
         [CSS Parser] Implement deferred parsing of properties, @media, @supports and @keyframes
diff --git a/LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click-expected.txt b/LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click-expected.txt
new file mode 100644 (file)
index 0000000..fc40106
--- /dev/null
@@ -0,0 +1,15 @@
+This tests tapping or clicking on an element with tabindex would focus the element.
+To manually test, tap or click on each element below. Each element should be focused
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS "Activated contentEditableElement"; document.activeElement is contentEditableElement
+PASS "Activated elementWithTabIndexAndClickHandler"; document.activeElement is elementWithTabIndexAndClickHandler
+PASS "Activated elementWithTabIndex"; document.activeElement is elementWithTabIndex
+PASS successfullyParsed is true
+
+TEST COMPLETE
+An element with contenteditable attribute
+An element with click event handler and tabindex attribute
+An element with tabindex attribute
diff --git a/LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click.html b/LayoutTests/fast/events/focusing-element-with-tabindex-by-tap-or-click.html
new file mode 100644 (file)
index 0000000..89ff636
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/ui-helper.js"></script>
+<script src="../../resources/js-test.js"></script>
+<div id="contentEditableElement" contenteditable>An element with contenteditable attribute</div>
+<section id="elementWithTabIndexAndClickHandler" onclick="false" tabindex="0">An element with click event handler and tabindex attribute</section>
+<main id="elementWithTabIndex" tabindex="0">An element with tabindex attribute</main>
+<style>
+div:focus {
+    background: #9cf;
+}
+</style>
+<script>
+
+description('This tests tapping or clicking on an element with tabindex would focus the element.<br>'
+    + 'To manually test, tap or click on each element below. Each element should be focused');
+jsTestIsAsync = true;
+
+async function runTests() {
+    await UIHelper.activateAt(contentEditableElement.offsetLeft + 5, contentEditableElement.offsetTop + 5);
+    shouldBe('"Activated contentEditableElement"; document.activeElement', 'contentEditableElement');
+
+    await UIHelper.activateAt(elementWithTabIndexAndClickHandler.offsetLeft + 5, elementWithTabIndexAndClickHandler.offsetTop + 5);
+    shouldBe('"Activated elementWithTabIndexAndClickHandler"; document.activeElement', 'elementWithTabIndexAndClickHandler');
+
+    await UIHelper.activateAt(elementWithTabIndex.offsetLeft + 5, elementWithTabIndex.offsetTop + 5);
+    shouldBe('"Activated elementWithTabIndex"; document.activeElement', 'elementWithTabIndex');
+}
+
+UIHelper.wait(runTests());
+
+</script>
+</body>
+</html>
index 5275a50..865cd19 100644 (file)
@@ -1774,16 +1774,17 @@ fast/dom/Window/post-message-crash.html [ Pass Failure ]
 webkit.org/b/123431 http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html [ Pass Failure ]
 
 # eventSender.mouseDown is not implemented
+fast/events/focusing-element-with-tabindex-by-tap-or-click.html [ Skip ]
 fast/loader/location-hash-user-gesture.html [ Skip ]
 imported/blink/editing/selection/selectstart-event-crash.html [ Skip ]
 fast/dom/Window/post-message-user-action.html [ Skip ]
 fast/images/image-usemap-parsing.html [ Skip ]
 fast/shadow-dom/click-on-slotted-anchor-with-hover.html [ Skip ]
 fast/shadow-dom/click-text-inside-linked-slot.html [ Skip ]
-fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html
-fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html
-fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html
-fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html
+fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html [ Skip ]
+fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html [ Skip ]
+fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html [ Skip ]
+fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html [ Skip ]
 
 # No touch events
 http/tests/contentdispositionattachmentsandbox/referer-header-stripped-with-meta-referer-always.html [ Skip ]
index 68c4b70..0042ce1 100644 (file)
@@ -5,9 +5,14 @@ window.UIHelper = class UIHelper {
         return navigator.userAgent.includes('iPhone');
     }
 
+    static isWebKit2()
+    {
+        return window.testRunner.isWebKit2;
+    }
+
     static activateAt(x, y)
     {
-        if (!testRunner.runUIScript || !this.isIOS()) {
+        if (!this.isWebKit2() || !this.isIOS()) {
             eventSender.mouseMoveTo(x, y);
             eventSender.mouseDown();
             eventSender.mouseUp();
@@ -25,8 +30,16 @@ window.UIHelper = class UIHelper {
     static wait(promise)
     {
         testRunner.waitUntilDone();
-        return promise.then(
-            function () { testRunner.notifyDone(); },
-            function (error) { testRunner.notifyDone(); return Promise.reject(error); });
+        if (window.finishJSTest)
+            window.jsTestIsAsync = true;
+
+        let finish = () => {
+            if (window.finishJSTest)
+                finishJSTest();
+            else
+                testRunner.notifyDone();
+        }
+
+        return promise.then(finish, finish);
     }
 }
index b90066d..fc5d40b 100644 (file)
@@ -1,3 +1,18 @@
+2016-12-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        iOS: An element with tabindex is not focusable unless there is no mouse event handler
+        https://bugs.webkit.org/show_bug.cgi?id=165843
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by ancestorRespondingToClickEvents not checking the precense of tabindex attribute.
+        Check that condition along with event listeners.
+
+        Test: fast/events/focusing-element-with-tabindex-by-tap-or-click.html
+
+        * page/ios/FrameIOS.mm:
+        (WebCore::ancestorRespondingToClickEvents):
+
 2016-12-14  Alex Christensen  <achristensen@webkit.org>
 
         Progress towards using ANGLE to do WebGL rendering
index a3cb955..df5e9eb 100644 (file)
@@ -260,8 +260,6 @@ static Node* ancestorRespondingToClickEvents(const HitTestResult& hitTestResult,
 
     Node* pointerCursorNode = nullptr;
     for (Node* node = hitTestResult.innerNode(); node && node != terminationNode; node = node->parentInComposedTree()) {
-        ASSERT(!node->isInShadowTree() || node->containingShadowRoot()->mode() != ShadowRootMode::UserAgent);
-
         // We only accept pointer nodes before reaching the body tag.
         if (node->hasTagName(HTMLNames::bodyTag)) {
 #if USE(UIKIT_EDITING)
@@ -284,7 +282,7 @@ static Node* ancestorRespondingToClickEvents(const HitTestResult& hitTestResult,
         else if (pointerCursorNode)
             pointerCursorStillValid = false;
 
-        if (node->willRespondToMouseClickEvents() || node->willRespondToMouseMoveEvents()) {
+        if (node->willRespondToMouseClickEvents() || node->willRespondToMouseMoveEvents() || (is<Element>(*node) && downcast<Element>(*node).isMouseFocusable())) {
             // If we're at the body or higher, use the pointer cursor node (which may be null).
             if (bodyHasBeenReached)
                 node = pointerCursorNode;
index fb5e3bd..40f4a81 100644 (file)
@@ -1,3 +1,18 @@
+2016-12-14  Ryosuke Niwa  <rniwa@webkit.org>
+
+        iOS: An element with tabindex is not focusable unless there is no mouse event handler
+        https://bugs.webkit.org/show_bug.cgi?id=165843
+
+        Reviewed by Antti Koivisto.
+
+        Add testRunner.isWebKit2 which is always true in WebKitTestRunner.
+        Without this, it's really hard to reliably differentiate DumpRenderTree and WebKitTestRunner,
+        and DumpRenderTree's runUIScript would hit an assertion :(
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+        (WTR::TestRunner::isWebKit2):
+
 2016-12-14  Brady Eidson  <beidson@apple.com>
 
         IndexedDB 2.0: Massively speedup IDBIndex.get().
index 62d80ee..b528cd5 100644 (file)
@@ -24,6 +24,8 @@
  */
 
 interface TestRunner {
+    readonly attribute boolean isWebKit2;
+
     // The basics.
     void dumpAsText(boolean dumpPixels);
     void dumpChildFramesAsText();
index 0108f02..dd2d491 100644 (file)
@@ -60,6 +60,8 @@ public:
 
     void makeWindowObject(JSContextRef, JSObjectRef windowObject, JSValueRef* exception);
 
+    bool isWebKit2() const { return true; }
+
     // The basics.
     WKURLRef testURL() const { return m_testURL.get(); }
     void setTestURL(WKURLRef url) { m_testURL = url; }