Re-design logic for commandfor form-owner combinations

In whatwg/html#9841 the behaviour for commandfor= as a form owner has subtly changed: - <button commandfor=> (implicit submit) that has a form owner now does nothing (so we will log a warning to the console telling the user that this is the case and they should add `type=button` to make it a command button). - <button type=reset command..> and <button type=submit command..> that have a form owner should do the behaviour explicitly laid out by their `type`, and ignore and `command`/`commandfor` attributes semantics. In this case we log a warning to the console telling the user that the command/commandfor are being ignored. To keep changes light here, only the DefaultEventHandler/CommandForElement logic has changed; a more significant refactor might be needed to adjust `type_` to ensure it does not resolve to `kSubmit` implicitly, but that should be handled more delicately, I believe. Bug: 382238696 Change-Id: I7d5583d34a2d615faeed80905816edb3f261d60d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6070260 Auto-Submit: Keith Cirkel <[email protected]> Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Keith Cirkel <[email protected]> Cr-Commit-Position: refs/heads/main@{#1393810}
web-platform-tests · Dec 9, 2024 · 3d3ea3a · 3d3ea3a
1 parent 9d05543
commit 3d3ea3a
Show file tree

Hide file tree

Showing 3 changed files with 209 additions and 7 deletions.
diff --git a/html/semantics/forms/the-button-element/button-click-resets-with-commandfor.tentative.html b/html/semantics/forms/the-button-element/button-click-resets-with-commandfor.tentative.html
@@ -0,0 +1,64 @@
+<!doctype html>
+<meta charset="utf-8" />
+<title>Clicking a button should submit the form</title>
+<meta name="author" title="Keith Cirkel" href="mailto:[email protected]" />
+<link
+  rel="help"
+  href="https://html.spec.whatwg.org/multipage/#attr-button-type-submit-state"
+/>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<form id="form">
+  <button id="button" type="reset"></button>
+</form>
+
+<script>
+  const form = document.getElementById("form");
+  const button = document.getElementById("button");
+
+  function resetState() {
+    button.removeAttribute("commandfor");
+    button.removeAttribute("command");
+    button.removeAttribute("disabled");
+    button.removeAttribute("form");
+    button.setAttribute("type", "reset");
+  }
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+
+    let called = false;
+    form.addEventListener("reset", (e) => {
+      called = true;
+    });
+    button.click();
+    assert_true(called, "reset should have been dispatched");
+  }, "clicking a reset button should trigger a reset (with command attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("commandfor", "whatever");
+
+    let called = false;
+    form.addEventListener("reset", (e) => {
+      called = true;
+    });
+    button.click();
+    assert_true(called, "reset should have been dispatched");
+  }, "clicking a button should trigger a reset (with commandfor attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+    button.setAttribute("commandfor", "whatever");
+
+    let called = false;
+    form.addEventListener("reset", (e) => {
+      called = true;
+    });
+    button.click();
+    assert_true(called, "reset should have been dispatched");
+  }, "clicking a button should trigger a reset (with command and commandfor attribute)");
+</script>
diff --git a/html/semantics/forms/the-button-element/button-click-submits-with-commandfor.tentative.html b/html/semantics/forms/the-button-element/button-click-submits-with-commandfor.tentative.html
@@ -0,0 +1,110 @@
+<!doctype html>
+<meta charset="utf-8" />
+<title>Clicking a button should submit the form</title>
+<meta name="author" title="Keith Cirkel" href="mailto:[email protected]" />
+<link
+  rel="help"
+  href="https://html.spec.whatwg.org/multipage/#attr-button-type-submit-state"
+/>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<form id="form">
+  <button id="button"></button>
+</form>
+
+<script>
+  const form = document.getElementById("form");
+  const button = document.getElementById("button");
+
+  function resetState() {
+    button.removeAttribute("commandfor");
+    button.removeAttribute("command");
+    button.removeAttribute("disabled");
+    button.removeAttribute("form");
+    button.removeAttribute("type");
+  }
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_false(called, "submit should not have been dispatched");
+  }, "clicking a button (implicit type) should NOT trigger a submit (with command attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("commandfor", "whatever");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_false(called, "submit should not have been dispatched");
+  }, "clicking a button (implicit type) should NOT trigger a submit (with commandfor attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+    button.setAttribute("commandfor", "whatever");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_false(called, "submit should not have been dispatched");
+  }, "clicking a button (implicit type) should NOT trigger a submit (with command and commandfor attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+    button.setAttribute("type", "submit");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_true(called, "submit should have been dispatched");
+  }, "clicking a button (explicit type=submit) SHOULD trigger a submit (with command attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("commandfor", "whatever");
+    button.setAttribute("type", "submit");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_true(called, "submit should have been dispatched");
+  }, "clicking a button (explicit type=submit) SHOULD trigger a submit (with commandfor attribute)");
+
+  test((t) => {
+    t.add_cleanup(resetState);
+    button.setAttribute("command", "--foo");
+    button.setAttribute("commandfor", "whatever");
+    button.setAttribute("type", "submit");
+
+    let called = false;
+    form.addEventListener("submit", (e) => {
+      e.preventDefault();
+      called = true;
+    }, { once: true });
+    button.click();
+    assert_true(called, "submit should have been dispatched");
+  }, "clicking a button (explicit type=submit) SHOULD trigger a submit (with command and commandfor attribute)");
+</script>
diff --git a/html/semantics/invokers/invoketarget-button-event-dispatch.tentative.html b/html/semantics/invokers/invoketarget-button-event-dispatch.tentative.html
@@ -37,7 +37,7 @@
     assert_equals(event.isTrusted, true, "isTrusted");
     assert_equals(event.command, "--custom-command", "command");
     assert_equals(event.target, invokee, "target");
-    assert_equals(event.source, invokerbutton, "invoker");
+    assert_equals(event.source, invokerbutton, "source");
   }, "event dispatches on click");
 
   // valid custom invokeactions
@@ -56,7 +56,7 @@
         assert_equals(event.isTrusted, true, "isTrusted");
         assert_equals(event.command, command, "command");
         assert_equals(event.target, invokee, "target");
-        assert_equals(event.source, invokerbutton, "invoker");
+        assert_equals(event.source, invokerbutton, "source");
       }, `setting custom command property to ${command} (must include dash) sets event command`);
 
       promise_test(async function (t) {
@@ -72,7 +72,7 @@
         assert_equals(event.isTrusted, true, "isTrusted");
         assert_equals(event.command, command, "command");
         assert_equals(event.target, invokee, "target");
-        assert_equals(event.source, invokerbutton, "invoker");
+        assert_equals(event.source, invokerbutton, "source");
       }, `setting custom command attribute to ${command} (must include dash) sets event command`);
     },
   );
@@ -139,19 +139,47 @@
     let called = false;
     invokee.addEventListener("command", (e) => (called = true), { once: true });
     invokerbutton.setAttribute("form", "aform");
+    invokerbutton.removeAttribute("type");
     await clickOn(invokerbutton);
     assert_false(called, "event was not called");
-  }, "event does not dispatch if invoker is form associated without `type`");
+  }, "event does NOT dispatch if button is form associated, with implicit type");
+
+  promise_test(async function (t) {
+    t.add_cleanup(resetState);
+    let event;
+    invokee.addEventListener("command", (e) => (event = e), { once: true });
+    invokerbutton.setAttribute("form", "aform");
+    invokerbutton.setAttribute("type", "button");
+    await clickOn(invokerbutton);
+    assert_true(event instanceof CommandEvent, "event is CommandEvent");
+    assert_equals(event.type, "command", "type");
+    assert_equals(event.bubbles, false, "bubbles");
+    assert_equals(event.composed, true, "composed");
+    assert_equals(event.isTrusted, true, "isTrusted");
+    assert_equals(event.command, "--custom-command", "command");
+    assert_equals(event.target, invokee, "target");
+    assert_equals(event.source, invokerbutton, "source");
+  }, "event dispatches if button is form associated, with explicit type=button");
 
   promise_test(async function (t) {
     t.add_cleanup(resetState);
     let called = false;
     invokee.addEventListener("command", (e) => (called = true), { once: true });
     invokerbutton.setAttribute("form", "aform");
-    invokerbutton.setAttribute("type", "button");
+    invokerbutton.setAttribute("type", "submit");
+    await clickOn(invokerbutton);
+    assert_false(called, "event was not called");
+  }, "event does NOT dispatch if button is form associated, with explicit type=submit");
+
+  promise_test(async function (t) {
+    t.add_cleanup(resetState);
+    let called = false;
+    invokee.addEventListener("command", (e) => (called = true), { once: true });
+    invokerbutton.setAttribute("form", "aform");
+    invokerbutton.setAttribute("type", "reset");
     await clickOn(invokerbutton);
-    assert_true(called, "event was not called");
-  }, "event dispatches if invoker is form associated with `type=button`");
+    assert_false(called, "event was not called");
+  }, "event does NOT dispatch if button is form associated, with explicit type=reset");
 
   promise_test(async function (t) {
     svgInvokee = document.createElementNS("http://www.w3.org/2000/svg", "svg");