fix(html): align button command activation

moli-renderer-v8/src/native_bridge/element.rs

@@ -571,11 +571,13 @@ use pointer_capture::{
     node_has_pointer_capture_callback, node_release_pointer_capture_callback,
     node_set_pointer_capture_callback,
 };
+pub(super) use popover::{
+    dispatch_popover_hide_events, dispatch_popover_show_events, dispatch_popover_toggle_events,
+};
 pub(crate) use popover::{
     dispatch_popover_removal_events, handle_popover_attribute_change,
     perform_popover_invoker_default_action,
 };
-pub(super) use popover::{dispatch_popover_show_events, dispatch_popover_toggle_events};
 pub(super) use popover::{
     node_hide_popover_callback, node_popover_getter_function, node_popover_setter_function,
     node_show_popover_callback, node_toggle_popover_callback,

moli-renderer-v8/src/native_bridge/element/activation/default_action.rs

@@ -21,14 +21,19 @@ use crate::{
 };
 
 use super::super::super::JsContextHost;
-use super::super::forms::{FormAssociatedResetCallbackTiming, reset_form_default_action};
+use super::super::forms::{
+    FormAssociatedResetCallbackTiming, normalized_button_command, reset_form_default_action,
+};
 use super::super::{
     NodePublicEventDispatchOutcome, TextEditInputType, cache_input_files_from_selected_files,
-    closed_details_ancestors_to_reveal, construct_click_event_with_detail_and_modifiers, construct_command_event,
+    closed_details_ancestors_to_reveal, construct_click_event_with_detail_and_modifiers,
+    construct_command_event,
     construct_input_event, construct_simple_event, contenteditable_editing_host,
-    dispatch_beforeinput, dispatch_popover_toggle_events, dispatch_public_event, element_attribute,
-    element_has_attribute, form_associated_form_owner, is_disabled_form_control, is_focusable,
-    is_valid_submit_button, label_activation_control_handle,
+    dispatch_beforeinput, dispatch_popover_hide_events, dispatch_popover_show_events,
+    dispatch_popover_toggle_events,
+    dispatch_public_event, element_attribute, element_has_attribute, form_associated_form_owner,
+    is_disabled_form_control, is_focusable, is_valid_submit_button,
+    label_activation_control_handle,
     perform_popover_invoker_default_action, perform_summary_click_default_action,
     replace_text_control_selection, resolve_url_like_attribute,
     resolved_reflected_element_attribute_handle, scroll_node_into_view_at_start,
@@ -1543,8 +1548,7 @@ fn perform_click_default_action(
     {
         return None;
     }
-    dispatch_button_command_event_if_needed(scope, runtime_ptr, handle);
-    if dispatch_button_popover_toggle_events_if_needed(scope, runtime_ptr, handle) {
+    if perform_button_command_default_action(scope, runtime_ptr, handle) {
         return None;
     }
     if perform_popover_invoker_default_action(scope, runtime_ptr, handle) {
@@ -1615,59 +1619,70 @@ fn element_is_anchor_with_href(runtime: &JsContextHost, handle: DomHandle) -> bo
     )
 }
 
-fn dispatch_button_command_event_if_needed(
-    scope: &mut v8::PinScope<'_, '_>,
-    runtime_ptr: *mut JsContextHost,
-    handle: DomHandle,
-) {
-    let runtime = unsafe { &*runtime_ptr };
-    if !runtime.dom_host().is_html_element_named(handle, "button") {
-        return;
-    }
-    let Some(command) =
-        element_attribute(runtime, handle, "command").filter(|value| !value.is_empty())
-    else {
-        return;
-    };
-    let Some(target) = command_for_element_target(runtime_ptr, handle) else {
-        return;
-    };
-    let Some(source) = node_wrapper_from_handle(scope, handle) else {
-        return;
-    };
-    let Some(event) = construct_command_event(scope, &command, source.into()) else {
-        return;
-    };
-    if dispatch_public_event(scope, runtime_ptr, target, event).allows_default()
-        && command.eq_ignore_ascii_case("toggle-popover")
-    {
-        dispatch_popover_toggle_events(scope, runtime_ptr, target, handle);
-    }
-}
-
-fn dispatch_button_popover_toggle_events_if_needed(
+fn perform_button_command_default_action(
     scope: &mut v8::PinScope<'_, '_>,
     runtime_ptr: *mut JsContextHost,
     handle: DomHandle,
 ) -> bool {
-    let runtime = unsafe { &*runtime_ptr };
-    if !runtime.dom_host().is_html_element_named(handle, "button") {
-        return false;
-    }
-    let Some(target) = popover_target_element_target(runtime_ptr, handle) else {
+    let (command, target) = {
+        let runtime = unsafe { &*runtime_ptr };
+        if !runtime.dom_host().is_html_element_named(handle, "button") {
+            return false;
+        }
+        let command = normalized_button_command(element_attribute(runtime, handle, "command"));
+        if command.is_empty() {
+            return false;
+        }
+        let Some(target) = command_for_element_target(runtime_ptr, handle) else {
+            return false;
+        };
+        let Some(target_element) = runtime.dom_host().node(target).and_then(Node::as_element)
+        else {
+            return false;
+        };
+        let compatible = if command.starts_with("--") {
+            true
+        } else {
+            match command.as_str() {
+                "show-popover" | "hide-popover" | "toggle-popover" => {
+                    target_element.namespace() == "http://www.w3.org/1999/xhtml"
+                }
+                "show-modal" | "close" | "request-close" => {
+                    target_element.is_html_element("dialog")
+                }
+                _ => false,
+            }
+        };
+        if !compatible {
+            return false;
+        }
+        (command, target)
+    };
+    let Some(source) = node_wrapper_from_handle(scope, handle) else {
         return false;
     };
-    dispatch_popover_toggle_events(scope, runtime_ptr, target, handle);
+    let Some(event) = construct_command_event(scope, &command, source.into()) else {
+        return false;
+    };
+    let outcome = dispatch_public_event(scope, runtime_ptr, target, event);
+    if !outcome.allows_default() {
+        return true;
+    }
+    let runtime = unsafe { &*runtime_ptr };
+    if !runtime.dom_host().is_connected(target)
+        || !element_has_attribute(runtime, target, "popover")
+    {
+        return true;
+    }
+    match command.as_str() {
+        "show-popover" => dispatch_popover_show_events(scope, runtime_ptr, target, handle),
+        "hide-popover" => dispatch_popover_hide_events(scope, runtime_ptr, target, handle),
+        "toggle-popover" => dispatch_popover_toggle_events(scope, runtime_ptr, target, handle),
+        _ => {}
+    }
     true
 }
 
-fn popover_target_element_target(
-    runtime_ptr: *mut JsContextHost,
-    handle: DomHandle,
-) -> Option<DomHandle> {
-    resolved_reflected_element_attribute_handle(unsafe { &*runtime_ptr }, handle, "popovertarget")
-}
-
 fn command_for_element_target(
     runtime_ptr: *mut JsContextHost,
     handle: DomHandle,

moli-renderer-v8/src/native_bridge/element/forms.rs

@@ -200,6 +200,7 @@ pub(in crate::native_bridge) use self::select::{
     select_size_getter_function, select_size_setter_function, select_value_getter_function,
     select_value_setter_function, set_select_indexed_option,
 };
+pub(in crate::native_bridge::element) use self::simple_controls::normalized_button_command;
 pub(in crate::native_bridge) use self::simple_controls::{
     button_command_for_element_getter_function, button_command_for_element_setter_function,
     button_command_getter_function, button_command_setter_function,

moli-renderer-v8/src/native_bridge/element/forms/simple_controls.rs

@@ -1244,7 +1244,9 @@ pub(in crate::native_bridge) fn button_command_for_element_getter_function<'s>(
     set_wrapped_button_element_or_null(scope, &mut rv, runtime_ptr, source, target);
 }
 
-fn normalized_button_command(value: Option<String>) -> String {
+pub(in crate::native_bridge::element) fn normalized_button_command(
+    value: Option<String>,
+) -> String {
     let Some(value) = value else {
         return String::new();
     };

moli-renderer-v8/src/native_bridge/element/popover.rs

@@ -143,6 +143,15 @@ pub(crate) fn dispatch_popover_show_events(
     let _ = set_popover_open_state(scope, runtime_ptr, target, true, Some(source_handle));
 }
 
+pub(crate) fn dispatch_popover_hide_events(
+    scope: &mut v8::PinScope<'_, '_>,
+    runtime_ptr: *mut JsContextHost,
+    target: DomHandle,
+    source_handle: DomHandle,
+) {
+    let _ = set_popover_open_state(scope, runtime_ptr, target, false, Some(source_handle));
+}
+
 pub(crate) fn dispatch_popover_toggle_events(
     scope: &mut v8::PinScope<'_, '_>,
     runtime_ptr: *mut JsContextHost,
@@ -617,26 +626,12 @@ pub(crate) fn perform_popover_invoker_default_action(
     invoker: DomHandle,
 ) -> bool {
     let runtime = unsafe { &*runtime_ptr };
-    let (target_id, action) =
-        if let Some(target_id) = element_attribute(runtime, invoker, "popovertarget") {
-            let action = element_attribute(runtime, invoker, "popovertargetaction")
-                .map(|value| value.to_ascii_lowercase())
-                .unwrap_or_else(|| "toggle".to_owned());
-            (target_id, action)
-        } else if let Some(target_id) = element_attribute(runtime, invoker, "commandfor") {
-            let action = match element_attribute(runtime, invoker, "command")
-                .map(|value| value.to_ascii_lowercase())
-                .as_deref()
-            {
-                Some("show-popover") => "show",
-                Some("hide-popover") => "hide",
-                Some("toggle-popover") => "toggle",
-                _ => return false,
-            };
-            (target_id, action.to_owned())
-        } else {
-            return false;
-        };
+    let Some(target_id) = element_attribute(runtime, invoker, "popovertarget") else {
+        return false;
+    };
+    let action = element_attribute(runtime, invoker, "popovertargetaction")
+        .map(|value| value.to_ascii_lowercase())
+        .unwrap_or_else(|| "toggle".to_owned());
     if target_id.is_empty() {
         return false;
     }

moli-renderer-v8/src/script_vm/tests/dom_xhr/forms/event_activation.rs

@@ -243,6 +243,131 @@ fn command_interfaces_apply_reflection_and_webidl_conversion() {
     );
 }
 
+#[test]
+fn button_commands_validate_targets_and_apply_popover_actions_once() {
+    let mut vm = new_storage_test_vm("https://button-command-activation.test/");
+
+    let result = vm
+        .eval(
+            r#"
+(() => {
+  const host = document.body || document.documentElement || document;
+  const container = document.createElement('div');
+  const popover = document.createElement('div');
+  popover.id = 'command-popover';
+  popover.setAttribute('popover', '');
+  const button = document.createElement('button');
+  button.type = 'button';
+  button.setAttribute('commandfor', popover.id);
+  container.append(popover, button);
+  host.appendChild(container);
+
+  const states = [];
+  button.command = 'toggle-popover';
+  button.click();
+  states.push(popover.matches(':popover-open'));
+  button.click();
+  states.push(popover.matches(':popover-open'));
+
+  button.command = 'show-popover';
+  button.click();
+  states.push(popover.matches(':popover-open'));
+  button.click();
+  states.push(popover.matches(':popover-open'));
+  popover.hidePopover();
+
+  popover.addEventListener('command', event => event.preventDefault(), { once: true });
+  button.click();
+  states.push(popover.matches(':popover-open'));
+
+  popover.addEventListener('command', () => {
+    button.command = 'hide-popover';
+  }, { once: true });
+  button.command = 'show-popover';
+  button.click();
+  states.push(popover.matches(':popover-open'));
+
+  popover.addEventListener('command', event => event.preventDefault(), { once: true });
+  button.command = 'hide-popover';
+  button.click();
+  states.push(popover.matches(':popover-open'));
+  button.click();
+  states.push(popover.matches(':popover-open'));
+
+  let invalidEvents = 0;
+  const recordInvalidEvent = () => invalidEvents++;
+  popover.addEventListener('command', recordInvalidEvent);
+  button.setAttribute('command', 'not-a-command');
+  button.click();
+  const invalidCommandIgnored = invalidEvents === 0;
+  popover.removeEventListener('command', recordInvalidEvent);
+
+  const plainTarget = document.createElement('div');
+  plainTarget.id = 'plain-command-target';
+  container.appendChild(plainTarget);
+  let plainEvent = null;
+  plainTarget.addEventListener('command', event => plainEvent = event);
+  button.setAttribute('commandfor', plainTarget.id);
+  button.command = 'show-popover';
+  button.click();
+
+  const svgTarget = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
+  svgTarget.id = 'svg-command-target';
+  container.appendChild(svgTarget);
+  const svgCommands = [];
+  svgTarget.addEventListener('command', event => svgCommands.push(event.command));
+  button.setAttribute('commandfor', svgTarget.id);
+  button.command = '--MiXeD';
+  button.click();
+  button.command = 'show-popover';
+  button.click();
+
+  button.setAttribute('commandfor', popover.id);
+  button.command = 'show-popover';
+  let disconnectedSafe = true;
+  popover.addEventListener('command', () => popover.remove(), { once: true });
+  try {
+    button.click();
+  } catch {
+    disconnectedSafe = false;
+  }
+
+  const targetActionPopover = document.createElement('div');
+  targetActionPopover.id = 'target-action-popover';
+  targetActionPopover.setAttribute('popover', '');
+  const targetActionButton = document.createElement('button');
+  targetActionButton.setAttribute('popovertarget', targetActionPopover.id);
+  targetActionButton.setAttribute('popovertargetaction', 'show');
+  container.append(targetActionPopover, targetActionButton);
+  targetActionButton.click();
+  const targetActionStates = [targetActionPopover.matches(':popover-open')];
+  targetActionButton.click();
+  targetActionStates.push(targetActionPopover.matches(':popover-open'));
+  targetActionButton.setAttribute('popovertargetaction', 'hide');
+  targetActionButton.click();
+  targetActionStates.push(targetActionPopover.matches(':popover-open'));
+
+  return JSON.stringify({
+    states,
+    invalidCommandIgnored,
+    plainEvent: [plainEvent instanceof CommandEvent, plainEvent.command,
+                 plainEvent.source === button],
+    svgCommands,
+    disconnectedSafe,
+    disconnectedOpen: popover.matches(':popover-open'),
+    targetActionStates,
+  });
+})()
+"#,
+        )
+        .expect("button command activation probe should evaluate");
+
+    assert_eq!(
+        result,
+        r#"{"states":[true,false,true,true,false,true,true,false],"invalidCommandIgnored":true,"plainEvent":[true,"show-popover",true],"svgCommands":["--MiXeD"],"disconnectedSafe":true,"disconnectedOpen":false,"targetActionStates":[true,true,false]}"#
+    );
+}
+
 #[test]
 fn button_auto_type_state_tracks_commands_form_owner_and_select_parent() {
     let mut vm = new_storage_test_vm("https://button-auto-type-state.test/");