Bug 2068440 - Validate fieldset listed elements. r=smaug,dom-core-reviewers

Make sure we have a fieldset. We can't check _which_ fieldset because
fieldset.elements work across multiple fieldsets.

Track elements inside legends and check the disabled-ness more
explicitly instead.

Differential Revision: https://phabricator.services.mozilla.com/D322883

dom/html/HTMLElement.cpp

@@ -293,8 +293,9 @@ void HTMLElement::AfterClearForm(bool aUnbindOrDelete) {
 
 void HTMLElement::UpdateFormOwner() {
   MOZ_ASSERT(IsFormAssociatedElement());
-  DebugOnly<CustomElementData*> data = GetCustomElementData();
-  MOZ_ASSERT(data && data->mState == CustomElementData::State::eCustom);
+  MOZ_ASSERT(GetCustomElementData());
+  MOZ_ASSERT(GetCustomElementData()->mState ==
+             CustomElementData::State::eCustom);
 
   // If @form is set, the element *has* to be in a composed document,
   // otherwise it wouldn't be possible to find an element with the
@@ -415,8 +416,7 @@ void HTMLElement::UpdateFormOwner(bool aBindToTree, Element* aFormIdElement) {
 }
 
 bool HTMLElement::IsFormAssociatedElement() const {
-  CustomElementData* data = GetCustomElementData();
-  return data && data->IsFormAssociated();
+  return IsFormAssociatedCustomElement();
 }
 
 void HTMLElement::FieldSetDisabledChanged(bool aNotify) {

dom/html/HTMLFieldSetElement.cpp

@@ -88,7 +88,8 @@ void HTMLFieldSetElement::GetType(nsAString& aType) const {
 bool HTMLFieldSetElement::MatchListedElements(Element* aElement,
                                               int32_t aNamespaceID,
                                               nsAtom* aAtom, void* aData) {
-  return nsIFormControl::FromNodeOrNull(aElement) != nullptr;
+  auto* control = nsIFormControl::FromNodeOrNull(aElement);
+  return control && !!control->GetFieldSet();
 }
 
 HTMLCollection* HTMLFieldSetElement::Elements() {
@@ -185,9 +186,13 @@ void HTMLFieldSetElement::AddElement(nsGenericHTMLFormElement* aElement) {
   // If the element is a form-associated custom element, adding element might be
   // caused by FACE upgrade which won't trigger mutation observer, so mark
   // mElements dirty manually here.
-  CustomElementData* data = aElement->GetCustomElementData();
-  if (data && data->IsFormAssociated() && mElements) {
-    mElements->SetDirty();
+  if (CustomElementData* data = aElement->GetCustomElementData();
+      data && data->IsFormAssociated()) {
+    for (auto* fs = this; fs; fs = fs->GetFieldSet()) {
+      if (fs->mElements) {
+        fs->mElements->SetDirty();
+      }
+    }
   }
 
   // We need to update the validity of the fieldset.

dom/html/nsGenericHTMLElement.cpp

@@ -1945,10 +1945,9 @@ nsresult nsGenericHTMLFormElement::BindToTree(BindContext& aContext,
     if (HasAttr(nsGkAtoms::form) ? IsInComposedDoc() : aParent.IsContent()) {
       UpdateFormOwner(true, nullptr);
     }
+    // Set parent fieldset which should be used for the disabled state.
+    UpdateFieldSet(false);
   }
-
-  // Set parent fieldset which should be used for the disabled state.
-  UpdateFieldSet(false);
   return NS_OK;
 }
 
@@ -1956,7 +1955,8 @@ void nsGenericHTMLFormElement::UnbindFromTree(UnbindContext& aContext) {
   // Save state before doing anything else.
   SaveState();
 
-  if (IsFormAssociatedElement()) {
+  const bool formAssociated = IsFormAssociatedElement();
+  if (formAssociated) {
     if (HTMLFormElement* form = GetFormInternal()) {
       // Might need to unset form
       if (aContext.IsUnbindRoot(this)) {
@@ -1982,8 +1982,10 @@ void nsGenericHTMLFormElement::UnbindFromTree(UnbindContext& aContext) {
 
   nsGenericHTMLElement::UnbindFromTree(aContext);
 
-  // The element might not have a fieldset anymore.
-  UpdateFieldSet(false);
+  if (formAssociated) {
+    // The element might not have a fieldset anymore.
+    UpdateFieldSet(false);
+  }
 }
 
 void nsGenericHTMLFormElement::BeforeSetAttr(int32_t aNameSpaceID,
@@ -2259,43 +2261,47 @@ void nsGenericHTMLFormElement::UpdateFormOwner(bool aBindToTree,
 }
 
 void nsGenericHTMLFormElement::UpdateFieldSet(bool aNotify) {
-  if (IsInNativeAnonymousSubtree() || !IsFormAssociatedElement()) {
-    MOZ_ASSERT_IF(IsFormAssociatedElement(), !GetFieldSetInternal());
+  MOZ_ASSERT(IsFormAssociatedElement());
+  if (IsInNativeAnonymousSubtree()) {
+    MOZ_ASSERT(!GetFieldSetInternal());
     return;
   }
+  if (IsFormAssociatedCustomElement() &&
+      GetCustomElementData()->mState != CustomElementData::State::eCustom) {
+    MOZ_ASSERT(!GetFieldSetInternal());
+    return;
+  }
+  auto* oldFieldSet = GetFieldSetInternal();
+  auto* newFieldSet = FirstAncestorOfType<HTMLFieldSetElement>();
+  if (newFieldSet == oldFieldSet) {
+    // We already have the right fieldset;
+    return;
+  }
+  if (oldFieldSet) {
+    oldFieldSet->RemoveElement(this);
+  }
+  SetFieldSetInternal(newFieldSet);
+  if (newFieldSet) {
+    newFieldSet->AddElement(this);
+  }
+  // The disabled state may have changed
+  FieldSetDisabledChanged(aNotify);
+}
 
-  nsIContent* parent = nullptr;
-  nsIContent* prev = nullptr;
-  HTMLFieldSetElement* fieldset = GetFieldSetInternal();
-
-  for (parent = GetParent(); parent;
-       prev = parent, parent = parent->GetParent()) {
-    HTMLFieldSetElement* parentFieldset = HTMLFieldSetElement::FromNode(parent);
-    if (parentFieldset && (!prev || parentFieldset->GetFirstLegend() != prev)) {
-      if (fieldset == parentFieldset) {
-        // We already have the right fieldset;
-        return;
-      }
-
-      if (fieldset) {
-        fieldset->RemoveElement(this);
-      }
-      SetFieldSetInternal(parentFieldset);
-      parentFieldset->AddElement(this);
-
-      // The disabled state may have changed
-      FieldSetDisabledChanged(aNotify);
-      return;
+// https://html.spec.whatwg.org/#concept-fe-disabled
+bool nsGenericHTMLFormElement::IsDisabledByAncestorFieldSet() const {
+  for (auto* fieldset = GetFieldSetInternal(); fieldset;
+       fieldset = fieldset->GetFieldSet()) {
+    if (!fieldset->IsDisabled()) {
+      continue;
     }
+    const nsIContent* legend = fieldset->GetFirstLegend();
+    if (legend && IsInclusiveDescendantOf(legend)) {
+      continue;
+    }
+    return true;
   }
-
-  // No fieldset found.
-  if (fieldset) {
-    fieldset->RemoveElement(this);
-    SetFieldSetInternal(nullptr);
-    // The disabled state may have changed
-    FieldSetDisabledChanged(aNotify);
-  }
+  return false;
 }
 
 void nsGenericHTMLFormElement::UpdateDisabledState(bool aNotify) {
@@ -2303,9 +2309,8 @@ void nsGenericHTMLFormElement::UpdateDisabledState(bool aNotify) {
     return;
   }
 
-  HTMLFieldSetElement* fieldset = GetFieldSetInternal();
   const bool isDisabled =
-      HasAttr(nsGkAtoms::disabled) || (fieldset && fieldset->IsDisabled());
+      HasAttr(nsGkAtoms::disabled) || IsDisabledByAncestorFieldSet();
 
   const ElementState disabledStates =
       isDisabled ? ElementState::DISABLED : ElementState::ENABLED;

dom/html/nsGenericHTMLElement.h

@@ -1064,7 +1064,9 @@ class nsGenericHTMLFormElement : public nsGenericHTMLElement {
    */
   virtual void FieldSetDisabledChanged(bool aNotify);
 
-  void FieldSetFirstLegendChanged(bool aNotify) { UpdateFieldSet(aNotify); }
+  void FieldSetFirstLegendChanged(bool aNotify) {
+    FieldSetDisabledChanged(aNotify);
+  }
 
   /**
    * This callback is called by a fieldset on all it's elements when it's being
@@ -1111,6 +1113,7 @@ class nsGenericHTMLFormElement : public nsGenericHTMLElement {
    * state to decide whether our disabled flag should be toggled.
    */
   virtual void UpdateDisabledState(bool aNotify);
+  bool IsDisabledByAncestorFieldSet() const;
   bool IsReadOnlyInternal() const final;
 
   virtual void SetFormInternal(mozilla::dom::HTMLFormElement* aForm,

testing/web-platform/tests/custom-elements/form-associated/fieldset-elements-after-upgrade.html

@@ -0,0 +1,24 @@
+<!doctype html>
+<title>fieldset.elements is updated when a form-associated custom element upgrades</title>
+<link rel="help" href="https://html.spec.whatwg.org/#dom-fieldset-elements">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<fieldset id="outer">
+  <fieldset id="inner">
+    <legend><x-bar></x-bar></legend>
+    <x-bar></x-bar>
+  </fieldset>
+</fieldset>
+<script>
+test(() => {
+  const outer = document.getElementById("outer");
+  const inner = document.getElementById("inner");
+  assert_equals(outer.elements.length, 1, "outer before upgrade");
+  assert_equals(inner.elements.length, 0, "inner before upgrade");
+  customElements.define("x-bar", class extends HTMLElement {
+    static formAssociated = true;
+  });
+  assert_equals(outer.elements.length, 3, "outer after upgrade");
+  assert_equals(inner.elements.length, 2, "inner after upgrade");
+}, "Upgrading form-associated custom elements updates all ancestor fieldsets, including inside legend");
+</script>

testing/web-platform/tests/custom-elements/form-associated/fieldset-elements-during-construction.html

@@ -0,0 +1,28 @@
+<!doctype html>
+<title>Form associated custom element isn't a fieldset listed element until fully custom</title>
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2068440">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<fieldset id="fs"><x-foo></x-foo></fieldset>
+<script>
+async_test(function(t) {
+  const fieldset = document.querySelector("fieldset");
+  customElements.define('x-foo', class extends HTMLElement {
+    static formAssociated = true;
+    constructor() {
+      super();
+      t.step(() => {
+        assert_equals(fieldset.elements.length, 0, "Length in pre-customized state");
+      });
+      this.remove();
+      fieldset.appendChild(this);
+      t.step(() => {
+        assert_equals(fieldset.elements.length, 0, "Length in pre-customized state after re-bind");
+      });
+      t.step_timeout(t.step_func_done(() => {
+        assert_equals(fieldset.elements.length, 1, "Length in post-customized state");
+      }), 0);
+    }
+  });
+});
+</script>

testing/web-platform/tests/custom-elements/form-associated/form-disabled-legend.html

@@ -0,0 +1,91 @@
+<!DOCTYPE html>
+<title>Form-associated custom elements inside a legend of a disabled fieldset</title>
+<link rel="help" href="https://html.spec.whatwg.org/#concept-fe-disabled">
+<body>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+class MyControl extends HTMLElement {
+  static get formAssociated() { return true; }
+
+  constructor() {
+    super();
+    this.internals_ = this.attachInternals();
+    this.disabledHistory_ = [];
+  }
+
+  formDisabledCallback(isDisabled) {
+    this.disabledHistory_.push(isDisabled);
+  }
+  disabledHistory() {
+    return this.disabledHistory_;
+  }
+}
+customElements.define('my-control', MyControl);
+
+function setup(html) {
+  const container = document.createElement('div');
+  container.innerHTML = html;
+  document.body.appendChild(container);
+  return container;
+}
+
+test(() => {
+  const container = setup('<fieldset disabled><legend><my-control></my-control></legend><my-control></my-control></fieldset>');
+  const [inLegend, outsideLegend] = container.querySelectorAll('my-control');
+  assert_true(inLegend.matches(':enabled'), 'in legend :enabled');
+  assert_false(inLegend.matches(':disabled'), 'in legend :disabled');
+  assert_array_equals(inLegend.disabledHistory(), []);
+
+  assert_false(outsideLegend.matches(':enabled'), 'outside legend :enabled');
+  assert_true(outsideLegend.matches(':disabled'), 'outside legend :disabled');
+  assert_array_equals(outsideLegend.disabledHistory(), [true]);
+}, 'Custom element inside the first legend of a disabled fieldset is not disabled');
+
+test(() => {
+  const container = setup('<fieldset disabled><legend></legend><legend><my-control></my-control></legend></fieldset>');
+  const control = container.querySelector('my-control');
+  assert_true(control.matches(':disabled'));
+  assert_array_equals(control.disabledHistory(), [true]);
+}, 'Custom element inside a legend other than the first one is disabled');
+
+test(() => {
+  const container = setup('<fieldset disabled><fieldset><legend><my-control></my-control></legend></fieldset></fieldset>');
+  const control = container.querySelector('my-control');
+  assert_true(control.matches(':disabled'));
+  assert_array_equals(control.disabledHistory(), [true]);
+}, 'Legend of an inner fieldset does not shield from an outer disabled fieldset');
+
+test(() => {
+  const container = setup('<fieldset disabled><legend><my-control></my-control></legend></fieldset>');
+  const fieldset = container.firstElementChild;
+  const control = container.querySelector('my-control');
+
+  fieldset.disabled = false;
+  fieldset.disabled = true;
+  assert_true(control.matches(':enabled'));
+  assert_array_equals(control.disabledHistory(), []);
+}, 'Toggling disabled on the fieldset does not affect a custom element inside its first legend');
+
+test(() => {
+  const container = setup('<fieldset disabled><legend><my-control></my-control></legend></fieldset>');
+  const fieldset = container.firstElementChild;
+  const legend = fieldset.firstElementChild;
+  const control = container.querySelector('my-control');
+
+  const newLegend = document.createElement('legend');
+  fieldset.prepend(newLegend);
+  assert_true(control.matches(':disabled'), 'after inserting a new first legend');
+  assert_array_equals(control.disabledHistory(), [true]);
+
+  newLegend.remove();
+  assert_true(control.matches(':enabled'), 'after removing the new first legend');
+  assert_array_equals(control.disabledHistory(), [true, false]);
+}, 'Changing the first legend updates the disabled state of a custom element');
+
+test(() => {
+  const container = setup('<fieldset><legend><my-control></my-control></legend><my-control></my-control></fieldset>');
+  const fieldset = container.firstElementChild;
+  assert_equals(fieldset.elements.length, 2);
+}, 'fieldset.elements includes custom elements inside the legend');
+</script>

testing/web-platform/tests/html/semantics/forms/the-fieldset-element/disabled-nested-legend.html

@@ -0,0 +1,34 @@
+<!doctype html>
+<title>Disabled fieldset with nested fieldsets and legends</title>
+<link rel="help" href="https://html.spec.whatwg.org/#concept-fe-disabled">
+<link rel="help" href="https://html.spec.whatwg.org/#dom-fieldset-elements">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<fieldset disabled id="a"><fieldset id="b"><legend><input id="i1"></legend><input id="i2"></fieldset></fieldset>
+<fieldset disabled id="c"><legend><fieldset id="d"><input id="i3"></fieldset></legend><legend><input id="i4"></legend></fieldset>
+<fieldset disabled id="e"><legend><input id="i5"></legend></fieldset>
+<script>
+const $ = id => document.getElementById(id);
+const disabled = id => $(id).matches(":disabled");
+test(() => {
+  assert_true(disabled("b"), "b");
+  assert_true(disabled("i1"), "i1: inside inner legend, but outer fieldset is disabled");
+  assert_true(disabled("i2"), "i2");
+}, "Inner fieldset's legend doesn't shield from an outer disabled fieldset");
+test(() => {
+  assert_false(disabled("d"), "d");
+  assert_false(disabled("i3"), "i3: inside first legend of disabled fieldset");
+  assert_true(disabled("i4"), "i4: inside second legend");
+  assert_false(disabled("i5"), "i5");
+}, "Descendants of the first legend aren't disabled");
+test(() => {
+  assert_equals($("e").elements.length, 1, "e.elements includes controls in the legend");
+  assert_equals($("a").elements.length, 3, "a.elements includes nested controls");
+}, "fieldset.elements includes controls inside legend");
+test(() => {
+  $("c").prepend(document.createElement("legend"));
+  assert_true(disabled("d"), "d after legend change");
+  assert_true(disabled("i3"), "i3 after legend change");
+  assert_true(disabled("i4"), "i4 after legend change");
+}, "Changing the first legend updates the disabled state");
+</script>