Changeset 170808 in webkit


Ignore:
Timestamp:
Jul 4, 2014, 12:12:25 PM (10 years ago)
Author:
commit-queue@webkit.org
Message:

input type=range element should only fire change events after committing 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:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r170806 r170808  
     12014-07-04  Julien Quint  <pom@graougraou.com>
     2
     3        input type=range element should only fire change events after committing a  value
     4        https://bugs.webkit.org/show_bug.cgi?id=134545
     5
     6        Reviewed by Dean Jackson.
     7
     8        Two existing tests are updated to count "input" events as well as
     9        "change" events. The tests now verify that "change" is only fired once
     10        after every slider drag completes, whereas "input" may be fired more
     11        than once.
     12
     13        * fast/forms/range/range-drag-expected.txt:
     14        * fast/forms/range/range-drag-when-toggled-disabled-expected.txt:
     15        * fast/forms/range/range-drag-when-toggled-disabled.html:
     16        * fast/forms/range/range-drag.html:
     17
    1182014-07-04  Mario Sanchez Prada  <mario.prada@samsung.com>
    219

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

    r93593 r170808  
    44readOnly=false, disabled=false
    55PASS input.value is "100"
    6 PASS changeEventCounter is >= lastChangeEventCounter + 1
     6PASS inputEventCounter is >= lastInputEventCounter + 1
     7PASS changeEventCounter is lastChangeEventCounter + 1
    78readOnly=true
    89PASS input.value is "50"
     10PASS lastInputEventCounter is inputEventCounter
    911PASS lastChangeEventCounter is changeEventCounter
    1012disabled=true
    1113PASS input.value is "50"
     14PASS lastInputEventCounter is inputEventCounter
    1215PASS lastChangeEventCounter is changeEventCounter
    1316
    … …  
    1518readOnly=false, disabled=false
    1619PASS input.value is "100"
    17 PASS changeEventCounter is >= lastChangeEventCounter + 1
     20PASS inputEventCounter is >= lastInputEventCounter + 1
     21PASS changeEventCounter is lastChangeEventCounter + 1
    1822readOnly=true
    1923PASS input.value is "50"
     24PASS lastInputEventCounter is inputEventCounter
    2025PASS lastChangeEventCounter is changeEventCounter
    2126disabled=true
    2227PASS input.value is "50"
     28PASS lastInputEventCounter is inputEventCounter
    2329PASS lastChangeEventCounter is changeEventCounter
    2430

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

    r112547 r170808  
    44readOnly=false, disabled=false
    55PASS input.value is "100"
    6 PASS changeEventCounter is >= lastChangeEventCounter + 1
     6PASS inputEventCounter is >= lastInputEventCounter + 1
    77PASS input.value is "0"
    8 PASS changeEventCounter is >= lastChangeEventCounter + 1
     8PASS inputEventCounter is >= lastInputEventCounter + 1
    99readOnly=true
    1010PASS input.value is "0"
    11 PASS lastChangeEventCounter is changeEventCounter
     11PASS lastInputEventCounter is inputEventCounter
    1212
    1313Tests for range dragging while it toggles to be disabled.
    1414readOnly=false, disabled=false
    1515PASS input.value is "100"
    16 PASS changeEventCounter is >= lastChangeEventCounter + 1
     16PASS inputEventCounter is >= lastInputEventCounter + 1
    1717PASS input.value is "0"
    18 PASS changeEventCounter is >= lastChangeEventCounter + 1
     18PASS inputEventCounter is >= lastInputEventCounter + 1
    1919disabled=true
    2020PASS input.value is "0"
    21 PASS lastChangeEventCounter is changeEventCounter
     21PASS lastInputEventCounter is inputEventCounter
     22
     23PASS changeEventCounter is 1
    2224
    2325PASS successfullyParsed is true

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

    r155268 r170808  
    1212
    1313var changeEventCounter = 0;
    14 var lastChangeEventCounter = changeEventCounter;
    1514function handleChange() {
    1615    changeEventCounter++;
     16}
     17
     18var inputEventCounter = 0;
     19var lastInputEventCounter = inputEventCounter;
     20function handleInput() {
     21    inputEventCounter++;
    1722}
    1823
    … …  
    2530    input = document.getElementById(id);
    2631    input.onchange = handleChange;
     32    input.oninput = handleInput;
    2733    input.focus();
    2834
    … …  
    5056    debug('readOnly=false, disabled=false');
    5157    input.valueAsNumber = 50;
    52     lastChangeEventCounter = changeEventCounter;
     58    lastInputEventCounter = inputEventCounter;
    5359    dragToRightEdge();
    5460    shouldBe('input.value', '"100"');
    55     shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
    56     lastChangeEventCounter = changeEventCounter;
     61    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
     62    lastInputEventCounter = inputEventCounter;
    5763    dragToLeftEdge();
    5864    shouldBe('input.value', '"0"');
    59     shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
     65    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
    6066
    6167    // Toggle (readonly | disabled).
    … …  
    6470
    6571    // Attempt to drag to right edge. Should not change.
    66     lastChangeEventCounter = changeEventCounter;
     72    lastInputEventCounter = inputEventCounter;
    6773    dragToRightEdge();
    6874    shouldBe('input.value', '"0"');
    69     shouldBe('lastChangeEventCounter', 'changeEventCounter');
     75    shouldBe('lastInputEventCounter', 'inputEventCounter');
    7076
    7177    stopDrag();
    … …  
    8490debug('');
    8591
     92shouldBe('changeEventCounter', '1');
     93debug('');
     94
    8695</script>
    8796<script src="../../../resources/js-test-post.js"></script>

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

    r155268 r170808  
    88<div id="console"></div>
    99<script>
     10
     11var inputEventCounter = 0;
     12function handleInput() {
     13    inputEventCounter++;
     14}
     15var lastInputEventCounter = inputEventCounter;
    1016
    1117var changeEventCounter = 0;
    … …  
    4147    debug('readOnly=false, disabled=false');
    4248    input.valueAsNumber = 50;
     49    lastInputEventCounter = inputEventCounter;
    4350    lastChangeEventCounter = changeEventCounter;
    4451    dragMouse();
    4552    shouldBe('input.value', '"100"');
    46     shouldBeGreaterThanOrEqual('changeEventCounter', 'lastChangeEventCounter + 1');
     53    shouldBeGreaterThanOrEqual('inputEventCounter', 'lastInputEventCounter + 1');
     54    shouldBe('changeEventCounter', 'lastChangeEventCounter + 1');
    4755
    4856    debug('readOnly=true');
    … …  
    5058    input.readOnly = true;
    5159    input.valueAsNumber = 50;
     60    lastInputEventCounter = inputEventCounter;
    5261    lastChangeEventCounter = changeEventCounter;
    5362    dragMouse();
    5463    shouldBe('input.value', '"50"');
     64    shouldBe('lastInputEventCounter', 'inputEventCounter');
    5565    shouldBe('lastChangeEventCounter', 'changeEventCounter');
    5666
    … …  
    5969    input.disabled = true;
    6070    input.valueAsNumber = 50;
     71    lastInputEventCounter = inputEventCounter;
    6172    lastChangeEventCounter = changeEventCounter;
    6273    dragMouse();
    6374    shouldBe('input.value', '"50"');
     75    shouldBe('lastInputEventCounter', 'inputEventCounter');
    6476    shouldBe('lastChangeEventCounter', 'changeEventCounter');
    6577}
    … …  
    7284debug('Tests for range dragging from center.');
    7385var input = document.getElementById('range1');
     86input.oninput = handleInput;
    7487input.onchange = handleChange;
    7588input.focus();
    … …  
    7992debug('Tests for range dragging from edge');
    8093var input = document.getElementById('range2');
     94input.oninput = handleInput;
    8195input.onchange = handleChange;
    8296input.focus();

  • trunk/Source/WebCore/ChangeLog

    r170807 r170808  
     12014-07-04  Julien Quint  <pom@graougraou.com>
     2
     3        input type=range element should only fire change events after committing a  value
     4        https://bugs.webkit.org/show_bug.cgi?id=134545
     5
     6        Reviewed by Dean Jackson.
     7
     8        A "change" event was fired every time the slider thumb element was dragged
     9        by the user. The "change" event is now fired only after the thumb
     10        element has stopped moving; previously, both "input" and "change" events
     11        where dispatched while changes were being made. This new behavior is
     12        consistent with the specification (cf.
     13        http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#event-input-change),
     14        as well as other implementations such as Firefox and Chrome.
     15
     16        * Modules/mediacontrols/mediaControlsApple.js:
     17        (Controller.prototype.createControls): Listen to the "input" event
     18        rather than the "change" event for the timeline control in order to
     19        keep track of value changes when the user is dragging the thumb.
     20        * accessibility/AccessibilitySlider.cpp:
     21        (WebCore::AccessibilitySlider::setValue): Dispatch "change" event while
     22        setting the new value rather than dispatching later, since setting the
     23        value now clears the change flag.
     24        * html/RangeInputType.cpp:
     25        (WebCore::RangeInputType::setValue): Update the text value of the
     26        control in the case when no event is to be dispatched, so that this
     27        value can be checked the next time a "change" event dispatch is
     28        requested.
     29        * html/shadow/SliderThumbElement.cpp:
     30        (WebCore::SliderThumbElement::setPositionFromPoint): Removed the
     31        dispatch of the "change" event, and no longer track the text value of
     32        the element as a result of dispatching a "change" event.
     33        (WebCore::SliderThumbElement::stopDragging): Dispatch the "change" event
     34        on completing the drag.
     35
    1362014-07-04  Andreas Kling  <akling@apple.com>
    237

  • trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js

    r170315 r170808  
    331331        timeline.style.backgroundImage = '-webkit-canvas(timeline-' + this.timelineID + ')';
    332332        timeline.type = 'range';
    333         this.listenFor(timeline, 'change', this.handleTimelineChange);
     333        this.listenFor(timeline, 'input', this.handleTimelineChange);
    334334        this.listenFor(timeline, 'mouseover', this.handleTimelineMouseOver);
    335335        this.listenFor(timeline, 'mouseout', this.handleTimelineMouseOut);

  • trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp

    r165676 r170808  
    134134        return;
    135135
    136     input->setValue(value);
    137 
    138     // Fire change event manually, as RenderSlider::setValueForPosition does.
    139     input->dispatchFormControlChangeEvent();
     136    input->setValue(value, DispatchChangeEvent);
    140137}
    141138

  • trunk/Source/WebCore/html/RangeInputType.cpp

    r170774 r170808  
    328328        return;
    329329
     330    if (eventBehavior == DispatchNoEvent)
     331        element().setTextAsOfLastFormControlChangeEvent(value);
     332
    330333    typedSliderThumbElement().setPositionFromValue();
    331334}

  • trunk/Source/WebCore/html/shadow/SliderThumbElement.cpp

    r170774 r170808  
    262262        return;
    263263
    264     input->setTextAsOfLastFormControlChangeEvent(input->value());
    265 
    266264    // Do all the tracking math relative to the input's renderer's box.
    267265    RenderBox& inputRenderer = *toRenderBox(input->renderer());
    … …  
    313311    if (renderer())
    314312        renderer()->setNeedsLayout();
    315     input->dispatchFormControlChangeEvent();
    316313}
    317314
    … …  
    334331    if (renderer())
    335332        renderer()->setNeedsLayout();
     333
     334    RefPtr<HTMLInputElement> input = hostInput();
     335    if (input)
     336        input->dispatchFormControlChangeEvent();
    336337}
    337338
Note: See TracChangeset for help on using the changeset viewer.