Changeset 288734 in webkit


Ignore:
Timestamp:
Jan 27, 2022 11:58:57 PM (6 months ago)
Author:
Chris Dumez
Message:

Radio buttons with no form owner are not grouped
​https://bugs.webkit.org/show_bug.cgi?id=220502
<rdar://problem/73300895>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline WPT tests now that more checks are passing.

  • web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
  • web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
  • web-platform-tests/html/semantics/forms/the-input-element/radio-expected.txt:

Source/WebCore:

Per the HTML specification and to match the behavior of both Gecko and Blink,
radio buttons should still be grouped, even if they are disconnected and not
owned by a form.

This patch aligns our behavior with Gecko and Blink and is based on the following
Blink commit:

No new tests, rebaselined existing tests.

  • html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::setChecked):
(WebCore::HTMLInputElement::didChangeForm):
(WebCore::HTMLInputElement::insertedIntoAncestor):
(WebCore::HTMLInputElement::removedFromAncestor):
(WebCore::HTMLInputElement::checkedRadioButtonForGroup const):

  • html/InputType.h:

(WebCore::InputType::willUpdateCheckedness):

  • html/RadioInputType.cpp:

(WebCore::RadioInputType::valueMissing const):
(WebCore::RadioInputType::willUpdateCheckedness):

  • html/RadioInputType.h:

LayoutTests:

  • fast/forms/radio/ValidityState-valueMissing-radio-expected.txt:
  • fast/forms/radio/ValidityState-valueMissing-radio.html:
  • fast/forms/radio/radio-live-validation-style-expected.txt:
  • fast/forms/radio/radio-live-validation-style.html:

Update existing tests to reflect behavior change. I have verified that our behavior on those tests
is aligned with both Firefox and Chrome.

  • platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
  • platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
  • platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
  • platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
  • platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
  • platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:

Rebaseline existing WPT tests now that more checks are passing.

Location:
trunk
Files:
20 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r288728 r288734  
     12022-01-27  Chris Dumez  <cdumez@apple.com>
     2
     3        Radio buttons with no form owner are not grouped
     4        https://bugs.webkit.org/show_bug.cgi?id=220502
     5        <rdar://problem/73300895>
     6
     7        Reviewed by Darin Adler.
     8
     9        * fast/forms/radio/ValidityState-valueMissing-radio-expected.txt:
     10        * fast/forms/radio/ValidityState-valueMissing-radio.html:
     11        * fast/forms/radio/radio-live-validation-style-expected.txt:
     12        * fast/forms/radio/radio-live-validation-style.html:
     13        Update existing tests to reflect behavior change. I have verified that our behavior on those tests
     14        is aligned with both Firefox and Chrome.
     15
     16        * platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
     17        * platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
     18        * platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
     19        * platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
     20        * platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
     21        * platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
     22        Rebaseline existing WPT tests now that more checks are passing.
     23
    1242022-01-27  Antoine Quint  <graouts@webkit.org>
    225

  • trunk/LayoutTests/fast/forms/radio/ValidityState-valueMissing-radio-expected.txt

    r288631 r288734  
    4141
    4242Not in a radio button group
    43 PASS requiredButton.validity.valueMissing is false
     43PASS requiredButton.validity.valueMissing is true
    4444PASS requiredButton.validity.valueMissing is true
    4545PASS button.validity.valueMissing is true
    4646PASS button.validity.valueMissing is false
    47 PASS requiredButton.validity.valueMissing is false
     47PASS requiredButton.validity.valueMissing is true
    4848PASS successfullyParsed is true
    4949

  • trunk/LayoutTests/fast/forms/radio/ValidityState-valueMissing-radio.html

    r288631 r288734  
    7272requiredButton.name = 'victim';
    7373requiredButton.required = true;
    74 shouldBeFalse('requiredButton.validity.valueMissing');
     74shouldBeTrue('requiredButton.validity.valueMissing');
    7575
    7676parent.innerHTML = '<input name=victim type=radio required><input name=victim type=radio>';
    … …  
    8282shouldBeFalse('button.validity.valueMissing');
    8383parent.removeChild(requiredButton);
    84 shouldBeFalse('requiredButton.validity.valueMissing');
     84shouldBeTrue('requiredButton.validity.valueMissing');
    8585
    8686</script>

  • trunk/LayoutTests/fast/forms/radio/radio-live-validation-style-expected.txt

    r288631 r288734  
    88PASS parent.removeChild($("radio2")); backgroundOf($("radio1")) is invalidColor
    99PASS $("radio1").remove(); radio2.matches(":valid") is false
    10 PASS radio2.remove(); radio2.matches(":valid") is true
     10PASS radio2.remove(); radio2.matches(":valid") is false
    1111
    1212Removing a checked radio button from a required radio button group by name attribute change:

  • trunk/LayoutTests/fast/forms/radio/radio-live-validation-style.html

    r288631 r288734  
    3636var radio2 = $('radio2');
    3737shouldBeFalse('$("radio1").remove(); radio2.matches(":valid")');
    38 shouldBeTrue('radio2.remove(); radio2.matches(":valid")');
     38shouldBeFalse('radio2.remove(); radio2.matches(":valid")');
    3939debug('');
    4040

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r288707 r288734  
     12022-01-27  Chris Dumez  <cdumez@apple.com>
     2
     3        Radio buttons with no form owner are not grouped
     4        https://bugs.webkit.org/show_bug.cgi?id=220502
     5        <rdar://problem/73300895>
     6
     7        Reviewed by Darin Adler.
     8
     9        Rebaseline WPT tests now that more checks are passing.
     10
     11        * web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
     12        * web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
     13        * web-platform-tests/html/semantics/forms/the-input-element/radio-expected.txt:
     14
    1152022-01-27  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt

    r288631 r288734  
    2424PASS [INPUT in NUMBER status] validity.valid must be false if validity.valueMissing is true
    2525PASS [INPUT in CHECKBOX status] validity.valid must be false if validity.valueMissing is true
    26 FAIL [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true assert_false: The validity.valid should be false. expected false got true
     26PASS [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true
    2727PASS [INPUT in FILE status] validity.valid must be false if validity.valueMissing is true
    2828PASS [select]  validity.valid must be false if validity.valueMissing is true

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt

    r288631 r288734  
    3939PASS [INPUT in RADIO status] The required attribute is not set
    4040PASS [INPUT in RADIO status] The checked attribute is true
    41 FAIL [INPUT in RADIO status] The checked attribute is false assert_true: The validity.valueMissing should be true. expected true got false
     41PASS [INPUT in RADIO status] The checked attribute is false
    4242PASS [INPUT in RADIO status] The checked attribute is false and the name attribute is empty
    4343PASS [INPUT in FILE status] The required attribute is not set

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

    r288631 r288734  
    88PASS changing the name of a radio input element and setting its checkedness to true makes all the other elements' checkedness in the same radio button group be set to false
    99PASS moving radio input element out of or into a form should still work as expected
    10 FAIL Radio buttons in an orphan tree should make a group assert_false: The second radio should be unchecked after setting checked expected false got true
    11 FAIL Radio buttons in different groups (because they have different form owners or no form owner) do not affect each other's checkedness assert_false: radio5 should be unchecked expected false got true
     10PASS Radio buttons in an orphan tree should make a group
     11PASS Radio buttons in different groups (because they have different form owners or no form owner) do not affect each other's checkedness
    1212PASS Radio buttons in different groups (because they are not in the same tree) do not affect each other's checkedness
    1313PASS Radio buttons in different groups (because they have different name attribute values, or no name attribute) do not affect each other's checkedness

  • trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt

    r288631 r288734  
    3939PASS [INPUT in NUMBER status] validity.valid must be false if validity.valueMissing is true
    4040PASS [INPUT in CHECKBOX status] validity.valid must be false if validity.valueMissing is true
    41 FAIL [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true assert_false: The validity.valid should be false. expected false got true
     41PASS [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true
    4242PASS [INPUT in FILE status] validity.valid must be false if validity.valueMissing is true
    4343PASS [select]  validity.valid must be false if validity.valueMissing is true

  • trunk/LayoutTests/platform/gtk/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt

    r288631 r288734  
    8585PASS [INPUT in RADIO status] The required attribute is not set
    8686PASS [INPUT in RADIO status] The checked attribute is true
    87 FAIL [INPUT in RADIO status] The checked attribute is false assert_true: The validity.valueMissing should be true. expected true got false
     87PASS [INPUT in RADIO status] The checked attribute is false
    8888PASS [INPUT in RADIO status] The checked attribute is false and the name attribute is empty
    8989PASS [INPUT in FILE status] The required attribute is not set

  • trunk/LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt

    r288631 r288734  
    3939PASS [INPUT in NUMBER status] validity.valid must be false if validity.valueMissing is true
    4040PASS [INPUT in CHECKBOX status] validity.valid must be false if validity.valueMissing is true
    41 FAIL [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true assert_false: The validity.valid should be false. expected false got true
     41PASS [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true
    4242PASS [INPUT in FILE status] validity.valid must be false if validity.valueMissing is true
    4343PASS [select]  validity.valid must be false if validity.valueMissing is true

  • trunk/LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt

    r288631 r288734  
    8585PASS [INPUT in RADIO status] The required attribute is not set
    8686PASS [INPUT in RADIO status] The checked attribute is true
    87 FAIL [INPUT in RADIO status] The checked attribute is false assert_true: The validity.valueMissing should be true. expected true got false
     87PASS [INPUT in RADIO status] The checked attribute is false
    8888PASS [INPUT in RADIO status] The checked attribute is false and the name attribute is empty
    8989PASS [INPUT in FILE status] The required attribute is not set

  • trunk/LayoutTests/platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt

    r288631 r288734  
    3939PASS [INPUT in NUMBER status] validity.valid must be false if validity.valueMissing is true
    4040PASS [INPUT in CHECKBOX status] validity.valid must be false if validity.valueMissing is true
    41 FAIL [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true assert_false: The validity.valid should be false. expected false got true
     41PASS [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true
    4242PASS [INPUT in FILE status] validity.valid must be false if validity.valueMissing is true
    4343PASS [select]  validity.valid must be false if validity.valueMissing is true

  • trunk/LayoutTests/platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt

    r288631 r288734  
    8585PASS [INPUT in RADIO status] The required attribute is not set
    8686PASS [INPUT in RADIO status] The checked attribute is true
    87 FAIL [INPUT in RADIO status] The checked attribute is false assert_true: The validity.valueMissing should be true. expected true got false
     87PASS [INPUT in RADIO status] The checked attribute is false
    8888PASS [INPUT in RADIO status] The checked attribute is false and the name attribute is empty
    8989PASS [INPUT in FILE status] The required attribute is not set

  • trunk/Source/WebCore/ChangeLog

    r288733 r288734  
     12022-01-27  Chris Dumez  <cdumez@apple.com>
     2
     3        Radio buttons with no form owner are not grouped
     4        https://bugs.webkit.org/show_bug.cgi?id=220502
     5        <rdar://problem/73300895>
     6
     7        Reviewed by Darin Adler.
     8
     9        Per the HTML specification and to match the behavior of both Gecko and Blink,
     10        radio buttons should still be grouped, even if they are disconnected and not
     11        owned by a form.
     12
     13        This patch aligns our behavior with Gecko and Blink and is based on the following
     14        Blink commit:
     15        - https://chromium-review.googlesource.com/c/chromium/src/+/1988087
     16
     17        No new tests, rebaselined existing tests.
     18
     19        * html/HTMLInputElement.cpp:
     20        (WebCore::HTMLInputElement::setChecked):
     21        (WebCore::HTMLInputElement::didChangeForm):
     22        (WebCore::HTMLInputElement::insertedIntoAncestor):
     23        (WebCore::HTMLInputElement::removedFromAncestor):
     24        (WebCore::HTMLInputElement::checkedRadioButtonForGroup const):
     25        * html/InputType.h:
     26        (WebCore::InputType::willUpdateCheckedness):
     27        * html/RadioInputType.cpp:
     28        (WebCore::RadioInputType::valueMissing const):
     29        (WebCore::RadioInputType::willUpdateCheckedness):
     30        * html/RadioInputType.h:
     31
    1322022-01-27  Diego Pino Garcia  <dpino@igalia.com>
    233

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

    r288631 r288734  
    6262#include "PlatformMouseEvent.h"
    6363#include "PseudoClassChangeInvalidation.h"
     64#include "RadioInputType.h"
    6465#include "RenderTextControlSingleLine.h"
    6566#include "RenderTheme.h"
    … …  
    981982        return;
    982983
     984    m_inputType->willUpdateCheckedness(isChecked);
     985
    983986    Style::PseudoClassChangeInvalidation checkedInvalidation(*this, CSSSelector::PseudoClassChecked, isChecked);
    984987
    … …  
    15871590    resetListAttributeTargetObserver();
    15881591#endif
     1592    if (isRadioButton())
     1593        updateValidity();
    15891594    return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
    15901595}
    … …  
    16091614    HTMLTextFormControlElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
    16101615    ASSERT(!isConnected());
     1616    if (removalType.disconnectedFromDocument && !form() && isRadioButton())
     1617        updateValidity();
    16111618#if ENABLE(DATALIST_ELEMENT)
    16121619    resetListAttributeTargetObserver();
    … …  
    19571964RefPtr<HTMLInputElement> HTMLInputElement::checkedRadioButtonForGroup() const
    19581965{
     1966    ASSERT(isRadioButton());
     1967
     1968    if (checked())
     1969        return const_cast<HTMLInputElement*>(this);
     1970
     1971    auto& name = this->name();
    19591972    if (RadioButtonGroups* buttons = radioButtonGroups())
    1960         return buttons->checkedButtonForGroup(name());
    1961     return nullptr;
     1973        return buttons->checkedButtonForGroup(name);
     1974
     1975    if (name.isEmpty())
     1976        return nullptr;
     1977
     1978    ASSERT(!isConnected());
     1979    ASSERT(!form());
     1980
     1981    // The input is not managed by a RadioButtonGroups, we'll need to traverse the tree.
     1982    RefPtr<HTMLInputElement> checkedRadio;
     1983    RadioInputType::forEachButtonInDetachedGroup(rootNode(), name, [&](auto& input) {
     1984        if (input.checked()) {
     1985            checkedRadio = &input;
     1986            return false;
     1987        }
     1988        return true;
     1989    });
     1990    return checkedRadio;
    19621991}
    19631992

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

    r288631 r288734  
    364364    virtual bool isFocusingWithDataListDropdown() const { return false; };
    365365#endif
     366    virtual void willUpdateCheckedness(bool /*nowChecked*/) { }
    366367
    367368    // Parses the specified string for the type, and return

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

    r288631 r288734  
    4747{
    4848    ASSERT(element());
    49     return element()->isInRequiredRadioButtonGroup() && !element()->checkedRadioButtonForGroup();
     49    auto& name = element()->name();
     50    if (auto* buttons = element()->radioButtonGroups())
     51        return !buttons->checkedButtonForGroup(name) && buttons->isInRequiredGroup(*element());
     52
     53    if (name.isEmpty())
     54        return false;
     55
     56    ASSERT(!element()->isConnected());
     57    ASSERT(!element()->form());
     58
     59    bool isRequired = false;
     60    bool foundCheckedRadio = false;
     61    forEachButtonInDetachedGroup(element()->rootNode(), name, [&](auto& input) {
     62        if (input.checked()) {
     63            foundCheckedRadio = true;
     64            return false;
     65        }
     66        if (input.isRequired())
     67            isRequired = true;
     68        return true;
     69    });
     70    return isRequired && !foundCheckedRadio;
     71}
     72
     73void RadioInputType::forEachButtonInDetachedGroup(ContainerNode& rootNode, const String& groupName, const Function<bool(HTMLInputElement&)>& apply)
     74{
     75    ASSERT(!groupName.isEmpty());
     76
     77    for (auto* descendant = Traversal<HTMLElement>::inclusiveFirstWithin(rootNode); descendant;) {
     78        if (is<HTMLFormElement>(*descendant)) {
     79            // No need to consider the descendants of a <form> since they will have a form owner and we're only
     80            // interested in <input> elements without a form owner.
     81            descendant = Traversal<HTMLElement>::nextSkippingChildren(*descendant, &rootNode);
     82            continue;
     83        }
     84        auto* input = dynamicDowncast<HTMLInputElement>(*descendant);
     85        if (input && input->isRadioButton() && !input->form() && input->name() == groupName) {
     86            bool shouldContinue = apply(*input);
     87            if (!shouldContinue)
     88                return;
     89        }
     90        descendant = Traversal<HTMLElement>::next(*descendant, &rootNode);
     91    }
     92}
     93
     94void RadioInputType::willUpdateCheckedness(bool nowChecked)
     95{
     96    if (!nowChecked)
     97        return;
     98    if (element()->radioButtonGroups()) {
     99        // Buttons in RadioButtonGroups are handled in HTMLInputElement::setChecked().
     100        return;
     101    }
     102    if (auto input = element()->checkedRadioButtonForGroup())
     103        input->setChecked(false);
    50104}
    51105

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

    r288631 r288734  
    4141    explicit RadioInputType(HTMLInputElement& element) : BaseCheckableInputType(Type::Radio, element) { }
    4242
     43    static void forEachButtonInDetachedGroup(ContainerNode& rootName, const String& groupName, const Function<bool(HTMLInputElement&)>&);
     44
    4345private:
    4446    const AtomString& formControlType() const final;
    … …  
    5355    void didDispatchClick(Event&, const InputElementClickState&) final;
    5456    bool matchesIndeterminatePseudoClass() const final;
     57    void willUpdateCheckedness(bool nowChecked) final;
    5558};
    5659
Note: See TracChangeset for help on using the changeset viewer.