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 <chromium@keithcirkel.co.uk>
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Keith Cirkel <chromium@keithcirkel.co.uk>
Cr-Commit-Position: refs/heads/main@{#1393810}

third_party/blink/renderer/core/html/forms/html_button_element.cc

@@ -141,7 +141,7 @@ Element* HTMLButtonElement::commandForElement() {
   }
 
   if (!IsInTreeScope() || IsDisabledFormControl() ||
-      (Form() && CanBeSuccessfulSubmitButton())) {
+      (Form() && FastHasAttribute(html_names::kTypeAttr) && type_ == kSubmit)) {
     return nullptr;
   }
 
@@ -248,24 +248,44 @@ CommandEventType HTMLButtonElement::GetCommandEventType() const {
 
 void HTMLButtonElement::DefaultEventHandler(Event& event) {
   if (event.type() == event_type_names::kDOMActivate) {
+    bool potentialCommand = (FastHasAttribute(html_names::kCommandforAttr) ||
+                             FastHasAttribute(html_names::kCommandAttr));
+    bool implicitSubmit =
+        type_ == kSubmit && !FastHasAttribute(html_names::kTypeAttr);
+
     if (!IsDisabledFormControl()) {
       if (Form() && type_ == kSubmit) {
+        if (implicitSubmit && potentialCommand) {
+          AddConsoleMessage(mojom::blink::ConsoleMessageSource::kOther,
+                            mojom::blink::ConsoleMessageLevel::kWarning,
+                            "Buttons associated with forms that include "
+                            "command or commandfor attributes are "
+                            "ambiguous, and require a type=button attribute. "
+                            "No action will be taken.");
+          return;
+        } else if (potentialCommand) {
+          DCHECK(FastHasAttribute(html_names::kTypeAttr));
+          AddConsoleMessage(
+              mojom::blink::ConsoleMessageSource::kOther,
+              mojom::blink::ConsoleMessageLevel::kWarning,
+              "Buttons with an explicit type=submit will always submit a form, "
+              "so command or commandfor attributes will be ignored.");
+        }
         Form()->PrepareForSubmission(&event, this);
         event.SetDefaultHandled();
         return;
       }
       if (Form() && type_ == kReset) {
+        if (potentialCommand) {
+          AddConsoleMessage(mojom::blink::ConsoleMessageSource::kOther,
+                            mojom::blink::ConsoleMessageLevel::kWarning,
+                            "Buttons with a type of reset will ignore the "
+                            "command or commandfor attributes.");
+        }
         Form()->reset();
         event.SetDefaultHandled();
         return;
       }
-      if (Form() && type_ != kButton && commandForElement()) {
-        AddConsoleMessage(
-            mojom::blink::ConsoleMessageSource::kOther,
-            mojom::blink::ConsoleMessageLevel::kWarning,
-            "commandfor is ignored on form buttons without type=button.");
-        return;
-      }
     }
 
     // Buttons with a commandfor will dispatch a CommandEvent on the

third_party/blink/web_tests/external/wpt/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:wpt@keithcirkel.co.uk" />
+<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>

third_party/blink/web_tests/external/wpt/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:wpt@keithcirkel.co.uk" />
+<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>

third_party/blink/web_tests/external/wpt/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");