Changeset 286855 in webkit


Ignore:
Timestamp:
Dec 10, 2021 9:16:51 AM (7 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.

  • dom/ContainerNode.h:

(WebCore::ContainerNode::rootNode const):

  • dom/ElementTraversal.h:

(WebCore::Traversal<ElementType>::inclusiveFirstWithin):

  • 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/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 WPT tests now that more checks are passing.

Location:
trunk
Files:
23 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r286854 r286855  
     12021-12-10  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/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt:
     17        * platform/mac-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valueMissing-expected.txt:
     18        Rebaseline WPT tests now that more checks are passing.
     19
    1202021-12-10  Youenn Fablet  <youenn@apple.com>
    221

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

    r108612 r286855  
    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

    r155268 r286855  
    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

    r174336 r286855  
    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

    r174336 r286855  
    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

    r286853 r286855  
     12021-12-10  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
    1152021-12-10  Patrick Griffis  <pgriffis@igalia.com>
    216

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

    r286482 r286855  
    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

    r286482 r286855  
    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

    r279427 r286855  
    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/ios-wk2/imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-validity-valid-expected.txt

    r286482 r286855  
    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

    r286482 r286855  
    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

    r286482 r286855  
    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

    r286482 r286855  
    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

    r286853 r286855  
     12021-12-10  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        * dom/ContainerNode.h:
     20        (WebCore::ContainerNode::rootNode const):
     21        * dom/ElementTraversal.h:
     22        (WebCore::Traversal<ElementType>::inclusiveFirstWithin):
     23        * html/HTMLInputElement.cpp:
     24        (WebCore::HTMLInputElement::setChecked):
     25        (WebCore::HTMLInputElement::didChangeForm):
     26        (WebCore::HTMLInputElement::insertedIntoAncestor):
     27        (WebCore::HTMLInputElement::removedFromAncestor):
     28        (WebCore::HTMLInputElement::checkedRadioButtonForGroup const):
     29        * html/InputType.h:
     30        (WebCore::InputType::willUpdateCheckedness):
     31        * html/RadioInputType.cpp:
     32        (WebCore::RadioInputType::valueMissing const):
     33        (WebCore::RadioInputType::willUpdateCheckedness):
     34        * html/RadioInputType.h:
     35
    1362021-12-10  Patrick Griffis  <pgriffis@igalia.com>
    237

  • trunk/Source/WebCore/dom/ContainerNode.h

    r286091 r286855  
    6060    void stringReplaceAll(const String&);
    6161    void replaceAll(Node*);
     62
     63    ContainerNode& rootNode() const { return downcast<ContainerNode>(Node::rootNode()); }
    6264
    6365    // These methods are only used during parsing.

  • trunk/Source/WebCore/dom/ElementTraversal.h

    r208179 r286855  
    4444    static ElementType* lastWithin(const Node&);
    4545    static ElementType* lastWithin(const ContainerNode&);
     46    static ElementType* inclusiveFirstWithin(Node&);
     47    static ElementType* inclusiveFirstWithin(ContainerNode&);
     48    static ElementType* inclusiveLastWithin(Node&);
     49    static ElementType* inclusiveLastWithin(ContainerNode&);
    4650
    4751    // Pre-order traversal skipping non-ElementType nodes.
    … …  
    236240
    237241template <typename ElementType>
     242inline ElementType* Traversal<ElementType>::inclusiveFirstWithin(ContainerNode& current)
     243{
     244    if (is<ElementType>(current))
     245        return &downcast<ElementType>(current);
     246    return firstWithin(current);
     247}
     248
     249template <typename ElementType>
     250inline ElementType* Traversal<ElementType>::inclusiveFirstWithin(Node& current)
     251{
     252    if (is<ElementType>(current))
     253        return &downcast<ElementType>(current);
     254    return firstWithin(current);
     255}
     256
     257template <typename ElementType>
     258inline ElementType* Traversal<ElementType>::inclusiveLastWithin(ContainerNode& current)
     259{
     260    if (is<ElementType>(current))
     261        return &downcast<ElementType>(current);
     262    return lastWithin(current);
     263}
     264
     265template <typename ElementType>
     266inline ElementType* Traversal<ElementType>::inclusiveLastWithin(Node& current)
     267{
     268    if (is<ElementType>(current))
     269        return &downcast<ElementType>(current);
     270    return lastWithin(current);
     271}
     272
     273template <typename ElementType>
    238274inline ElementType* Traversal<ElementType>::firstWithin(const ContainerNode& current) { return firstWithinTemplate(current); }
    239275template <typename ElementType>

  • trunk/Source/WebCore/dom/TypedElementDescendantIterator.h

    r257192 r286855  
    6969};
    7070
     71template<typename ElementType> class InclusiveElementDescendantRange {
     72public:
     73    InclusiveElementDescendantRange(const ContainerNode& root);
     74    ElementDescendantIterator<ElementType> begin() const;
     75    static constexpr std::nullptr_t end() { return nullptr; }
     76    ElementDescendantIterator<ElementType> beginAt(ElementType&) const;
     77    ElementDescendantIterator<ElementType> from(Element&) const;
     78
     79    ElementType* first() const;
     80    ElementType* last() const;
     81
     82private:
     83    const ContainerNode& m_root;
     84};
     85
    7186template<typename ElementType> class DoubleElementDescendantRange {
    7287public:
    … …  
    172187}
    173188
     189// InclusiveElementDescendantRange
     190
     191template<typename ElementType> InclusiveElementDescendantRange<ElementType>::InclusiveElementDescendantRange(const ContainerNode& root)
     192    : m_root(root)
     193{
     194}
     195
     196template<typename ElementType> ElementDescendantIterator<ElementType> InclusiveElementDescendantRange<ElementType>::begin() const
     197{
     198    return ElementDescendantIterator<ElementType>(m_root, Traversal<ElementType>::inclusiveFirstWithin(const_cast<ContainerNode&>(m_root)));
     199}
     200
     201template<typename ElementType> ElementDescendantIterator<ElementType> InclusiveElementDescendantRange<ElementType>::beginAt(ElementType& descendant) const
     202{
     203    ASSERT(&m_root == &descendant || descendant.isDescendantOf(m_root));
     204    return ElementDescendantIterator<ElementType>(m_root, &descendant);
     205}
     206
     207template<typename ElementType> ElementDescendantIterator<ElementType> InclusiveElementDescendantRange<ElementType>::from(Element& descendant) const
     208{
     209    ASSERT(&m_root == &descendant || descendant.isDescendantOf(m_root));
     210    if (is<ElementType>(descendant))
     211        return ElementDescendantIterator<ElementType>(m_root, downcast<ElementType>(&descendant));
     212    ElementType* next = Traversal<ElementType>::next(descendant, &m_root);
     213    return ElementDescendantIterator<ElementType>(m_root, next);
     214}
     215
     216template<typename ElementType> ElementType* InclusiveElementDescendantRange<ElementType>::first() const
     217{
     218    return Traversal<ElementType>::inclusiveFirstWithin(m_root);
     219}
     220
     221template<typename ElementType> ElementType* InclusiveElementDescendantRange<ElementType>::last() const
     222{
     223    return Traversal<ElementType>::inclusiveLastWithin(m_root);
     224}
     225
    174226// DoubleElementDescendantRange
    175227
    … …  
    252304}
    253305
     306template<typename ElementType> InclusiveElementDescendantRange<ElementType> inclusiveDescendantsOfType(ContainerNode& root)
     307{
     308    return InclusiveElementDescendantRange<ElementType>(root);
     309}
     310
    254311template<typename ElementType> ElementDescendantRange<const ElementType> descendantsOfType(const ContainerNode& root)
    255312{

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

    r285575 r286855  
    302302}
    303303
    304 bool HTMLFormControlElement::isRequired() const
    305 {
    306     return m_isRequired;
    307 }
    308 
    309304void HTMLFormControlElement::didRecalcStyle(Style::Change)
    310305{

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

    r286447 r286855  
    7777    bool isEnumeratable() const override { return false; }
    7878
    79     bool isRequired() const;
     79    bool isRequired() const { return m_isRequired; }
    8080
    8181    const AtomString& type() const { return formControlType(); }

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

    r286560 r286855  
    6969#include "StyleGeneratedImage.h"
    7070#include "TextControlInnerElements.h"
     71#include "TypedElementDescendantIterator.h"
    7172#include <wtf/IsoMallocInlines.h>
    7273#include <wtf/Language.h>
    … …  
    980981        return;
    981982
     983    m_inputType->willUpdateCheckedness(nowChecked);
     984
    982985    m_dirtyCheckednessFlag = true;
    983986    m_isChecked = nowChecked;
    … …  
    15771580void HTMLInputElement::didChangeForm()
    15781581{
     1582    addToRadioButtonGroup();
    15791583    HTMLTextFormControlElement::didChangeForm();
    1580     addToRadioButtonGroup();
    15811584}
    15821585
    15831586Node::InsertedIntoAncestorResult HTMLInputElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
    15841587{
     1588    if (isRadioButton())
     1589        updateValidity();
    15851590    HTMLTextFormControlElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
    15861591#if ENABLE(DATALIST_ELEMENT)
    … …  
    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    if (checked())
     1967        return const_cast<HTMLInputElement*>(this);
     1968
     1969    auto& name = this->name();
    19591970    if (RadioButtonGroups* buttons = radioButtonGroups())
    1960         return buttons->checkedButtonForGroup(name());
     1971        return buttons->checkedButtonForGroup(name);
     1972
     1973    if (name.isEmpty())
     1974        return nullptr;
     1975
     1976    // The input is not managed by a RadioButtonGroups, we'll need to traverse the tree.
     1977    for (auto& descendant : descendantsOfType<HTMLInputElement>(rootNode())) {
     1978        if (descendant.checked() && descendant.isRadioButton() && !descendant.form() && name == descendant.name())
     1979            return &descendant;
     1980    }
    19611981    return nullptr;
    19621982}

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

    r286560 r286855  
    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

    r272097 r286855  
    3333#include "NodeTraversal.h"
    3434#include "SpatialNavigation.h"
     35#include "TypedElementDescendantIterator.h"
    3536
    3637namespace WebCore {
    … …  
    4647{
    4748    ASSERT(element());
    48     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    bool isRequired = false;
     57    for (auto& input : inclusiveDescendantsOfType<HTMLInputElement>(element()->rootNode())) {
     58        if (!input.isRadioButton() || input.form() || input.name() != name)
     59            continue;
     60        if (input.checked())
     61            return false;
     62        if (input.isRequired())
     63            isRequired = true;
     64    }
     65    return isRequired;
     66}
     67
     68void RadioInputType::willUpdateCheckedness(bool nowChecked)
     69{
     70    if (!nowChecked)
     71        return;
     72    if (element()->radioButtonGroups()) {
     73        // Buttons in RadioButtonGroups are handled in HTMLInputElement::setChecked().
     74        return;
     75    }
     76    if (auto input = element()->checkedRadioButtonForGroup())
     77        input->setChecked(false);
    4978}
    5079

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

    r272097 r286855  
    5353    void didDispatchClick(Event&, const InputElementClickState&) final;
    5454    bool matchesIndeterminatePseudoClass() const final;
     55    void willUpdateCheckedness(bool nowChecked) final;
    5556};
    5657
Note: See TracChangeset for help on using the changeset viewer.