Mouse release on AutoFill button activates it; should only activate on click
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jun 2015 00:23:45 +0000 (00:23 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jun 2015 00:23:45 +0000 (00:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145774
<rdar://problem/21069245>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Fixes an issue where a click event was dispatched to a shadow tree node regardless of whether
both the mouse press and mouse release were targeted at it. In particular, releasing the mouse
on the AutoFill button activates it regardless of whether the mouse was pressed on it.

Currently we always dispatch a click event to a node n where the mouse was released when n is
in a shadow tree regardless of whether the mouse was pressed on n. Instead we should only
dispatch a click event to n if the mouse was pressed and released on n. If n is a shadow tree
descendant, the mouse was released on n, and n never received a mouse press then we should
dispatch the click event at the shadow host element of n to preserve the illusion to web
developers that the shadow host element is a single element.

Test: fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html

* page/EventHandler.cpp:
(WebCore::targetNodeForClickEvent): Added; returns the target node for the DOM click event.
(WebCore::EventHandler::handleMouseReleaseEvent): Modified to use dispatch the DOM click event
at the node returned by targetNodeForClickEvent().
(WebCore::mouseIsReleasedOnPressedElement): Deleted.

LayoutTests:

Add test to ensure we only dispatch a click event at the HTML input element when pressing
on the editable portion of the input element and releasing the mouse on the AutoFill button.

* fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button-expected.txt: Added.
* fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp

index b13fdc3..6ab260e 100644 (file)
@@ -1,3 +1,17 @@
+2015-06-08  Daniel Bates  <dabates@apple.com>
+
+        Mouse release on AutoFill button activates it; should only activate on click
+        https://bugs.webkit.org/show_bug.cgi?id=145774
+        <rdar://problem/21069245>
+
+        Reviewed by Ryosuke Niwa.
+
+        Add test to ensure we only dispatch a click event at the HTML input element when pressing
+        on the editable portion of the input element and releasing the mouse on the AutoFill button.
+
+        * fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button-expected.txt: Added.
+        * fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html: Added.
+
 2015-06-08  Brady Eidson  <beidson@apple.com>
 
         Fix up the layouttest situation after r185322.
diff --git a/LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button-expected.txt b/LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button-expected.txt
new file mode 100644 (file)
index 0000000..1dc1dbd
--- /dev/null
@@ -0,0 +1,12 @@
+This test checks that pressing the mouse in an password field and releasing the mouse on the AutoFill button dispatches a DOM click event at the password field and not at the AutoFill button. This test can only be run in the test tool.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+PASS event.type is "click"
+PASS event.target is document.getElementById("password")
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html b/LayoutTests/fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html
new file mode 100644 (file)
index 0000000..e70eb55
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../resources/js-test-pre.js"></script>
+<script src="../resources/common.js"></script>
+<script>
+window.jsTestIsAsync = true;
+
+var password;
+var event;
+
+window.onload = function ()
+{
+    password = document.getElementById("password");
+    password.onclick = checkEventAndDone;
+
+    runTest();
+};
+
+function runTest()
+{
+    if (!window.internals || !window.eventSender)
+        return;
+
+    internals.setShowAutoFillButton(password, true);
+
+    var autoFillButton = getElementByPseudoId(internals.shadowRoot(password), "-webkit-auto-fill-button");
+    autoFillButton.onclick = checkEventAndDone;
+
+    eventSender.mouseMoveTo(password.offsetLeft + 10, password.offsetTop + password.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.leapForward(100);
+    eventSender.mouseMoveTo(autoFillButton.offsetLeft + autoFillButton.offsetWidth / 2, autoFillButton.offsetTop + autoFillButton.offsetHeight / 2);
+    eventSender.mouseUp();
+}
+
+function checkEventAndDone(e)
+{
+    event = e;
+    shouldBeEqualToString("event.type", "click");
+    shouldBe("event.target", 'document.getElementById("password")');
+    finishJSTest();
+}
+</script>
+</head>
+<body>
+<p id="description"></p>
+<input type="password" id="password">
+<div id="console"></div>
+<script>
+description("This test checks that pressing the mouse in an password field and releasing the mouse on the AutoFill " +
+            "button dispatches a DOM click event at the password field and not at the AutoFill button. This test " +
+            "can only be run in the test tool.");
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 696ce83..83fcc20 100644 (file)
@@ -1,3 +1,30 @@
+2015-06-08  Daniel Bates  <dabates@apple.com>
+
+        Mouse release on AutoFill button activates it; should only activate on click
+        https://bugs.webkit.org/show_bug.cgi?id=145774
+        <rdar://problem/21069245>
+
+        Reviewed by Ryosuke Niwa.
+
+        Fixes an issue where a click event was dispatched to a shadow tree node regardless of whether
+        both the mouse press and mouse release were targeted at it. In particular, releasing the mouse
+        on the AutoFill button activates it regardless of whether the mouse was pressed on it.
+
+        Currently we always dispatch a click event to a node n where the mouse was released when n is
+        in a shadow tree regardless of whether the mouse was pressed on n. Instead we should only
+        dispatch a click event to n if the mouse was pressed and released on n. If n is a shadow tree
+        descendant, the mouse was released on n, and n never received a mouse press then we should
+        dispatch the click event at the shadow host element of n to preserve the illusion to web
+        developers that the shadow host element is a single element.
+
+        Test: fast/forms/auto-fill-button/mouse-down-input-mouse-release-auto-fill-button.html
+
+        * page/EventHandler.cpp:
+        (WebCore::targetNodeForClickEvent): Added; returns the target node for the DOM click event.
+        (WebCore::EventHandler::handleMouseReleaseEvent): Modified to use dispatch the DOM click event
+        at the node returned by targetNodeForClickEvent().
+        (WebCore::mouseIsReleasedOnPressedElement): Deleted.
+
 2015-06-08  Chris Dumez  <cdumez@apple.com>
 
         WebContent crash in WebCore::Page::sessionID() const + 0 (Page.cpp:1660)
index ade324b..216abc7 100644 (file)
@@ -2037,28 +2037,22 @@ void EventHandler::invalidateClick()
     m_clickNode = nullptr;
 }
 
-inline static bool mouseIsReleasedOnPressedElement(Node* targetNode, Node* clickNode)
+static Node* targetNodeForClickEvent(Node* mousePressNode, Node* mouseReleaseNode)
 {
-    if (targetNode == clickNode)
-        return true;
-
-    if (!targetNode)
-        return false;
-
-    ShadowRoot* containingShadowRoot = targetNode->containingShadowRoot();
-    if (!containingShadowRoot)
-        return false;
+    if (!mousePressNode || !mouseReleaseNode)
+        return nullptr;
 
-    // FIXME: When an element in UA ShadowDOM (e.g. inner element in <input>) is clicked,
-    // we assume that the host element is clicked. This is necessary for implementing <input type="range"> etc.
-    // However, we should not check ShadowRoot type basically.
-    // https://bugs.webkit.org/show_bug.cgi?id=108047
-    if (containingShadowRoot->type() != ShadowRoot::UserAgentShadowRoot)
-        return false;
+    if (mousePressNode == mouseReleaseNode)
+        return mouseReleaseNode;
 
-    Node* adjustedTargetNode = targetNode->shadowHost();
-    Node* adjustedClickNode = clickNode ? clickNode->shadowHost() : 0;
-    return adjustedTargetNode == adjustedClickNode;
+    Element* mouseReleaseShadowHost = mouseReleaseNode->shadowHost();
+    if (mouseReleaseShadowHost && mouseReleaseShadowHost == mousePressNode->shadowHost()) {
+        // We want to dispatch the click to the shadow tree host element to give listeners the illusion that the
+        // shadom tree is a single element. For example, we want to give the illusion that <input type="range">
+        // is a single element even though it is a composition of multiple shadom tree elements.
+        return mouseReleaseShadowHost;
+    }
+    return nullptr;
 }
 
 bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& platformMouseEvent)
@@ -2122,7 +2116,8 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& platformMou
 
     bool contextMenuEvent = platformMouseEvent.button() == RightButton;
 
-    bool swallowClickEvent = m_clickCount > 0 && !contextMenuEvent && mouseIsReleasedOnPressedElement(mouseEvent.targetNode(), m_clickNode.get()) && !dispatchMouseEvent(eventNames().clickEvent, mouseEvent.targetNode(), true, m_clickCount, platformMouseEvent, true);
+    Node* nodeToClick = targetNodeForClickEvent(m_clickNode.get(), mouseEvent.targetNode());
+    bool swallowClickEvent = m_clickCount > 0 && !contextMenuEvent && nodeToClick && !dispatchMouseEvent(eventNames().clickEvent, nodeToClick, true, m_clickCount, platformMouseEvent, true);
 
     if (m_resizeLayer) {
         m_resizeLayer->setInResizeMode(false);