Bug 1973497 - Refactor HTMButtonElement::ActivationBehavior r=dom-core,edgar

This refactoring aligns the button activation behaviour to the exact
steps specified in HTML, which has some small changes from the prior
implementation:

- button activation returns early after submitting, or resetting, not
  triggering popovers or command/commandfor.
- buttons with form owners and a type attribute in the Auto state must
  return early, not triggering popovers or command/commandfor.

This also has some other non-observable changes to the code:

- collapses the HandleCommandForAction method directly into
  ActivationBehavior.
- adds code comments for each of the spec steps

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

dom/html/HTMLButtonElement.cpp

@@ -267,34 +267,108 @@ void EndSubmitClick(EventChainVisitor& aVisitor) {
   }
 }
 
+// https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:activation-behaviour
 void HTMLButtonElement::ActivationBehavior(EventChainPostVisitor& aVisitor) {
   if (!aVisitor.mPresContext) {
     // Should check whether EndSubmitClick is needed here.
     return;
   }
 
-  if (!IsDisabled()) {
+  auto endSubmit = MakeScopeExit([&] { EndSubmitClick(aVisitor); });
-    if (mForm) {
+
-      // Hold a strong ref while dispatching
+  // 1. If element is disabled, then return.
-      RefPtr<mozilla::dom::HTMLFormElement> form(mForm);
+  if (IsDisabled()) {
-      if (mType == FormControlType::ButtonReset) {
+    return;
-        form->MaybeReset(this);
+  }
-        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+
-      } else if (mType == FormControlType::ButtonSubmit) {
+  // 2. If element's node document is not fully active, then return.
-        form->MaybeSubmit(this);
+
-        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+  // 3. If element has a form owner:
-      }
+  if (mForm) {
-      // https://html.spec.whatwg.org/multipage/form-elements.html#attr-button-type-button-state
+    // Hold a strong ref while dispatching
-      // NS_FORM_BUTTON_BUTTON do nothing.
+    RefPtr<mozilla::dom::HTMLFormElement> form(mForm);
+    // 3.1. If element is a submit button, then submit element's form owner from
+    // element with userInvolvement set to event's user navigation involvement,
+    // and return.
+    if (mType == FormControlType::ButtonSubmit) {
+      form->MaybeSubmit(this);
+      aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+      return;
     }
-    if (!GetCommandForElement()) {
+    // 3.2. If element's type attribute is in the Reset Button state, then reset
-      HandlePopoverTargetAction();
+    // element's form owner, and return.
-    } else {
+    if (mType == FormControlType::ButtonReset) {
-      HandleCommandForAction();
+      form->MaybeReset(this);
+      aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+      return;
+    }
+    // 3.3. If element's type attribute is in the Auto state, then return.
+    //
+    // (Auto state is only possible if the content attribute is not present)
+    if (!HasAttr(nsGkAtoms::type)) {
+      return;
     }
   }
 
-  EndSubmitClick(aVisitor);
+  // 4. Let target be the result of running element's get the
+  // commandfor-associated element.
+  RefPtr<Element> target = GetCommandForElement();
+
+  // 5. If target is not null:
+  if (target) {
+    // 5.1. Let command be element's command attribute.
+    const nsAttrValue* attr = GetParsedAttr(nsGkAtoms::command);
+    nsAtom* commandRaw = attr ? attr->GetAtomValue() : nsGkAtoms::_empty;
+    Command command = GetCommand(commandRaw);
+
+    // 5.2. If command is in the Unknown state, then return.
+    if (command == Command::Invalid) {
+      return;
+    }
+
+    // 5.3. Let isPopover be true if target's popover attribute is not in the No
+    // Popover state; otherwise false.
+    // 5.4. If isPopover is false and command is not in the Custom state:
+    // (Checking isPopover is handled as part of IsValidCommandAction)
+    // 5.4.1. Assert: target's namespace is the HTML namespace.
+    // 5.4.2. If this standard does not define is valid invoker command steps
+    // for target's local name, then return.
+    // 5.4.3. Otherwise, if the result of running target's corresponding is
+    // valid invoker command steps given command is false, then return.
+    if (command != Command::Custom && !target->IsValidCommandAction(command)) {
+      return;
+    }
+
+    // 5.5. Let continue be the result of firing an event named command at
+    // target, using CommandEvent, with its command attribute initialized to
+    // command, its source attribute initialized to element, and its cancelable
+    // and composed attributes initialized to true.
+    CommandEventInit init;
+    commandRaw->ToString(init.mCommand);
+    init.mSource = this;
+    init.mCancelable = true;
+    init.mComposed = true;
+    RefPtr<Event> event = CommandEvent::Constructor(this, u"command"_ns, init);
+    event->SetTrusted(true);
+    event->SetTarget(target);
+    EventDispatcher::DispatchDOMEvent(target, nullptr, event, nullptr, nullptr);
+
+    // 5.6. If continue is false, then return.
+    // 5.7. If target is not connected, then return.
+    // 5.8. If command is in the Custom state, then return.
+    if (event->DefaultPrevented() || !target->IsInComposedDoc() ||
+        command == Command::Custom) {
+      return;
+    }
+
+    // Steps 5.9...5.12. handled with HandleCommandInternal:
+    target->HandleCommandInternal(this, command, IgnoreErrors());
+
+  } else {
+    // 6. Otherwise, run the popover target attribute activation behavior given
+    // element and event's target.
+    HandlePopoverTargetAction();
+  }
 }
 
 void HTMLButtonElement::LegacyCanceledActivationBehavior(
@@ -439,49 +513,6 @@ void HTMLButtonElement::UpdateValidityElementStates(bool aNotify) {
   }
 }
 
-void HTMLButtonElement::HandleCommandForAction() {
-  RefPtr<Element> invokee = GetCommandForElement();
-
-  if (!invokee) {
-    return;
-  }
-
-  // 1. Let action be element's command attribute.
-  const nsAttrValue* attr = GetParsedAttr(nsGkAtoms::command);
-
-  nsAtom* actionRaw = attr ? attr->GetAtomValue() : nsGkAtoms::_empty;
-  Command action = GetCommand(actionRaw);
-
-  // 5.3. Otherwise, if the result of running invokee's corresponding is valid
-  // invoke action steps given action is not true, then return.
-  if (action != Command::Custom && !invokee->IsValidCommandAction(action)) {
-    return;
-  }
-
-  // 6. Let continue be the result of firing an event named invoke at invokee,
-  // using CommandEvent, with its action attribute initialized to action's
-  // value, its invoker attribute initialized to element, and its cancelable and
-  // composed attributes initialized to true.
-  CommandEventInit init;
-  actionRaw->ToString(init.mCommand);
-  init.mSource = this;
-  init.mCancelable = true;
-  init.mComposed = true;
-  RefPtr<Event> event = CommandEvent::Constructor(this, u"command"_ns, init);
-  event->SetTrusted(true);
-  event->SetTarget(invokee);
-
-  EventDispatcher::DispatchDOMEvent(invokee, nullptr, event, nullptr, nullptr);
-
-  // 7. If continue is false, then return.
-  // 8. If isCustom is true, then return.
-  if (action == Command::Custom || event->DefaultPrevented()) {
-    return;
-  }
-
-  invokee->HandleCommandInternal(this, action, IgnoreErrors());
-}
-
 void HTMLButtonElement::GetCommand(nsAString& aValue) const {
   const nsAttrValue* attr = GetParsedAttr(nsGkAtoms::command);
   if (attr) {

dom/html/HTMLButtonElement.h

@@ -136,7 +136,6 @@ class HTMLButtonElement final : public nsGenericHTMLFormControlElementWithState,
   void SetCustomValidity(const nsAString& aError);
 
   // Command & CommandFor
-  MOZ_CAN_RUN_SCRIPT void HandleCommandForAction();
   Element* GetCommandForElement() const;
   void SetCommandForElement(Element*);
   void GetCommand(nsAString& aValue) const;

testing/web-platform/meta/html/semantics/popovers/button-type-popovertarget.html.ini

@@ -1,6 +0,0 @@
-[button-type-popovertarget.html]
-  [Button type=reset in form should trigger form reset and not toggle popover]
-    expected: FAIL
-
-  [Button type=reset with form attr should trigger form reset and not toggle popover]
-    expected: FAIL

testing/web-platform/meta/html/semantics/the-button-element/command-and-commandfor/button-event-dispatch.html.ini

@@ -1,15 +1,3 @@
 [button-event-dispatch.html]
   [event dispatches on click with oncommand property]
     expected: FAIL
-
-  [event does NOT dispatch if button is form associated, with implicit type]
-    expected: FAIL
-
-  [event does NOT dispatch if button is form associated, with explicit type=invalid]
-    expected: FAIL
-
-  [event does NOT dispatch if button is form associated, with explicit type=submit]
-    expected: FAIL
-
-  [event does NOT dispatch if button is form associated, with explicit type=reset]
-    expected: FAIL

testing/web-platform/meta/html/semantics/the-button-element/command-and-commandfor/button-type-behavior.html.ini

@@ -34,15 +34,3 @@
 
   [Button missing type with form attr and only commandfor should not trigger form submit and not toggle popover]
     expected: FAIL
-
-  [Button type=reset in form should trigger form reset and not toggle popover]
-    expected: FAIL
-
-  [Button type=submit in form should trigger form submit and not toggle popover]
-    expected: FAIL
-
-  [Button type=reset with form attr should trigger form reset and not toggle popover]
-    expected: FAIL
-
-  [Button type=submit with form attr should trigger form submit and not toggle popover]
-    expected: FAIL