Bug 1829874 - Allow events on disabled fieldsets r=edgar

The HTML spec does not allow fieldset to be "disabled as a form control". The spec does not explicitly define whether fieldset is a form control or not (and thus can be disabled as a child of another fieldset), but here we assume it does not as:

* It's not a "control", right?
* It would be confusing if it could not be disabled by its own but could be disabled as a child.

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

dom/base/test/chrome/file_bug816340.xhtml

@@ -16,12 +16,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=816340
       window.arguments[0].ok(val, msg);
     }
 
-    var elems = 
+    var elems =
       [
         "input",
         "textarea",
         "select",
-        "fieldset",
         "button",
       ];
 
@@ -49,7 +48,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=816340
       ok(contentDidGetEvent == contentShouldGetEvent, "content: " + el + (disabled ? " disabled" : ""));
       ok(chromeDidGetEvent, "chrome: " + el + (disabled ? " disabled" : ""));
     }
-      
+
     function start() {
       // Test common element.
       testElement("div", false, true);

dom/html/HTMLFieldSetElement.cpp

@@ -7,6 +7,7 @@
 #include "mozilla/BasicEvents.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/StaticPrefs_dom.h"
 #include "mozilla/dom/CustomElementRegistry.h"
 #include "mozilla/dom/HTMLFieldSetElement.h"
 #include "mozilla/dom/HTMLFieldSetElementBinding.h"
@@ -49,6 +50,9 @@ NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED(HTMLFieldSetElement,
 NS_IMPL_ELEMENT_CLONE(HTMLFieldSetElement)
 
 bool HTMLFieldSetElement::IsDisabledForEvents(WidgetEvent* aEvent) {
+  if (StaticPrefs::dom_forms_fieldset_disable_only_descendants_enabled()) {
+    return false;
+  }
   return IsElementDisabledForEvents(aEvent, nullptr);
 }
 

dom/html/nsGenericHTMLElement.cpp

@@ -4,8 +4,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "mozilla/ArrayUtils.h"
-#include "mozilla/DeclarationBlock.h"
 #include "mozilla/EditorBase.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventListenerManager.h"
@@ -15,29 +13,24 @@
 #include "mozilla/IMEStateManager.h"
 #include "mozilla/MappedDeclarations.h"
 #include "mozilla/Maybe.h"
-#include "mozilla/Likely.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/PresShell.h"
+#include "mozilla/StaticPrefs_dom.h"
 #include "mozilla/TextEditor.h"
 #include "mozilla/TextEvents.h"
 #include "mozilla/StaticPrefs_html5.h"
-#include "mozilla/StaticPrefs_layout.h"
 #include "mozilla/StaticPrefs_accessibility.h"
-
 #include "mozilla/dom/FormData.h"
 #include "nscore.h"
 #include "nsGenericHTMLElement.h"
-#include "nsAttrValueInlines.h"
 #include "nsCOMPtr.h"
 #include "nsAtom.h"
 #include "nsQueryObject.h"
-#include "nsIContentInlines.h"
 #include "mozilla/dom/BindContext.h"
 #include "mozilla/dom/Document.h"
 #include "nsMappedAttributes.h"
 #include "nsHTMLStyleSheet.h"
 #include "nsPIDOMWindow.h"
-#include "nsEscape.h"
 #include "nsIFrameInlines.h"
 #include "nsIScrollableFrame.h"
 #include "nsView.h"
@@ -45,7 +38,6 @@
 #include "nsIWidget.h"
 #include "nsRange.h"
 #include "nsPresContext.h"
-#include "nsNameSpaceManager.h"
 #include "nsError.h"
 #include "nsIPrincipal.h"
 #include "nsContainerFrame.h"
@@ -60,7 +52,6 @@
 #include "mozilla/dom/DirectionalityUtils.h"
 #include "mozilla/dom/DocumentOrShadowRoot.h"
 #include "nsString.h"
-#include "nsUnicharUtils.h"
 #include "nsGkAtoms.h"
 #include "nsDOMCSSDeclaration.h"
 #include "nsITextControlFrame.h"
@@ -68,15 +59,11 @@
 #include "mozilla/dom/HTMLFormElement.h"
 #include "nsFocusManager.h"
 
-#include "mozilla/InternalMutationEvent.h"
 #include "nsDOMStringMap.h"
 #include "nsDOMString.h"
 
 #include "nsLayoutUtils.h"
-#include "mozAutoDocUpdate.h"
-#include "nsHtml5Module.h"
 #include "mozilla/dom/DocumentInlines.h"
-#include "mozilla/dom/ElementInlines.h"
 #include "HTMLFieldSetElement.h"
 #include "nsTextNode.h"
 #include "HTMLBRElement.h"
@@ -84,20 +71,16 @@
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/FromParser.h"
 #include "mozilla/dom/Link.h"
-#include "mozilla/BloomFilter.h"
 #include "mozilla/dom/ScriptLoader.h"
 
-#include "nsVariant.h"
 #include "nsDOMTokenList.h"
 #include "nsThreadUtils.h"
-#include "nsTextFragment.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/MouseEventBinding.h"
 #include "mozilla/dom/ToggleEvent.h"
 #include "mozilla/dom/TouchEvent.h"
 #include "mozilla/ErrorResult.h"
 #include "nsHTMLDocument.h"
-#include "nsGlobalWindow.h"
 #include "mozilla/dom/HTMLBodyElement.h"
 #include "imgIContainer.h"
 #include "nsComputedDOMStyle.h"
@@ -2260,7 +2243,22 @@ void nsGenericHTMLFormElement::FieldSetDisabledChanged(bool aNotify) {
 //----------------------------------------------------------------------
 
 void nsGenericHTMLElement::Click(CallerType aCallerType) {
-  if (IsDisabled() || HandlingClick()) {
+  if (HandlingClick()) {
+    return;
+  }
+
+  // There are two notions of disabled.
+  // "disabled":
+  // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#attr-fe-disabled
+  // "actually disabled":
+  // https://html.spec.whatwg.org/multipage/semantics-other.html#concept-element-disabled
+  // click() reads the former but IsDisabled() is for the latter. <fieldset> is
+  // included only in the latter, so we exclude it here.
+  // XXX(krosylight): What about <optgroup>? And should we add a separate method
+  // for this?
+  if (IsDisabled() &&
+      !(mNodeInfo->Equals(nsGkAtoms::fieldset) &&
+        StaticPrefs::dom_forms_fieldset_disable_only_descendants_enabled())) {
     return;
   }
 

dom/html/test/test_bug557087-1.html

@@ -102,7 +102,7 @@ function checkClickEvent(aFieldset)
 
   sendMouseEvent({type:'click'}, aFieldset);
   SimpleTest.executeSoon(function() {
-    ok(!clickHandled, "When disabled, fieldset should prevent click events");
+    ok(clickHandled, "When disabled, fieldset should not prevent click events");
     SimpleTest.finish();
   });
 }

dom/html/test/test_bug557087-2.html

@@ -21,8 +21,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=557087
 
 SimpleTest.waitForExplicitFinish();
 
-var elementsPreventingClick = [ "input", "button", "select", "textarea", "fieldset" ];
-var elementsWithClick = [ "option", "optgroup", "output", "label", "object" ];
+var elementsPreventingClick = [ "input", "button", "select", "textarea" ];
+var elementsWithClick = [ "option", "optgroup", "output", "label", "object", "fieldset" ];
 var gHandled = 0;
 
 function clickShouldNotHappenHandler(aEvent)

modules/libpref/init/StaticPrefList.yaml

@@ -3018,6 +3018,13 @@
   value: @IS_NIGHTLY_BUILD@
   mirror: always
 
+# Whether to disable only the descendants or the parent fieldset element too
+# Note that this still allows it to be selected by `:disable`.
+- name: dom.forms.fieldset_disable_only_descendants.enabled
+  type: bool
+  value: @IS_NIGHTLY_BUILD@
+  mirror: always
+
 # Is support for HTMLInputElement.showPicker enabled?
 - name: dom.input.showPicker
   type: bool

testing/web-platform/meta/html/semantics/disabled-elements/fieldset-event-propagation.tentative.html.ini

@@ -1,6 +0,0 @@
-[fieldset-event-propagation.tentative.html]
-  [Disabled fieldset elements should not prevent click event propagation.]
-    expected: FAIL
-
-  [Disabled fieldset elements should not block click events.]
-    expected: FAIL

testing/web-platform/tests/html/semantics/disabled-elements/event-propagate-disabled.tentative.html

@@ -92,13 +92,28 @@
     return { target, observedEvents };
   }
 
+  /**
+   * @param {Element} element
+   * @returns {boolean}
+   */
+  function isFormControl(element) {
+    if (["button", "input", "select", "textarea"].includes(element.localName)) {
+      return true;
+    }
+    return element.constructor.formAssociated;
+  }
+
+  function isDisabledFormControl(element) {
+    return isFormControl(element) && element.disabled;
+  }
+
   /**
    * @param {Element} target
    * @param {*} observedEvent
    */
   function shouldNotBubble(target, observedEvent) {
     return (
-      target.disabled &&
+      isDisabledFormControl(target) &&
       observedEvent.isTrusted &&
       ["mousedown", "mouseup", "click"].includes(observedEvent.type)
     );
@@ -136,7 +151,7 @@
       await t.step_func(clickerFn)(target);
       await new Promise(resolve => t.step_timeout(resolve, 0));
 
-      const expected = element.disabled ? expectedEvents : nonDisabledExpectedEvents;
+      const expected = isDisabledFormControl(element) ? expectedEvents : nonDisabledExpectedEvents;
       assert_array_equals(observedEvents.map(e => e.type), expected, "Observed events");
 
       for (const observed of observedEvents) {

testing/web-platform/tests/html/semantics/disabled-elements/fieldset-event-propagation.tentative.html

@@ -15,34 +15,41 @@
 </div>
 
 <div id=target2parent>
-  <fieldset disabled id=target2fieldset>hello world</div>
+  <fieldset disabled id=target2fieldset>hello world</fieldset>
 </div>
 
 <script>
-promise_test(async () => {
-  let target1parentClicked = false;
-  let target1childClicked = false;
-  let target1fieldsetClicked = false;
-  target1parent.onclick = () => target1parentClicked = true;
-  target1child.onclick = () => target1childClicked = true;
-  target1fieldset.onclick = () => target1fieldsetClicked = true;
+  const clickers = {
+    "native click": target => test_driver.click(target),
+    "click()": target => target.click(),
+  };
 
-  await test_driver.click(target1child);
+  for (const [clickerName, clicker] of Object.entries(clickers)) {
+    promise_test(async () => {
+      let target1parentClicked = false;
+      let target1childClicked = false;
+      let target1fieldsetClicked = false;
+      target1parent.onclick = () => target1parentClicked = true;
+      target1child.onclick = () => target1childClicked = true;
+      target1fieldset.onclick = () => target1fieldsetClicked = true;
 
-  assert_true(target1parentClicked, 'The parent of the fieldset should receive a click event.');
-  assert_true(target1childClicked, 'The child of the fieldset should receive a click event.');
-  assert_true(target1fieldsetClicked, 'The fieldset element should receive a click event.');
-}, 'Disabled fieldset elements should not prevent click event propagation.');
+      await clicker(target1child);
 
-promise_test(async () => {
-  let target2parentClicked = false;
-  let target2fieldsetClicked = false;
-  target2parent.onclick = () => target2parentClicked = true;
-  target2fieldset.onclick = () => target2fieldsetClicked = true;
+      assert_true(target1parentClicked, 'The parent of the fieldset should receive a click event.');
+      assert_true(target1childClicked, 'The child of the fieldset should receive a click event.');
+      assert_true(target1fieldsetClicked, 'The fieldset element should receive a click event.');
+    }, `Disabled fieldset elements should not prevent click event propagation from ${clickerName}`);
 
-  await test_driver.click(target2fieldset);
+    promise_test(async () => {
+      let target2parentClicked = false;
+      let target2fieldsetClicked = false;
+      target2parent.onclick = () => target2parentClicked = true;
+      target2fieldset.onclick = () => target2fieldsetClicked = true;
 
-  assert_true(target2parentClicked, 'The parent of the fieldset should receive a click event.');
-  assert_true(target2fieldsetClicked, 'The fieldset element should receive a click event.');
-}, 'Disabled fieldset elements should not block click events.');
+      await clicker(target2fieldset);
+
+      assert_true(target2parentClicked, 'The parent of the fieldset should receive a click event.');
+      assert_true(target2fieldsetClicked, 'The fieldset element should receive a click event.');
+    }, `Disabled fieldset elements should not block click events from ${clickerName}.`);
+  }
 </script>