diff --git a/crates/js/src/formcalc.rs b/crates/js/src/formcalc.rs index 7a3de5a70..4ff77b74c 100644 --- a/crates/js/src/formcalc.rs +++ b/crates/js/src/formcalc.rs @@ -1617,19 +1617,13 @@ impl Interp<'_> { "presence" => { let p = self.to_str(&v); if matches!(p.as_str(), "visible" | "invisible" | "hidden" | "inactive") { - if let Some(n) = self.h.nodes.get_mut(i) { - n.presence = p.clone(); - } - self.h.push(XfaEffect::SetPresence { som, presence: p }); + self.h.set_state(i, XfaEffect::SetPresence { som, presence: p }); } } "access" => { let a = self.to_str(&v); if matches!(a.as_str(), "open" | "readOnly" | "protected" | "nonInteractive") { - if let Some(n) = self.h.nodes.get_mut(i) { - n.access = a.clone(); - } - self.h.push(XfaEffect::SetAccess { som, access: a }); + self.h.set_state(i, XfaEffect::SetAccess { som, access: a }); } } other => { diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 5e9512789..07f04614d 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -702,6 +702,27 @@ mod xfa_model { assert_eq!(o.result.as_deref(), Some("true:2")); } + /// Past the effect cap, presence and access set on a new object are left out of the snapshot + /// as well as the effects; one set again on an object that already has its effect still lands. + #[test] + fn presence_and_access_past_the_effect_cap_are_left_out_of_the_snapshot_too() { + let read = "qty.presence + ':' + qty.access + ':' + price.presence"; + let before = go(read, "form1[0].page1[0]", "click").result; + let o = go( + &format!( + "price.presence = 'invisible'; for (var i = 0; i < 20000; i++) xfa.host.beep(); qty.presence = 'hidden'; qty.access = 'readOnly'; price.presence = 'hidden'; {read}" + ), + "form1[0].page1[0]", + "click", + ); + assert_eq!(o.error, None, "{o:?}"); + assert_eq!(o.effects.len(), MAX_EFFECTS); + let unchanged = before.as_deref().and_then(|b| b.rsplit_once(':')).map(|(qty, _)| format!("{qty}:hidden")); + assert_eq!(o.result, unchanged); + assert!(o.effects.contains(&XfaEffect::SetPresence { som: "form1[0].page1[0].price[0]".into(), presence: "hidden".into() })); + assert!(!o.effects.iter().any(|e| matches!(e, XfaEffect::SetPresence { som, .. } | XfaEffect::SetAccess { som, .. } if som.contains("qty")))); + } + #[test] fn urls_and_reset_lists_are_clipped_and_deduplicated() { let o = go( @@ -803,6 +824,20 @@ mod formcalc { assert_eq!(names, 2); } + /// FormCalc presence and access past the effect cap are left out of the snapshot too. + #[test] + fn formcalc_presence_and_access_past_the_effect_cap_are_left_out_of_the_snapshot_too() { + let o = go( + "for i = 1 upto 20000 do $host.beep() endfor\nqty.presence = \"hidden\"\nqty.access = \"readOnly\"\nConcat(qty.presence, \":\", qty.access)", + "form1[0].page1[0]", + "click", + ); + assert_eq!(o.error, None, "{o:?}"); + assert_eq!(o.effects.len(), MAX_EFFECTS); + let before = go("Concat(qty.presence, \":\", qty.access)", "form1[0].page1[0]", "click"); + assert_eq!(o.result, before.result); + } + /// FormCalc reads `formattedValue` as the field's picture shows it too, until the script /// sets the value. #[test] diff --git a/crates/js/src/xfa.rs b/crates/js/src/xfa.rs index b8440454a..18013e001 100644 --- a/crates/js/src/xfa.rs +++ b/crates/js/src/xfa.rs @@ -199,6 +199,17 @@ pub(crate) fn reset_names(list: &str) -> Vec { out } +/// The (property, SOM) an effect setting a property of an object is merged by, see +/// [`XHost::set_at`]. +fn effect_key(e: &XfaEffect) -> Option<(u8, String)> { + match e { + XfaEffect::SetValue { som, .. } => Some((0, som.clone())), + XfaEffect::SetPresence { som, .. } => Some((1, som.clone())), + XfaEffect::SetAccess { som, .. } => Some((2, som.clone())), + _ => None, + } +} + /// Bytes of text `e` holds. fn payload(e: &XfaEffect) -> usize { match e { @@ -321,17 +332,35 @@ impl XHost { } } - /// Room for one more effect holding `bytes` of text, charged now: a change to the snapshot - /// is made only when the effect that applies it will be kept, so a script never sees a change - /// its effects leave out. A drop is counted when there is no room. - fn reserve(&mut self, bytes: usize) -> bool { - if self.effects.len() >= MAX_EFFECTS { + /// Room for `e`, which holds `bytes` of text, charged now: a change to the snapshot is made + /// only when the effect that applies it will be kept, so a script never sees a change its + /// effects leave out. An effect that replaces an earlier one needs no new slot. A drop is + /// counted when there is no room. + fn reserve(&mut self, e: &XfaEffect, bytes: usize) -> bool { + let replaces = effect_key(e).and_then(|k| self.set_at.get(&k)).is_some_and(|&at| at >= self.barrier && at < self.effects.len()); + if !replaces && self.effects.len() >= MAX_EFFECTS { self.dropped_effects += 1; return false; } self.room(bytes) } + /// Set the presence or access of node `i` to what `e` (a [`XfaEffect::SetPresence`] or + /// [`XfaEffect::SetAccess`]) says, in the snapshot only when the effect is kept. + pub(crate) fn set_state(&mut self, i: usize, e: XfaEffect) { + if !self.reserve(&e, payload(&e)) { + return; + } + if let Some(n) = self.nodes.get_mut(i) { + match &e { + XfaEffect::SetPresence { presence, .. } => n.presence = presence.clone(), + XfaEffect::SetAccess { access, .. } => n.access = access.clone(), + _ => {} + } + } + self.record(e); + } + /// `e` with its text cut to the lengths an effect keeps. fn clip_effect(&mut self, e: XfaEffect) -> XfaEffect { match e { @@ -352,13 +381,7 @@ impl XHost { /// Keep `e`, whose bytes are already charged, against the count limits. fn record(&mut self, e: XfaEffect) { - let key = match &e { - XfaEffect::SetValue { som, .. } => Some((0u8, som.clone())), - XfaEffect::SetPresence { som, .. } => Some((1, som.clone())), - XfaEffect::SetAccess { som, .. } => Some((2, som.clone())), - _ => None, - }; - if let Some(key) = key { + if let Some(key) = effect_key(&e) { if let Some(&at) = self.set_at.get(&key) && at >= self.barrier && let Some(slot) = self.effects.get_mut(at) @@ -773,7 +796,7 @@ impl XHost { let name = h.nodes.get(first).map(|n| n.name.clone()).unwrap_or_default(); let parent_som = h.nodes.get(parent).map(|p| p.som.clone()).unwrap_or_default(); let som = child_som(parent_som, &name, live.len()); - if !h.reserve(som.len()) { + if !h.reserve(&XfaEffect::AddInstance { som: String::new() }, som.len()) { return None; } let new = clone_subtree(h, first, Some(parent), 0)?; @@ -825,7 +848,7 @@ impl XHost { return; } let som = h.nodes.get(gone).map(|n| n.som.clone()).unwrap_or_default(); - if !h.reserve(som.len()) { + if !h.reserve(&XfaEffect::RemoveInstance { som: String::new() }, som.len()) { return; } if let Some(n) = h.nodes.get_mut(gone) { @@ -1061,20 +1084,14 @@ fn node_set(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult value.clone(), _ => return Ok(JsValue::from(true)), }; - if let Some(n) = h.nodes.get_mut(idx) { - n.presence = p.clone(); - } - h.push(XfaEffect::SetPresence { som, presence: p }); + h.set_state(idx, XfaEffect::SetPresence { som, presence: p }); } "access" => { let a = match value.as_str() { "open" | "readOnly" | "protected" | "nonInteractive" => value.clone(), _ => return Ok(JsValue::from(true)), }; - if let Some(n) = h.nodes.get_mut(idx) { - n.access = a.clone(); - } - h.push(XfaEffect::SetAccess { som, access: a }); + h.set_state(idx, XfaEffect::SetAccess { som, access: a }); } _ => {} }