Skip to content

Commit

Permalink
Regression(264098@main) HTMLFieldsetElement behavior for :enabled / :…
Browse files Browse the repository at this point in the history
…disabled CSS selectors is incorrect

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

Reviewed by Tim Nguyen.

264098@main updated HTMLFieldSetElement::isDisabledFormControl() to return false
since a fieldset element can never be disabled per the specification.

This had the side effect of affecting our behavior for :enabled / :disabled CSS
selectors, which relied on this function. However, the behavior of these CSS
selectors shouldn't have changed since they rely on the "actually disabled" state,
not the "disabled" state [1].

There were two issues here, some of our CSS selector logic was relying on
isDisabledFormControl() instead of isActuallyDisabled(). Also, 264098@main
shouldn't have changed the value returned by isActuallyDisabled() for
HTMLFieldsetElements.

This was pointed out at:
- whatwg/html#5886 (comment)

[1] https://html.spec.whatwg.org/multipage/semantics-other.html#selector-enabled
[2] https://html.spec.whatwg.org/multipage/semantics-other.html#selector-disabled
[3] https://html.spec.whatwg.org/multipage/semantics-other.html#concept-element-disabled
[4] https://html.spec.whatwg.org/multipage/form-elements.html#concept-fieldset-disabled

* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/disabled.html:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/selectors/pseudo-classes/enabled.html:
* Source/WebCore/css/SelectorCheckerTestFunctions.h:
(WebCore::matchesEnabledPseudoClass):
* Source/WebCore/html/HTMLElement.h:
* Source/WebCore/html/HTMLFieldSetElement.cpp:
(WebCore::HTMLFieldSetElement::isActuallyDisabled const):
* Source/WebCore/html/HTMLFieldSetElement.h:

Canonical link: https://commits.webkit.org/265166@main
  • Loading branch information
cdumez committed Jun 14, 2023
1 parent cae0100 commit a6d354d
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
<input type="radio" name="group12" readonly>
<span><input type="radio" name="group12" checked readonly class="checked"></span>
</form>
<fieldset disabled>
<fieldset disabled class="disabled">
<input type="radio" class="indeterminate disabled">
<input type="radio" class="indeterminate disabled">
<input type="radio" name="group1" class="indeterminate disabled">
Expand Down Expand Up @@ -141,7 +141,7 @@
<span><input type="radio" name="group12" checked readonly class="checked disabled"></span>
</fieldset>
<form>
<fieldset disabled>
<fieldset disabled class="disabled">
<input type="radio" class="indeterminate disabled">
<input type="radio" class="indeterminate disabled">
<input type="radio" name="group1" class="indeterminate disabled">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
</select>
<textarea id=textarea1>textarea1</textarea>
<textarea disabled id=textarea2>textarea2</textarea>
<fieldset id=fieldset1></fieldset>
<fieldset disabled id=fieldset2>
<legend><input type=checkbox id=club></legend>
<p><label>Name on card: <input id=clubname required></label></p>
Expand All @@ -39,21 +40,21 @@
<progress disabled></progress>

<script>
testSelectorIdsMatch(":disabled", ["button2", "input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should match only disabled elements");
testSelectorIdsMatch(":disabled", ["button2", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should match only disabled elements");

document.getElementById("button2").removeAttribute("disabled");
testSelectorIdsMatch(":disabled", ["input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should not match elements whose disabled attribute has been removed");
testSelectorIdsMatch(":disabled", ["input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should not match elements whose disabled attribute has been removed");

document.getElementById("button1").setAttribute("disabled", "disabled");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set");

document.getElementById("button1").setAttribute("disabled", "disabled");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set twice");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match elements whose disabled attribute has been set twice");

document.getElementById("input2").setAttribute("type", "submit"); // change input type to submit
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should also match disabled elements whose type has changed");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should also match disabled elements whose type has changed");

var input = document.createElement("input");
input.setAttribute("disabled", "disabled");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "clubname", "clubnum"], "':disabled' should not match elements not in the document");
testSelectorIdsMatch(":disabled", ["button1", "input2", "select2", "optgroup2", "option2", "textarea2", "fieldset2", "clubname", "clubnum"], "':disabled' should not match elements not in the document");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
</menu>
</form>
<fieldset id=fieldset1></fieldset>
<fieldset disabled id=fieldset2></fieldset>

<script>
testSelectorIdsMatch(":enabled", ["button1", "input1", "select1", "optgroup1", "option1", "textarea1", "submitbutton", "fieldset1"], "':enabled' elements that are not disabled");
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/css/SelectorCheckerTestFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ ALWAYS_INLINE bool matchesDisabledPseudoClass(const Element& element)
// https://html.spec.whatwg.org/multipage/scripting.html#selector-enabled
ALWAYS_INLINE bool matchesEnabledPseudoClass(const Element& element)
{
return is<HTMLElement>(element) && downcast<HTMLElement>(element).canBeActuallyDisabled() && !element.isDisabledFormControl();
return is<HTMLElement>(element) && downcast<HTMLElement>(element).canBeActuallyDisabled() && !downcast<HTMLElement>(element).isActuallyDisabled();
}

// https://dom.spec.whatwg.org/#concept-element-defined
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/html/HTMLElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class HTMLElement : public StyledElement {

// Only some element types can be disabled: https://html.spec.whatwg.org/multipage/scripting.html#concept-element-disabled
bool canBeActuallyDisabled() const;
bool isActuallyDisabled() const;
virtual bool isActuallyDisabled() const;

#if ENABLE(AUTOCAPITALIZE)
WEBCORE_EXPORT virtual AutocapitalizeType autocapitalizeType() const;
Expand Down
7 changes: 7 additions & 0 deletions Source/WebCore/html/HTMLFieldSetElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ bool HTMLFieldSetElement::isDisabledFormControl() const
return HTMLFormControlElement::isDisabledFormControl();
}

// https://html.spec.whatwg.org/#concept-element-disabled
// https://html.spec.whatwg.org/#concept-fieldset-disabled
bool HTMLFieldSetElement::isActuallyDisabled() const
{
return HTMLFormControlElement::isDisabledFormControl();
}

static void updateFromControlElementsAncestorDisabledStateUnder(HTMLElement& startNode, bool isDisabled)
{
auto range = inclusiveDescendantsOfType<Element>(startNode);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/html/HTMLFieldSetElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class HTMLFieldSetElement final : public HTMLFormControlElement {
~HTMLFieldSetElement();

bool isDisabledFormControl() const final;
bool isActuallyDisabled() const final;
bool isEnumeratable() const final { return true; }
bool supportsFocus() const final;
RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
Expand Down

0 comments on commit a6d354d

Please sign in to comment.