input type=range element should only fire change events after committ…

…ing a value

https://bugs.webkit.org/show_bug.cgi?id=134545

Patch by Julien Quint <pom@graougraou.com> on 2014-07-04
Reviewed by Dean Jackson.

Source/WebCore:
A "change" event was fired every time the slider thumb element was dragged
by the user. The "change" event is now fired only after the thumb
element has stopped moving; previously, both "input" and "change" events
where dispatched while changes were being made. This new behavior is
consistent with the specification (cf.
http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#event-input-change),
as well as other implementations such as Firefox and Chrome.

* Modules/mediacontrols/mediaControlsApple.js:
(Controller.prototype.createControls): Listen to the "input" event
rather than the "change" event for the timeline control in order to
keep track of value changes when the user is dragging the thumb.
* accessibility/AccessibilitySlider.cpp:
(WebCore::AccessibilitySlider::setValue): Dispatch "change" event while
setting the new value rather than dispatching later, since setting the
value now clears the change flag.
* html/RangeInputType.cpp:
(WebCore::RangeInputType::setValue): Update the text value of the
control in the case when no event is to be dispatched, so that this
value can be checked the next time a "change" event dispatch is
requested.
* html/shadow/SliderThumbElement.cpp:
(WebCore::SliderThumbElement::setPositionFromPoint): Removed the
dispatch of the "change" event, and no longer track the text value of
the element as a result of dispatching a "change" event.
(WebCore::SliderThumbElement::stopDragging): Dispatch the "change" event
on completing the drag.

LayoutTests:
Two existing tests are updated to count "input" events as well as
"change" events. The tests now verify that "change" is only fired once
after every slider drag completes, whereas "input" may be fired more
than once.

* fast/forms/range/range-drag-expected.txt:
* fast/forms/range/range-drag-when-toggled-disabled-expected.txt:
* fast/forms/range/range-drag-when-toggled-disabled.html:
* fast/forms/range/range-drag.html:

Canonical link: https://commits.webkit.org/152612@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@170808 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,20 @@
+2014-07-04  Julien Quint  <pom@graougraou.com>
+
+        input type=range element should only fire change events after committing a  value
+        https://bugs.webkit.org/show_bug.cgi?id=134545
+
+        Reviewed by Dean Jackson.
+
+        Two existing tests are updated to count "input" events as well as
+        "change" events. The tests now verify that "change" is only fired once
+        after every slider drag completes, whereas "input" may be fired more
+        than once.
+
+        * fast/forms/range/range-drag-expected.txt:
+        * fast/forms/range/range-drag-when-toggled-disabled-expected.txt:
+        * fast/forms/range/range-drag-when-toggled-disabled.html:
+        * fast/forms/range/range-drag.html:
+
 2014-07-04  Mario Sanchez Prada  <mario.prada@samsung.com>
 
         buildAccessibilityTree() needs to be removed from individual layout tests

LayoutTests/fast/forms/range/range-drag-expected.txt

@@ -3,23 +3,29 @@ Test for dragging operations of <input type=range>
 Tests for range dragging from center.
 readOnly=false, disabled=false
 PASS input.value is "100"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
+PASS changeEventCounter is lastChangeEventCounter + 1
 readOnly=true
 PASS input.value is "50"
+PASS lastInputEventCounter is inputEventCounter
 PASS lastChangeEventCounter is changeEventCounter
 disabled=true
 PASS input.value is "50"
+PASS lastInputEventCounter is inputEventCounter
 PASS lastChangeEventCounter is changeEventCounter
 
 Tests for range dragging from edge
 readOnly=false, disabled=false
 PASS input.value is "100"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
+PASS changeEventCounter is lastChangeEventCounter + 1
 readOnly=true
 PASS input.value is "50"
+PASS lastInputEventCounter is inputEventCounter
 PASS lastChangeEventCounter is changeEventCounter
 disabled=true
 PASS input.value is "50"
+PASS lastInputEventCounter is inputEventCounter
 PASS lastChangeEventCounter is changeEventCounter
 
 PASS successfullyParsed is true

LayoutTests/fast/forms/range/range-drag-when-toggled-disabled-expected.txt

@@ -3,22 +3,24 @@ Test for dragging operations of <input type=range> when readonly or disabled are
 Tests for range dragging while it toggles to be readonly.
 readOnly=false, disabled=false
 PASS input.value is "100"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
 PASS input.value is "0"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
 readOnly=true
 PASS input.value is "0"
-PASS lastChangeEventCounter is changeEventCounter
+PASS lastInputEventCounter is inputEventCounter
 
 Tests for range dragging while it toggles to be disabled.
 readOnly=false, disabled=false
 PASS input.value is "100"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
 PASS input.value is "0"
-PASS changeEventCounter is >= lastChangeEventCounter + 1
+PASS inputEventCounter is >= lastInputEventCounter + 1
 disabled=true
 PASS input.value is "0"
-PASS lastChangeEventCounter is changeEventCounter
+PASS lastInputEventCounter is inputEventCounter
+
+PASS changeEventCounter is 1
 
 PASS successfullyParsed is true
 

LayoutTests/fast/forms/range/range-drag-when-toggled-disabled.html

@@ -11,11 +11,16 @@
 <script>
 
 var changeEventCounter = 0;
-var lastChangeEventCounter = changeEventCounter;
 function handleChange() {
     changeEventCounter++;
 }
 
+var inputEventCounter = 0;
+var lastInputEventCounter = inputEventCounter;
+function handleInput() {
+    inputEventCounter++;
+}
+
 var input = null;
 
 function testInput(id, field) {
@@ -24,6 +29,7 @@
 
     input = document.getElementById(id);
     input.onchange = handleChange;
+    input.oninput = handleInput;
     input.focus();
 
     var centerY = input.offsetTop + input.offsetHeight / 2;
@@ -49,24 +55,24 @@
     // Drag from center, to right edge, to left edge.
     debug('readOnly=false, disabled=false');
     input.valueAsNumber = 50;
-    lastChangeEventCounter = changeEventCounter;
+    lastInputEventCounter = inputEventCounter;
     dragToRightEdge();
     shouldBe('input.value', '"100"');
-    shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
-    lastChangeEventCounter = changeEventCounter;
+    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
+    lastInputEventCounter = inputEventCounter;
     dragToLeftEdge();
     shouldBe('input.value', '"0"');
-    shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
+    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
 
     // Toggle (readonly | disabled).
     debug(field + '=true');
     input[field] = true;
 
     // Attempt to drag to right edge. Should not change.
-    lastChangeEventCounter = changeEventCounter;
+    lastInputEventCounter = inputEventCounter;
     dragToRightEdge();
     shouldBe('input.value', '"0"');
-    shouldBe('lastChangeEventCounter', 'changeEventCounter');
+    shouldBe('lastInputEventCounter', 'inputEventCounter');
 
     stopDrag();
 }
@@ -83,6 +89,9 @@
 testInput('range2', 'disabled');
 debug('');
 
+shouldBe('changeEventCounter', '1');
+debug('');
+
 </script>
 <script src="../../../resources/js-test-post.js"></script>
 </body>

LayoutTests/fast/forms/range/range-drag.html

@@ -8,6 +8,12 @@
 <div id="console"></div>
 <script>
 
+var inputEventCounter = 0;
+function handleInput() {
+    inputEventCounter++;
+}
+var lastInputEventCounter = inputEventCounter;
+
 var changeEventCounter = 0;
 function handleChange() {
     changeEventCounter++;
@@ -40,27 +46,33 @@
 
     debug('readOnly=false, disabled=false');
     input.valueAsNumber = 50;
+    lastInputEventCounter = inputEventCounter;
     lastChangeEventCounter = changeEventCounter;
     dragMouse();
     shouldBe('input.value', '"100"');
-    shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
+    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
+    shouldBe('changeEventCounter', 'lastChangeEventCounter + 1');
 
     debug('readOnly=true');
     input.disabled = false;
     input.readOnly = true;
     input.valueAsNumber = 50;
+    lastInputEventCounter = inputEventCounter;
     lastChangeEventCounter = changeEventCounter;
     dragMouse();
     shouldBe('input.value', '"50"');
+    shouldBe('lastInputEventCounter', 'inputEventCounter');
     shouldBe('lastChangeEventCounter', 'changeEventCounter');
 
     debug('disabled=true');
     input.readOnly = false;
     input.disabled = true;
     input.valueAsNumber = 50;
+    lastInputEventCounter = inputEventCounter;
     lastChangeEventCounter = changeEventCounter;
     dragMouse();
     shouldBe('input.value', '"50"');
+    shouldBe('lastInputEventCounter', 'inputEventCounter');
     shouldBe('lastChangeEventCounter', 'changeEventCounter');
 }
 
@@ -71,13 +83,15 @@
 
 debug('Tests for range dragging from center.');
 var input = document.getElementById('range1');
+input.oninput = handleInput;
 input.onchange = handleChange;
 input.focus();
 testInput(false);
 debug('');
 
 debug('Tests for range dragging from edge');
 var input = document.getElementById('range2');
+input.oninput = handleInput;
 input.onchange = handleChange;
 input.focus();
 testInput(true);

Source/WebCore/ChangeLog

@@ -1,3 +1,38 @@
+2014-07-04  Julien Quint  <pom@graougraou.com>
+
+        input type=range element should only fire change events after committing a  value
+        https://bugs.webkit.org/show_bug.cgi?id=134545
+
+        Reviewed by Dean Jackson.
+
+        A "change" event was fired every time the slider thumb element was dragged
+        by the user. The "change" event is now fired only after the thumb
+        element has stopped moving; previously, both "input" and "change" events
+        where dispatched while changes were being made. This new behavior is
+        consistent with the specification (cf.
+        http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#event-input-change),
+        as well as other implementations such as Firefox and Chrome.
+
+        * Modules/mediacontrols/mediaControlsApple.js:
+        (Controller.prototype.createControls): Listen to the "input" event
+        rather than the "change" event for the timeline control in order to
+        keep track of value changes when the user is dragging the thumb.
+        * accessibility/AccessibilitySlider.cpp:
+        (WebCore::AccessibilitySlider::setValue): Dispatch "change" event while
+        setting the new value rather than dispatching later, since setting the
+        value now clears the change flag.
+        * html/RangeInputType.cpp:
+        (WebCore::RangeInputType::setValue): Update the text value of the
+        control in the case when no event is to be dispatched, so that this
+        value can be checked the next time a "change" event dispatch is
+        requested.
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::SliderThumbElement::setPositionFromPoint): Removed the
+        dispatch of the "change" event, and no longer track the text value of
+        the element as a result of dispatching a "change" event.
+        (WebCore::SliderThumbElement::stopDragging): Dispatch the "change" event
+        on completing the drag.
+
 2014-07-04  Andreas Kling  <akling@apple.com>
 
         CTTE: ApplicationCacheHost always has a DocumentLoader.

Source/WebCore/Modules/mediacontrols/mediaControlsApple.js

@@ -330,7 +330,7 @@ Controller.prototype = {
         timeline.setAttribute('aria-label', this.UIString('Duration'));
         timeline.style.backgroundImage = '-webkit-canvas(timeline-' + this.timelineID + ')';
         timeline.type = 'range';
-        this.listenFor(timeline, 'change', this.handleTimelineChange);
+        this.listenFor(timeline, 'input', this.handleTimelineChange);
         this.listenFor(timeline, 'mouseover', this.handleTimelineMouseOver);
         this.listenFor(timeline, 'mouseout', this.handleTimelineMouseOut);
         this.listenFor(timeline, 'mousemove', this.handleTimelineMouseMove);

Source/WebCore/accessibility/AccessibilitySlider.cpp

@@ -133,10 +133,7 @@ void AccessibilitySlider::setValue(const String& value)
     if (input->value() == value)
         return;
 
-    input->setValue(value);
-
-    // Fire change event manually, as RenderSlider::setValueForPosition does.
-    input->dispatchFormControlChangeEvent();
+    input->setValue(value, DispatchChangeEvent);
 }
 
 HTMLInputElement* AccessibilitySlider::inputElement() const

Source/WebCore/html/RangeInputType.cpp

@@ -327,6 +327,9 @@ void RangeInputType::setValue(const String& value, bool valueChanged, TextFieldE
     if (!valueChanged)
         return;
 
+    if (eventBehavior == DispatchNoEvent)
+        element().setTextAsOfLastFormControlChangeEvent(value);
+
     typedSliderThumbElement().setPositionFromValue();
 }
 

Source/WebCore/html/shadow/SliderThumbElement.cpp

@@ -261,8 +261,6 @@ void SliderThumbElement::setPositionFromPoint(const LayoutPoint& absolutePoint)
     if (!trackElement->renderBox())
         return;
 
-    input->setTextAsOfLastFormControlChangeEvent(input->value());
-
     // Do all the tracking math relative to the input's renderer's box.
     RenderBox& inputRenderer = *toRenderBox(input->renderer());
     RenderBox& trackRenderer = *trackElement->renderBox();
@@ -312,7 +310,6 @@ void SliderThumbElement::setPositionFromPoint(const LayoutPoint& absolutePoint)
     input->setValueFromRenderer(valueString);
     if (renderer())
         renderer()->setNeedsLayout();
-    input->dispatchFormControlChangeEvent();
 }
 
 void SliderThumbElement::startDragging()
@@ -333,6 +330,10 @@ void SliderThumbElement::stopDragging()
     m_inDragMode = false;
     if (renderer())
         renderer()->setNeedsLayout();
+
+    RefPtr<HTMLInputElement> input = hostInput();
+    if (input)
+        input->dispatchFormControlChangeEvent();
 }
 
 #if !PLATFORM(IOS)