Changeset 236779 in webkit


Ignore:
Timestamp:
Oct 2, 2018 5:28:21 PM (6 years ago)
Author:
Chris Dumez
Message:

radio / checkbox inputs should fire "click, input, change" events in order when clicked
​https://bugs.webkit.org/show_bug.cgi?id=190223

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline a few WPT tests that are now passing. I have verified that those are passing in Gecko and Blink
as well.

  • web-platform-tests/html/semantics/forms/the-input-element/checkbox-click-events-expected.txt:
  • web-platform-tests/html/semantics/forms/the-input-element/checkbox-expected.txt:
  • web-platform-tests/html/semantics/forms/the-input-element/radio-expected.txt:

Source/WebCore:

radio / checkbox inputs should fire "click, input, change" events in order when clicked:

Gecko and Blink already behave this way. However, WebKit has the following issues:

  • the input event is not fired
  • the click event is fired after the change event

No new tests, updated / rebaselined existing tests.

  • html/BaseCheckableInputType.cpp:

(WebCore::BaseCheckableInputType::fireInputAndChangeEvents):

  • html/BaseCheckableInputType.h:
  • html/CheckboxInputType.cpp:

(WebCore::CheckboxInputType::willDispatchClick):
(WebCore::CheckboxInputType::didDispatchClick):

  • html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::setChecked):

  • html/HTMLInputElement.h:
  • html/RadioInputType.cpp:

(WebCore::RadioInputType::willDispatchClick):
(WebCore::RadioInputType::didDispatchClick):

LayoutTests:

Update existing test to reflect behavior change. I have verified that our new behavior
on this test is consistent with Gecko and Chrome.

  • fast/forms/radio/radio-group-keyboard-change-event-expected.txt:
  • fast/forms/radio/radio-group-keyboard-change-event.html:
Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r236778 r236779  
     12018-10-02  Chris Dumez  <cdumez@apple.com>
     2
     3        radio / checkbox inputs should fire "click, input, change" events in order when clicked
     4        https://bugs.webkit.org/show_bug.cgi?id=190223
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Update existing test to reflect behavior change. I have verified that our new behavior
     9        on this test is consistent with Gecko and Chrome.
     10
     11        * fast/forms/radio/radio-group-keyboard-change-event-expected.txt:
     12        * fast/forms/radio/radio-group-keyboard-change-event.html:
     13
    1142018-10-02  Chris Dumez  <cdumez@apple.com>
    215

  • trunk/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event-expected.txt

    r108612 r236779  
    99d e f
    1010
     11b dispatched click event
     12PASS: b is checked
     13b dispatched input event
     14PASS: b is checked
    1115b dispatched change event
     16PASS: b is checked
     17c dispatched click event
     18PASS: c is checked
     19c dispatched input event
     20PASS: c is checked
    1221c dispatched change event
    13 e dispatched change event
    14 f dispatched change event
     22PASS: c is checked
     23e dispatched click event
     24PASS: e is checked
     25f dispatched click event
     26PASS: f is checked
    1527PASS: a is not checked
    1628PASS: b is not checked

  • trunk/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html

    r121008 r236779  
    99
    1010<p>
    11 <input type=radio name=aaa value=a checked onchange="handleChange(event)" onclick="handleClick(event)">a
    12 <input type=radio name=aaa value=b onchange="handleChange(event)" onclick="handleClick(event)">b
    13 <input type=radio name=aaa value=c onchange="handleChange(event)" onclick="handleClick(event)">c
     11<input type=radio name=aaa value=a checked onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">a
     12<input type=radio name=aaa value=b onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">b
     13<input type=radio name=aaa value=c onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">c
    1414
    1515<p>For manual testing, focus a radio button in the second group and use the arrow keys. Change events
    … …  
    1717
    1818<p>
    19 <input type=radio name=bbb value=d checked onchange="handleChange(event)" onclick="handleClick(event)">d
    20 <input type=radio name=bbb value=e onchange="handleChange(event)" onclick="handleClick(event)">e
    21 <input type=radio name=bbb value=f onchange="handleChange(event)" onclick="handleClick(event)">f
     19<input type=radio name=bbb value=d checked onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">d
     20<input type=radio name=bbb value=e onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">e
     21<input type=radio name=bbb value=f onchange="handleChange(event)" onclick="handleClick(event)" oninput="handleInput(event)">f
    2222
    2323<pre id=out></pre>
    … …  
    2727var preventClickValues = 'def';
    2828
     29function handleInput(e)
     30{
     31    var value = e.target.value;
     32    print(value + ' dispatched input event');
     33    assertChecked(e.target.value);
     34}
     35
    2936function handleChange(e)
    3037{
    3138    var value = e.target.value;
    3239    print(value + ' dispatched change event');
     40    assertChecked(e.target.value);
    3341}
    3442
    … …  
    3644{
    3745    var value = e.target.value;
     46    print(value + ' dispatched click event');
     47    assertChecked(e.target.value);
    3848    if (preventClickValues.indexOf(value) !== -1)
    3949        e.preventDefault();

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r236778 r236779  
     12018-10-02  Chris Dumez  <cdumez@apple.com>
     2
     3        radio / checkbox inputs should fire "click, input, change" events in order when clicked
     4        https://bugs.webkit.org/show_bug.cgi?id=190223
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Rebaseline a few WPT tests that are now passing. I have verified that those are passing in Gecko and Blink
     9        as well.
     10
     11        * web-platform-tests/html/semantics/forms/the-input-element/checkbox-click-events-expected.txt:
     12        * web-platform-tests/html/semantics/forms/the-input-element/checkbox-expected.txt:
     13        * web-platform-tests/html/semantics/forms/the-input-element/radio-expected.txt:
     14
    1152018-10-02  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/checkbox-click-events-expected.txt

    r217225 r236779  
    22
    33PASS clicking and preventDefaulting a checkbox causes the checkbox to be checked during the click handler but reverted
    4 FAIL a checkbox input emits click, input, change events in order after synthetic click assert_array_equals: lengths differ, expected 3 got 2
    5 FAIL a checkbox input emits click, input, change events in order after dispatching click event assert_array_equals: lengths differ, expected 3 got 2
    6 FAIL checkbox input respects cancel behavior on synthetic clicks assert_array_equals: lengths differ, expected 1 got 2
     4PASS a checkbox input emits click, input, change events in order after synthetic click
     5PASS a checkbox input emits click, input, change events in order after dispatching click event
     6PASS checkbox input respects cancel behavior on synthetic clicks
    77

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/checkbox-expected.txt

    r204090 r236779  
    11
    2 FAIL click on mutable checkbox fires a click event, then an input event, then a change event assert_true: change event should fire after click event expected true got false
     2PASS click on mutable checkbox fires a click event, then an input event, then a change event
    33PASS click on non-mutable checkbox doesn't fire the input or change event
    44PASS pre-activation steps on unchecked checkbox

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/radio-expected.txt

    r217225 r236779  
    11
    2 FAIL click on mutable radio fires click event, then input event, then change event Can't find variable: click_fired
     2PASS click on mutable radio fires click event, then input event, then change event
    33PASS click on non-mutable radio doesn't fire the input event
    44PASS click on non-mutable radio doesn't fire the change event

  • trunk/Source/WebCore/ChangeLog

    r236778 r236779  
     12018-10-02  Chris Dumez  <cdumez@apple.com>
     2
     3        radio / checkbox inputs should fire "click, input, change" events in order when clicked
     4        https://bugs.webkit.org/show_bug.cgi?id=190223
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        radio / checkbox inputs should fire "click, input, change" events in order when clicked:
     9        - https://html.spec.whatwg.org/#radio-button-state-(type=radio)
     10        - https://html.spec.whatwg.org/#checkbox-state-(type=checkbox)
     11        - https://dom.spec.whatwg.org/#ref-for-eventtarget-activation-behavior③ (step 11)
     12
     13        Gecko and Blink already behave this way. However, WebKit has the following issues:
     14        - the input event is not fired
     15        - the click event is fired after the change event
     16
     17        No new tests, updated / rebaselined existing tests.
     18
     19        * html/BaseCheckableInputType.cpp:
     20        (WebCore::BaseCheckableInputType::fireInputAndChangeEvents):
     21        * html/BaseCheckableInputType.h:
     22        * html/CheckboxInputType.cpp:
     23        (WebCore::CheckboxInputType::willDispatchClick):
     24        (WebCore::CheckboxInputType::didDispatchClick):
     25        * html/HTMLInputElement.cpp:
     26        (WebCore::HTMLInputElement::setChecked):
     27        * html/HTMLInputElement.h:
     28        * html/RadioInputType.cpp:
     29        (WebCore::RadioInputType::willDispatchClick):
     30        (WebCore::RadioInputType::didDispatchClick):
     31
    1322018-10-02  Chris Dumez  <cdumez@apple.com>
    233

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

    r233122 r236779  
    120120}
    121121
     122void BaseCheckableInputType::fireInputAndChangeEvents()
     123{
     124    if (!element()->isConnected())
     125        return;
     126
     127    if (!shouldSendChangeEventAfterCheckedChanged())
     128        return;
     129
     130    element()->setTextAsOfLastFormControlChangeEvent(String());
     131    element()->dispatchInputEvent();
     132    element()->dispatchFormControlChangeEvent();
     133}
     134
    122135} // namespace WebCore

  • trunk/Source/WebCore/html/BaseCheckableInputType.h

    r221914 r236779  
    4040    explicit BaseCheckableInputType(HTMLInputElement& element) : InputType(element) { }
    4141    void handleKeydownEvent(KeyboardEvent&) override;
     42    void fireInputAndChangeEvents();
    4243
    4344private:

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

    r232496 r236779  
    7777        element()->setIndeterminate(false);
    7878
    79     element()->setChecked(!state.checked, DispatchChangeEvent);
     79    element()->setChecked(!state.checked);
    8080}
    8181
    … …  
    8686        element()->setIndeterminate(state.indeterminate);
    8787        element()->setChecked(state.checked);
    88     }
     88    } else
     89        fireInputAndChangeEvents();
    8990
    9091    // The work we did in willDispatchClick was default handling.

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

    r235004 r236779  
    913913}
    914914
    915 void HTMLInputElement::setChecked(bool nowChecked, TextFieldEventBehavior eventBehavior)
     915void HTMLInputElement::setChecked(bool nowChecked)
    916916{
    917917    if (checked() == nowChecked)
    … …  
    934934        if (AXObjectCache* cache = renderer()->document().existingAXObjectCache())
    935935            cache->checkedStateChanged(this);
    936     }
    937 
    938     // Only send a change event for items in the document (avoid firing during
    939     // parsing) and don't send a change event for a radio button that's getting
    940     // unchecked to match other browsers. DOM is not a useful standard for this
    941     // because it says only to fire change events at "lose focus" time, which is
    942     // definitely wrong in practice for these types of elements.
    943     if (eventBehavior != DispatchNoEvent && isConnected() && m_inputType->shouldSendChangeEventAfterCheckedChanged()) {
    944         setTextAsOfLastFormControlChangeEvent(String());
    945         dispatchFormControlChangeEvent();
    946936    }
    947937

  • trunk/Source/WebCore/html/HTMLInputElement.h

    r235556 r236779  
    156156
    157157    bool checked() const { return m_isChecked; }
    158     WEBCORE_EXPORT void setChecked(bool, TextFieldEventBehavior = DispatchNoEvent);
     158    WEBCORE_EXPORT void setChecked(bool);
    159159
    160160    // 'indeterminate' is a state independent of the checked state that causes the control to draw in a way that hides the actual state.

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

    r232496 r236779  
    158158    state.checkedRadioButton = element()->checkedRadioButtonForGroup();
    159159
    160     element()->setChecked(true, DispatchChangeEvent);
     160    element()->setChecked(true);
    161161}
    162162
    … …  
    170170        if (button && button->isRadioButton() && button->form() == element()->form() && button->name() == element()->name())
    171171            button->setChecked(true);
    172     }
     172    } else
     173        fireInputAndChangeEvents();
    173174
    174175    // The work we did in willDispatchClick was default handling.
Note: See TracChangeset for help on using the changeset viewer.