Input type=range issue with events not being raised when value set in js
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 15:51:28 +0000 (15:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Jul 2012 15:51:28 +0000 (15:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=84674

Source/WebCore:

Fix dispatching of change and click events for the input slider.
Prior to the fix, change events were not fired if the new value
matched the value at last change notification based on expected
behavior for text fields.  Clicks were not fired if the thumb
element was repositioned under the cursor on mouse down.

Patch by Kevin Ellis <kevers@chromium.org> on 2012-07-10
Reviewed by Kent Tamura.

Tests: fast/events/click-range-slider.html
       fast/events/onchange-range-slider.html

* html/shadow/SliderThumbElement.cpp:
(WebCore::SliderThumbElement::setPositionFromPoint):
* page/EventHandler.cpp:
(WebCore::EventHandler::handleMouseReleaseEvent):

LayoutTests:

Ensure that click and change events are properly triggered for range
sliders.  Prior to the fix, change events were not fired if the new
value matched the value at last change notification based on expected
behavior for text fields.  Clicks were not fired if the thumb element
was positioned under the cursor on mouse down.

Patch by Kevin Ellis <kevers@chromium.org> on 2012-07-10
Reviewed by Kent Tamura.

* fast/events/click-range-slider-expected.txt: Added.
* fast/events/click-range-slider.html: Added.
* fast/events/onchange-range-slider-expected.txt: Added.
* fast/events/onchange-range-slider.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/click-range-slider-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/click-range-slider.html [new file with mode: 0644]
LayoutTests/fast/events/onchange-range-slider-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/onchange-range-slider.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/shadow/SliderThumbElement.cpp
Source/WebCore/page/EventHandler.cpp

index e867920..fdffecb 100644 (file)
@@ -1,3 +1,21 @@
+2012-07-10  Kevin Ellis  <kevers@chromium.org>
+
+        Input type=range issue with events not being raised when value set in js
+        https://bugs.webkit.org/show_bug.cgi?id=84674
+
+        Ensure that click and change events are properly triggered for range 
+        sliders.  Prior to the fix, change events were not fired if the new
+        value matched the value at last change notification based on expected
+        behavior for text fields.  Clicks were not fired if the thumb element
+        was positioned under the cursor on mouse down.
+
+        Reviewed by Kent Tamura.
+
+        * fast/events/click-range-slider-expected.txt: Added.
+        * fast/events/click-range-slider.html: Added.
+        * fast/events/onchange-range-slider-expected.txt: Added.
+        * fast/events/onchange-range-slider.html: Added.
+
 2012-07-10  Zan Dobersek  <zandobersek@gmail.com>
 
         Unreviewed GTK gardening, adding a text expectation for the
diff --git a/LayoutTests/fast/events/click-range-slider-expected.txt b/LayoutTests/fast/events/click-range-slider-expected.txt
new file mode 100644 (file)
index 0000000..8858df1
--- /dev/null
@@ -0,0 +1,5 @@
+Test that click events are fired for a slider when the range of values is dense enough that the thumb element is repositioned under the cursor on mouse press.
+
+
+PASS clickCount is 3
+
diff --git a/LayoutTests/fast/events/click-range-slider.html b/LayoutTests/fast/events/click-range-slider.html
new file mode 100644 (file)
index 0000000..f95c212
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>Test that click events are fired for a slider when the range of values is dense enough that the thumb element is repositioned under the cursor on mouse press.</p>
+
+<!-- See bug 84674 -->
+<input id="slider" type="range" min="0" max="100" value="50"></input>
+<pre id="console"></pre>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+var clickCount = 0;
+var slider;
+
+function onClick(e)
+{
+    clickCount++;
+}
+
+window.onload = function()
+{
+    if (!window.testRunner)
+        return;
+
+    slider = document.getElementById("slider");
+    slider.addEventListener("click", onClick);
+
+    // Click respositions the slider thumb element under the cursor.
+    // Ensure that the click event still fires.
+    var x = slider.offsetLeft + 1;
+    var y = slider.offsetTop + slider.clientHeight / 2;
+
+    eventSender.mouseMoveTo(x, y);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    eventSender.mouseMoveTo(x + slider.clientWidth - 2, y);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    eventSender.mouseMoveTo(x + slider.clientWidth / 2, y);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    shouldBe("clickCount", "3");
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/onchange-range-slider-expected.txt b/LayoutTests/fast/events/onchange-range-slider-expected.txt
new file mode 100644 (file)
index 0000000..37f171e
--- /dev/null
@@ -0,0 +1,6 @@
+This test verifies that updating the slider for an input element with type=range fires a change event.
+
+
+PASS Change event fired.
+PASS slider.value is "0"
+
diff --git a/LayoutTests/fast/events/onchange-range-slider.html b/LayoutTests/fast/events/onchange-range-slider.html
new file mode 100644 (file)
index 0000000..a2f94ee
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test verifies that updating the slider for an input element with type=range fires a change event.</p>
+
+<!-- See bug 84674 -->
+<input id="slider" type="range" min="0" max="3" value="0"></input>
+<pre id="console"></pre>
+<script src="../js/resources/js-test-pre.js"></script>
+<script>
+var receivedChangeEvent = false;
+var slider;
+
+function onChange(e)
+{
+    testPassed('Change event fired.');
+    receivedChangeEvent = true;
+    shouldBeEqualToString("slider.value", "0");
+}
+
+window.onload = function()
+{
+    if (!window.testRunner)
+        return;
+
+    slider = document.getElementById("slider");
+    slider.addEventListener("change", onChange);
+
+    // Programmatically changing an input value should not fire a change event.
+    slider.value = 1;
+
+    // Changing back to original value should fire a change event.
+    var x = slider.offsetLeft + 1;
+    var y = slider.offsetTop + slider.clientHeight / 2;
+
+    eventSender.mouseMoveTo(x, y);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    if (!receivedChangeEvent)
+        testFailed('Change event not fired.');
+}
+</script>
+</body>
+</html>
index 9466e5e..c57afb7 100644 (file)
@@ -1,3 +1,24 @@
+2012-07-10  Kevin Ellis  <kevers@chromium.org>
+
+        Input type=range issue with events not being raised when value set in js
+        https://bugs.webkit.org/show_bug.cgi?id=84674
+
+        Fix dispatching of change and click events for the input slider.
+        Prior to the fix, change events were not fired if the new value
+        matched the value at last change notification based on expected
+        behavior for text fields.  Clicks were not fired if the thumb
+        element was repositioned under the cursor on mouse down.
+
+        Reviewed by Kent Tamura.
+
+        Tests: fast/events/click-range-slider.html
+               fast/events/onchange-range-slider.html
+
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::SliderThumbElement::setPositionFromPoint):
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleMouseReleaseEvent):
+
 2012-07-10  Huang Dongsung  <luxtella@company100.net>
 
         Don't destroy the decoded data of an image if WebKit is about to render the image.
index 502dae1..0186fa5 100644 (file)
@@ -218,6 +218,7 @@ void SliderThumbElement::setPositionFromPoint(const LayoutPoint& point)
     if (!input->renderer() || !renderer())
         return;
 
+    input->setTextAsOfLastFormControlChangeEvent(input->value());
     LayoutPoint offset = roundedLayoutPoint(input->renderer()->absoluteToLocal(point, false, true));
     bool isVertical = hasVerticalAppearance(input);
     LayoutUnit trackSize;
index 49b07df..a0c9afc 100644 (file)
@@ -1881,7 +1881,12 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
 
     bool swallowMouseUpEvent = dispatchMouseEvent(eventNames().mouseupEvent, targetNode(mev), true, m_clickCount, mouseEvent, false);
 
-    bool swallowClickEvent = m_clickCount > 0 && mouseEvent.button() != RightButton && targetNode(mev) == m_clickNode && dispatchMouseEvent(eventNames().clickEvent, targetNode(mev), true, m_clickCount, mouseEvent, true);
+    Node* clickTarget = targetNode(mev);
+    if (clickTarget)
+        clickTarget = clickTarget->shadowAncestorNode();
+    Node* adjustedClickNode = m_clickNode ? m_clickNode->shadowAncestorNode() : 0;
+
+    bool swallowClickEvent = m_clickCount > 0 && mouseEvent.button() != RightButton && clickTarget == adjustedClickNode && dispatchMouseEvent(eventNames().clickEvent, targetNode(mev), true, m_clickCount, mouseEvent, true);
 
     if (m_resizeLayer) {
         m_resizeLayer->setInResizeMode(false);