diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 48237d77..c8fd7c5d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -443,3 +443,23 @@ jobs: bun run build - name: Drive the fixture through the real process boundary run: cargo test -p ps-blitz-script --features debug-control --test debug_control + # Activation behaviour, driven by ps-qa against this checkout. + # + # The host is built here rather than installed: `qa-inspect-host` depends + # on the published engine, so an installed one judges the last release and + # a change in this workspace could not fail it. `ps-qa` itself carries no + # engine, so the published binary drives whatever host it is given. + - uses: actions/checkout@v5 + with: + repository: pathscale/ps-observability + path: .ps-observability + # Both binaries from the same checkout, rather than `cargo install ps-qa`. + # The harness and the host move together, and installing one while + # building the other makes this job depend on a release of the harness as + # well as on the checkout, so a change to either has to be published + # before it can be used here. + - name: Drive the fixtures with ps-qa + env: + PS_OBSERVABILITY: .ps-observability + QA_BUILD_PS_QA: "1" + run: tests/qa/run.sh diff --git a/Cargo.toml b/Cargo.toml index 8b17e19c..f5f29cc2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ exclude = ["sites", "packages/blitz-wasm/guest"] resolver = "2" [workspace.package] -version = "0.4.2" +version = "0.4.3" license = "MIT OR Apache-2.0" homepage = "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/pathscale/ps-blitz" repository = "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/pathscale/ps-blitz" diff --git a/packages/blitz-dom/src/document.rs b/packages/blitz-dom/src/document.rs index 80ab47b7..5f731cb8 100644 --- a/packages/blitz-dom/src/document.rs +++ b/packages/blitz-dom/src/document.rs @@ -151,6 +151,14 @@ pub trait Document: Any + 'static { } } +/// What the pre-click activation steps changed, so a cancelled click can put +/// it back. Produced by [`BaseDocument::run_pre_click_activation`]. +#[derive(Debug, Clone, PartialEq)] +pub struct PreClickActivation { + /// Every node whose checkedness moved, paired with the value it held. + previous: Vec<(NodeId, bool)>, +} + pub struct PlainDocument(pub BaseDocument); impl Document for PlainDocument { fn inner(&self) -> DocGuard<'_> { @@ -756,6 +764,79 @@ impl BaseDocument { } } + /// The checkedness a click changed, kept so the click's *canceled + /// activation steps* can put it back when a listener calls + /// `preventDefault()`. + /// + /// A radio carries its whole set, because selecting one clears the others. + pub fn run_pre_click_activation(&mut self, target: NodeId) -> Option { + let node_id = crate::events::pointer::checkable_activation_target(self, target)?; + let el = self.get_node(node_id)?.data.downcast_element()?; + let is_radio = el.attr(local_name!("type")) == Some("radio"); + + if !is_radio { + let previous = el.checkbox_input_checked()?; + let el = self.get_node_mut(node_id)?.data.downcast_element_mut()?; + Self::toggle_checkbox(el); + return Some(PreClickActivation { + previous: vec![(node_id, previous)], + }); + } + + let radio_set = el.attr(local_name!("name")).map(str::to_string); + let Some(radio_set) = radio_set else { + let previous = el.checkbox_input_checked()?; + let el = self.get_node_mut(node_id)?.data.downcast_element_mut()?; + *el.checkbox_input_checked_mut()? = true; + return Some(PreClickActivation { + previous: vec![(node_id, previous)], + }); + }; + + // Recorded *while* selecting rather than before it. Selecting one radio + // clears every other in the set, so cancelling has to restore all of + // them, and reading them first meant walking the whole arena twice for + // one press. The membership test is `toggle_radio`'s, deliberately: two + // predicates that disagreed would restore a different set than the one + // that changed. + // + // That predicate is name plus "has checkbox state", which is neither + // scoped to `type=radio` nor to a form owner. Wrong per HTML, and + // longstanding; matching it here keeps this change to the ordering it + // is about. + let mut previous: Vec<(NodeId, bool)> = Vec::new(); + for (id, node) in self.nodes.iter_mut() { + let Some(el) = node.data.downcast_element_mut() else { + continue; + }; + if el.attr(local_name!("name")) != Some(&*radio_set) { + continue; + } + let Some(is_checked) = el.checkbox_input_checked_mut() else { + continue; + }; + previous.push((id, *is_checked)); + *is_checked = id == node_id; + } + Some(PreClickActivation { previous }) + } + + /// Undo [`Self::run_pre_click_activation`]. The click's *canceled + /// activation steps*. + pub fn undo_pre_click_activation(&mut self, activation: PreClickActivation) { + for (node_id, was_checked) in activation.previous { + let Some(node) = self.get_node_mut(node_id) else { + continue; + }; + let Some(el) = node.data.downcast_element_mut() else { + continue; + }; + if let Some(is_checked) = el.checkbox_input_checked_mut() { + *is_checked = was_checked; + } + } + } + pub fn toggle_checkbox(el: &mut ElementData) -> bool { let Some(is_checked) = el.checkbox_input_checked_mut() else { return false; diff --git a/packages/blitz-dom/src/events/driver.rs b/packages/blitz-dom/src/events/driver.rs index efb72354..da60368b 100644 --- a/packages/blitz-dom/src/events/driver.rs +++ b/packages/blitz-dom/src/events/driver.rs @@ -341,8 +341,26 @@ impl<'doc, Handler: EventHandler> EventDriver<'doc, Handler> { fn process_queue(&mut self) { while let Some(mut event) = self.queue.pop_front() { + // HTML runs a click's *pre-click activation steps* before the event + // is dispatched, so a checkbox or radio has already taken its new + // value by the time any listener runs. Doing it afterwards -- as + // the default action, with everything else -- hands `click` + // listeners the value from before the press, which reads as a + // control that responds to every second press. Cancelling the click + // runs the canceled activation steps and puts it back. + let activation = match event.data { + DomEventData::Click(_) => { + self.doc.inner_mut().run_pre_click_activation(event.target) + } + _ => None, + }; + let event_state = self.run_handler_event(&mut event, EventState::default()); - if !event_state.is_cancelled() { + if event_state.is_cancelled() { + if let Some(activation) = activation { + self.doc.inner_mut().undo_pre_click_activation(activation); + } + } else { self.run_default_action(&mut event); } } diff --git a/packages/blitz-dom/src/events/mod.rs b/packages/blitz-dom/src/events/mod.rs index 5ccdf235..a14b657b 100644 --- a/packages/blitz-dom/src/events/mod.rs +++ b/packages/blitz-dom/src/events/mod.rs @@ -2,7 +2,7 @@ mod driver; mod focus; mod ime; mod keyboard; -mod pointer; +pub(crate) mod pointer; use crate::util::Point; use blitz_traits::events::{DomEvent, DomEventData, PointerCoords, UiEvent}; diff --git a/packages/blitz-dom/src/events/pointer.rs b/packages/blitz-dom/src/events/pointer.rs index fa5f4104..48f726ae 100644 --- a/packages/blitz-dom/src/events/pointer.rs +++ b/packages/blitz-dom/src/events/pointer.rs @@ -597,6 +597,56 @@ pub(crate) fn handle_pointerup( } } +/// The checkbox or radio that a click at `target` activates, or `None` if the +/// click activates something else. +/// +/// This walks the tree the way [`handle_click`] does -- stopping at a disabled +/// element or a text input -- because the two have to agree. The pre-click +/// activation steps flip the checkedness of whatever this returns, and +/// `handle_click` reports it afterwards; if they disagreed, a control would +/// either toggle without an `input` event or fire one without having toggled. +/// +/// A `