diff --git a/crates/forms/README.md b/crates/forms/README.md index 6126dc1cbd..67ca3b3bfb 100644 --- a/crates/forms/README.md +++ b/crates/forms/README.md @@ -75,6 +75,12 @@ let cells: Vec = grid.cells_for(names)?; // "Table1_1", "Table original entry is preserved. Positive limits still apply. - Values are validated (options, `MaxLen`, single vs multi-select, NoToggleToOff, read-only) and a failed call changes nothing. +- Native AF recognition stops at the first genuine matching function call. If its arguments + are unsupported, the complete action remains a custom script for the caller's `Scripts` + interpreter; a later literal call must not replace the original conditional validation. + Async for-of loop headers and bodies do not expose AF-looking text inside regex literals. +- Moving choice fields preserves their own options rather than copying an ancestor's `/Opt`; + inherited check-box and radio-button export mappings remain intact. - `FieldProps::appearance` applies a `LookPatch` independently to each widget. `None` keeps a property; `Some(None)` removes a border/fill colour. Unchanged `/DA`, custom font resources, widget-specific `/DA` overrides, indirect `/MK` and `/BS` contents, dash arrays and unknown diff --git a/crates/forms/src/af/calc.rs b/crates/forms/src/af/calc.rs index 5c8098dfbb..1c3724ef4a 100644 --- a/crates/forms/src/af/calc.rs +++ b/crates/forms/src/af/calc.rs @@ -19,13 +19,21 @@ pub(super) enum Js { type Token = (Range, Js); /// Words after which `/` starts a regular expression instead of dividing. -const BEFORE_REGEX: [&str; 13] = ["return", "typeof", "case", "do", "else", "in", "of", "new", "delete", "void", "throw", "instanceof", "yield"]; +const BEFORE_REGEX: [&str; 12] = ["return", "typeof", "case", "do", "else", "in", "new", "delete", "void", "throw", "instanceof", "yield"]; /// Words whose condition, in parentheses, comes before a statement, so a `/` after it starts a /// regular expression. The `while` that ends a `do` loop counts too: the loop ends at its `)`, so /// a `/` after it starts the next statement, and can't divide. const BEFORE_CONDITION: [&str; 4] = ["if", "while", "for", "with"]; +/// Context of an open delimiter. Only the left side of a for header can introduce `of`. +#[derive(Clone, Copy)] +enum Delimiter { + Group, + Condition, + For { left: bool }, +} + /// Whether a line ends at `s[j]`: JavaScript ends one at `\n`, `\r`, U+2028 and U+2029. fn line_ends_at(s: &[u8], j: usize) -> bool { matches!(s.get(j..), Some([b'\n' | b'\r', ..] | [0xE2, 0x80, 0xA8 | 0xA9, ..])) @@ -46,10 +54,11 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { // Where a regular expression that never ended stopped being read: every `/` before it // divides, so a line of them is read once, not once for each. let mut divides_until = 0; - // For each `(` still open, whether it opened the condition of `if`, `while`, `for` or - // `with`; and whether the last token is the `)` that closed one. - let mut conditions: Vec = Vec::new(); + // Nested groups hide the surrounding for header: an `of` in a destructuring default + // or nested expression is an identifier, not the for-of separator. + let mut delimiters: Vec<(char, Delimiter)> = Vec::new(); let mut after_condition = false; + let mut after_for_of = false; 'scan: while let Some(&c) = s.get(i) { let start = i; let token = match c { @@ -90,7 +99,12 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { loop { match s.get(j) { None => break 'scan, - Some(b'\\') => j += 2, + Some(b'\\') => { + let Some(step) = js.get(j + 1..).and_then(|r| r.chars().next()).map(char::len_utf8) else { + break 'scan; + }; + j += 1 + step; + } Some(b'`') => break, Some(_) => j += 1, } @@ -98,7 +112,7 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { i = j + 1; Js::Other } - b'/' if i >= divides_until && regex_may_follow(js, &out, after_condition) => { + b'/' if i >= divides_until && regex_may_follow(js, &out, after_condition, after_for_of) => { // A regular expression ends at a `/` outside a class; a line break first means it // was a division after all. let (mut j, mut class) = (i + 1, false); @@ -106,7 +120,14 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { match s.get(j) { None => break None, Some(_) if line_ends_at(s, j) => break None, - Some(b'\\') => j += 2, + Some(b'\\') => { + if s.get(j + 1).is_none() || line_ends_at(s, j + 1) { + j += 1; + break None; + } + let step = js.get(j + 1..).and_then(|r| r.chars().next()).map_or(1, char::len_utf8); + j += 1 + step; + } Some(b'/') if !class => break Some(j), Some(b) => { class = match b { @@ -133,6 +154,16 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { } } } + b'+' | b'-' + if s.get(i + 1) == Some(&c) + && !regex_may_follow(js, &out, after_condition, after_for_of) + && out.last().is_some_and(|(range, _)| !(range.end..i).any(|j| line_ends_at(s, j))) => + { + // Postfix update ends an expression: a following slash divides. A line + // terminator forbids postfix syntax, including one inside a comment. + i += 2; + Js::Other + } _ if c.is_ascii_digit() => { while word(i) || s.get(i) == Some(&b'.') { i += 1; @@ -150,38 +181,88 @@ pub(super) fn js_tokens(js: &str) -> (Vec, Vec>) { Js::Punct(char::from(c)) } }; - after_condition = match token { - Js::Punct('(') => { - conditions.push(opens_condition(js, &out)); - false - } - Js::Punct(')') => conditions.pop().unwrap_or(false), - _ => false, - }; - out.push((start..i, token)); + let token = (start..i, token); + (after_condition, after_for_of) = advance_context(js, &token, &out, &mut delimiters); + out.push(token); } (out, comments) } +/// Advance only the delimiter/for-header context after a token has been classified. +/// Returns whether it closes a statement's condition or is the for-of separator. +fn advance_context(js: &str, token: &Token, tokens: &[Token], delimiters: &mut Vec<(char, Delimiter)>) -> (bool, bool) { + let mut after_for_of = false; + if let Some((_, Delimiter::For { left })) = delimiters.last_mut() { + if matches!(token.1, Js::Ident) && js.get(token.0.clone()) == Some("of") && *left && for_target_ends(js, tokens) { + *left = false; + after_for_of = true; + } else if matches!(token.1, Js::Punct(';' | '=')) + || (matches!(token.1, Js::Ident) && js.get(token.0.clone()) == Some("in") && !matches!(tokens.last(), Some((_, Js::Punct('.'))))) + { + *left = false; + } + } + let mut after_condition = false; + match token.1 { + Js::Punct('(') => delimiters.push((')', opens_condition(js, tokens))), + Js::Punct('[') => delimiters.push((']', Delimiter::Group)), + Js::Punct('{') => delimiters.push(('}', Delimiter::Group)), + Js::Punct(close @ (')' | ']' | '}')) if delimiters.last().is_some_and(|(end, _)| *end == close) => { + after_condition = matches!(delimiters.pop(), Some((_, Delimiter::Condition | Delimiter::For { .. }))); + } + _ => {} + } + (after_condition, after_for_of) +} + /// Whether a `/` after `tokens` starts a regular expression: after an operator, an opening /// bracket, a keyword such as `return` or the condition of `if`, `while`, `for` or `with`, not /// after a value. `after_condition` says the last token is the `)` that ends such a condition. -fn regex_may_follow(js: &str, tokens: &[Token], after_condition: bool) -> bool { +fn regex_may_follow(js: &str, tokens: &[Token], after_condition: bool, after_for_of: bool) -> bool { match tokens.last() { None => true, - Some((range, Js::Ident)) => js.get(range.clone()).is_some_and(|w| BEFORE_REGEX.contains(&w)), + Some((range, Js::Ident)) => { + !matches!(tokens.get(tokens.len().wrapping_sub(2)), Some((_, Js::Punct('.')))) + && js.get(range.clone()).is_some_and(|w| BEFORE_REGEX.contains(&w) || (w == "of" && after_for_of)) + } Some((_, Js::Punct(')'))) => after_condition, Some((_, Js::Punct(c))) => !matches!(c, ']' | '}'), Some((_, Js::Str(_) | Js::Other)) => false, } } -/// Whether a `(` after `tokens` opens the condition of `if`, `while`, `for` or `with`, and not +/// Whether a `(` after `tokens` opens the condition of `if`, `while`, `for` (including +/// `for await`) or `with`, and not /// the arguments of a method that has one of those names (`o.if(x)`). -fn opens_condition(js: &str, tokens: &[Token]) -> bool { +fn opens_condition(js: &str, tokens: &[Token]) -> Delimiter { let mut back = tokens.iter().rev(); - let Some((range, Js::Ident)) = back.next() else { return false }; - js.get(range.clone()).is_some_and(|w| BEFORE_CONDITION.contains(&w)) && !matches!(back.next(), Some((_, Js::Punct('.')))) + let Some((range, Js::Ident)) = back.next() else { return Delimiter::Group }; + let mut keyword = js.get(range.clone()); + if keyword == Some("await") { + let Some((range, Js::Ident)) = back.next() else { return Delimiter::Group }; + keyword = js.get(range.clone()); + if keyword != Some("for") { + return Delimiter::Group; + } + } + if matches!(back.next(), Some((_, Js::Punct('.')))) { + return Delimiter::Group; + } + match keyword { + Some("for") => Delimiter::For { left: true }, + Some(w) if BEFORE_CONDITION.contains(&w) => Delimiter::Condition, + _ => Delimiter::Group, + } +} + +/// A binding/assignment target before the for-of separator, not a declaration keyword, +/// expression-opening operator, or the start of the header itself. +fn for_target_ends(js: &str, tokens: &[Token]) -> bool { + match tokens.last() { + Some((range, Js::Ident)) => !matches!(js.get(range.clone()), Some("var" | "let" | "const")), + Some((_, Js::Punct(']' | '}' | ')'))) => true, + _ => false, + } } /// Where a Calculate script keeps its simplified field notation: between `BVCALC` and the @@ -455,10 +536,12 @@ fn splice(src: &str, edits: impl IntoIterator, String)>) -> } /// Simplified field notation as the JavaScript Acrobat writes after its `BVCALC` comment: -/// each field name becomes `AFMakeNumber(this.getField("name").value)`, the rest stays. +/// Each field name becomes `AFMakeNumber(this.getField("name").value)`; adjacent signs +/// are separated so binary plus/minus followed by a unary sign cannot become an update. pub fn notation_js(expr: &str) -> String { let names = notation_tokens(expr).into_iter().filter_map(|(range, t)| match t { Notation::Name(name) => Some((range, format!("AFMakeNumber(this.getField({}).value)", js_str(&name)))), + Notation::Op(sign @ ('+' | '-')) if expr.get(range.end..).is_some_and(|rest| rest.starts_with(sign)) => Some((range, format!("{sign} "))), _ => None, }); splice(expr, names) @@ -507,6 +590,10 @@ pub(super) fn eval_notation(src: &str, value_of: &dyn Fn(&str) -> f64) -> Option *i += 1; Some(*n) } + T::Op('+') => { + *i += 1; + factor(t, i, d + 1) + } T::Op('-') => { *i += 1; factor(t, i, d + 1).map(|v| -v) @@ -558,3 +645,33 @@ pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option< } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn regex_scan_does_not_cross_line_break_after_backslash_or_split_utf8() { + let js = "x = /a\\\nthis.getField(\"price\").value / 2;"; + assert_eq!(calculation_reads(js), vec!["price".to_owned()]); + + let js_utf8 = "var r = /\\é/; event.value = this.getField(\"tax\").value;"; + assert_eq!(calculation_reads(js_utf8), vec!["tax".to_owned()]); + + let trailing_backslash = "event.value = this.getField(\"subtotal\").value + /unterminated\\"; + assert_eq!(calculation_reads(trailing_backslash), vec!["subtotal".to_owned()]); + let template_trailing = "event.value = this.getField(\"shipping\").value + `unterminated\\"; + assert_eq!(calculation_reads(template_trailing), vec!["shipping".to_owned()]); + } + + #[test] + fn eval_notation_supports_unary_plus() { + let c = Calculate::Notation("+a + (+b)".to_owned()); + let out = calculate(&c, &|name| match name { + "a" => vec!["3".to_owned()], + "b" => vec!["4".to_owned()], + _ => Vec::new(), + }); + assert_eq!(out.as_deref(), Some("7")); + } +} diff --git a/crates/forms/src/af/script.rs b/crates/forms/src/af/script.rs index f15714c411..8212da5bf2 100644 --- a/crates/forms/src/af/script.rs +++ b/crates/forms/src/af/script.rs @@ -17,7 +17,7 @@ impl Arg { match self { Arg::Num(n) => Some(*n), Arg::Bool(b) => Some(f64::from(u8::from(*b))), - Arg::Str(s) => s.trim().parse().ok(), + Arg::Str(s) => s.trim().parse::().ok().filter(|n| n.is_finite()), Arg::List(_) => None, } } @@ -99,17 +99,36 @@ impl Parser<'_> { let mut encoded = [0u8; 4]; out.extend_from_slice(ch.encode_utf8(&mut encoded).as_bytes()); } + b'u' if self.s.get(self.i) == Some(&b'{') => { + self.i += 1; + let start = self.i; + let mut code = 0u32; + while let Some(&digit) = self.s.get(self.i).filter(|&&b| b != b'}') { + code = code.checked_mul(16)?.checked_add(char::from(digit).to_digit(16)?)?; + if code > 0x10FFFF { + return None; + } + self.i += 1; + } + if self.i == start || self.s.get(self.i) != Some(&b'}') { + return None; + } + self.i += 1; + // Match the existing handling of unpaired UTF-16 surrogates. + let ch = char::from_u32(code).unwrap_or('\u{FFFD}'); + let mut encoded = [0u8; 4]; + out.extend_from_slice(ch.encode_utf8(&mut encoded).as_bytes()); + } b'u' => { let code = hex4(self.s.get(self.i..self.i.checked_add(4)?)?)?; self.i += 4; - let code = if (0xD800..=0xDBFF).contains(&code) && self.s.get(self.i..self.i.checked_add(6)?)?.starts_with(b"\\u") { - let low = hex4(self.s.get(self.i.checked_add(2)?..self.i.checked_add(6)?)?)?; - if (0xDC00..=0xDFFF).contains(&low) { - self.i += 6; - 0x10000 + ((u32::from(code) - 0xD800) << 10) + (u32::from(low) - 0xDC00) - } else { - u32::from(code) - } + let code = if (0xD800..=0xDBFF).contains(&code) + && let Some(pair) = self.i.checked_add(6).and_then(|end| self.s.get(self.i..end)) + && pair.starts_with(b"\\u") + && let Some(low) = hex4(&pair[2..6]).filter(|low| (0xDC00..=0xDFFF).contains(low)) + { + self.i += 6; + 0x10000 + ((u32::from(code) - 0xD800) << 10) + (u32::from(low) - 0xDC00) } else { u32::from(code) }; @@ -158,7 +177,7 @@ impl Parser<'_> { while self.i < self.s.len() && (self.s[self.i].is_ascii_digit() || matches!(self.s[self.i], b'.' | b'-' | b'+' | b'e' | b'E')) { self.i += 1; } - std::str::from_utf8(&self.s[start..self.i]).ok()?.parse().ok().map(Arg::Num) + std::str::from_utf8(&self.s[start..self.i]).ok()?.parse::().ok().filter(|n| n.is_finite()).map(Arg::Num) } } } @@ -196,15 +215,30 @@ impl Parser<'_> { } } -/// The first call to `name(` in `js` and its arguments. +/// The first genuine call to `name(` in `js` and its arguments. +/// An unsupported first call returns `None`, preserving the custom-script fallback; later calls +/// must not substitute a different native rule. Comments, strings and unrelated identifiers are skipped. pub(super) fn call(js: &str, name: &str) -> Option> { - let at = js.find(&format!("{name}("))?; - // Not part of a longer identifier (AFDate_Format vs AFDate_FormatEx). - if js[..at].chars().last().is_some_and(|c| c.is_alphanumeric() || c == '_') { - return None; + let (tokens, _) = js_tokens(js); + for (k, (range, tok)) in tokens.iter().enumerate() { + if *tok != super::calc::Js::Ident { + continue; + } + if k > 0 && matches!(tokens.get(k - 1), Some((_, super::calc::Js::Punct('.')))) { + continue; + } + let Some(target_end) = range.start.checked_add(name.len()) else { continue }; + if js.get(range.start..target_end) != Some(name) { + continue; + } + let Some(end_k) = (k..tokens.len()).take_while(|&m| tokens[m].0.end <= target_end).find(|&m| tokens[m].0.end == target_end) else { + continue; + }; + let Some((open, super::calc::Js::Punct('('))) = tokens.get(end_k + 1) else { continue }; + let mut p = Parser { s: js.as_bytes(), i: open.end, depth: 0 }; + return p.list(b')'); } - let mut p = Parser { s: js.as_bytes(), i: at + name.len() + 1, depth: 0 }; - p.list(b')') + None } pub fn parse_format(js: &str) -> Option { @@ -246,7 +280,8 @@ pub fn parse_validate(js: &str) -> Option { let args = call(js, "AFRange_Validate")?; let flag = |i: usize| args.get(i).is_some_and(Arg::truthy); let num = |i: usize| args.get(i).and_then(Arg::num); - Some(Validate::Range { min: if flag(0) { num(1) } else { None }, max: if flag(2) { num(3) } else { None } }) + // An enabled but unparseable bound is unsupported, not an absent bound. + Some(Validate::Range { min: if flag(0) { Some(num(1)?) } else { None }, max: if flag(2) { Some(num(3)?) } else { None } }) } pub fn parse_calculate(js: &str) -> Option { @@ -361,3 +396,89 @@ pub fn calculate_js(c: &Calculate) -> Option { Calculate::Notation(expr) => Some(format!("/** BVCALC {expr} EVCALC **/ event.value = {};", notation_js(expr))), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unsupported_first_validation_call_does_not_select_a_later_rule() { + let js = "if (event.value < 50) AFRange_Validate(true, 10 + 5, false, 0); else AFRange_Validate(false, 0, true, 100);"; + assert_eq!(parse_validate(js), None); + assert_eq!( + parse_validate("AFRange_Validate(true, 15, false, 0); AFRange_Validate(false, 0, true, 100);"), + Some(Validate::Range { min: Some(15.0), max: None }) + ); + } + + #[test] + fn validation_call_still_skips_non_calls_before_literal_arguments() { + let js = r#"// AFRange_Validate(true, 10 + 5, false, 0) + "AFRange_Validate(true, 10 + 5, false, 0)"; + my_AFRange_Validate(true, 10 + 5, false, 0); + other.AFRange_Validate(true, 10 + 5, false, 0); + AFRange_Validate(false, 0, true, 100);"#; + assert_eq!(parse_validate(js), Some(Validate::Range { min: None, max: Some(100.0) })); + } + + #[test] + fn unpaired_high_surrogate_at_end_of_string_does_not_abort_parsing() { + let js = "AFDate_FormatEx(\"\\uD800\")"; + assert_eq!(parse_format(js), Some(Format::Date("\u{FFFD}".to_owned()))); + } + + #[test] + fn call_skips_commented_or_prefixed_identifiers_and_rejects_infinite_numbers() { + let js = "// AFPercent_Format(0, 0);\nmy_AFPercent_Format(0, 0); $AFPercent_Format(0, 0); AFPercent_Format (2, 1);"; + assert_eq!(parse_format(js), Some(Format::Percent { decimals: 2, sep: 1 })); + + assert_eq!(parse_validate("AFRange_Validate(true, 1e500, false, 0);"), None); + } + #[test] + fn review_enabled_nonfinite_bound_is_unsupported() { + for script in [r#"AFRange_Validate(true, "1e500", false, 0)"#, r#"AFRange_Validate(false, 0, true, "-1e500")"#] { + assert_eq!(parse_validate(script), None); + } + assert_eq!(parse_validate(r#"AFRange_Validate(false, "1e500", false, "NaN")"#), Some(Validate::Range { min: None, max: None })); + } + + #[test] + fn review_format_after_postfix_division() { + for op in ["++", "--"] { + let js = format!("var y = x{op} / 2; AFPercent_Format(2, 1); var z = 1 / 2;"); + assert_eq!(parse_format(&js), Some(Format::Percent { decimals: 2, sep: 1 })); + let commented = format!("var y = x/* comment */{op} / 2; AFPercent_Format(2, 1); var z = 1 / 2;"); + assert_eq!(parse_format(&commented), Some(Format::Percent { decimals: 2, sep: 1 })); + } + assert_eq!(parse_format("var of = 8; of / 2; AFPercent_Format(2, 1); var z = 1 / 2;"), Some(Format::Percent { decimals: 2, sep: 1 })); + assert_eq!( + parse_format("for (var x of /AFPercent_Format(0, 0)/g) {} AFPercent_Format(2, 1);"), + Some(Format::Percent { decimals: 2, sep: 1 }) + ); + // A newline makes ++ a prefix operator; the following regex must remain opaque. + for js in ["x\n++/AFPercent_Format(2, 1)/.lastIndex;", "x/*\n*/++/AFPercent_Format(2, 1)/.lastIndex;"] { + assert_eq!(parse_format(js), None); + } + } + + #[test] + fn review_format_after_brace_unicode() { + assert_eq!(parse_format(r#"var s = "\u{1F600}"; AFPercent_Format(2, 1);"#), Some(Format::Percent { decimals: 2, sep: 1 })); + assert_eq!(parse_format(r#"AFDate_FormatEx("\u{1F600}")"#), Some(Format::Date("😀".to_owned()))); + assert_eq!(parse_format(r#"AFDate_FormatEx("\u{00000041}\u0042\uD83D\uDE00")"#), Some(Format::Date("AB😀".to_owned()))); + for escape in ["\\u{}", "\\u{110000}", "\\u{xyz}", "\\u{41"] { + assert_eq!(parse_format(&format!("AFDate_FormatEx(\"{escape}\")")), None); + } + } + + #[test] + fn review_notation_separates_adjacent_operators() { + let field = |name| format!("AFMakeNumber(this.getField(\"{name}\").value)"); + for (expr, operators) in [("a++b", "+ +"), ("a--b", "- -"), ("a+++b", "+ + +")] { + let js = calculate_js(&Calculate::Notation(expr.to_owned())).unwrap(); + let executable = js.split("event.value = ").nth(1).unwrap(); + assert_eq!(executable, format!("{}{operators}{};", field("a"), field("b"))); + assert_eq!(super::super::calc::calculate(&Calculate::Notation(expr.to_owned()), &|_| vec!["2".to_owned()]), Some("4".to_owned())); + } + } +} diff --git a/crates/forms/src/af/tests.rs b/crates/forms/src/af/tests.rs index 1e546db75a..3c6dab1bef 100644 --- a/crates/forms/src/af/tests.rs +++ b/crates/forms/src/af/tests.rs @@ -444,3 +444,45 @@ fn validation_and_calculation() { assert_eq!(calculate(&Calculate::Notation("(a + b) * 2 - Unit\\ Price / 4".into()), &vals), Some("10".into())); assert_eq!(calculate(&Calculate::Notation("a +".into()), &vals), None); } + +/// Async for-of headers have the same regex-literal contexts as ordinary for-of headers. +#[test] +fn for_await_regex_literals_do_not_become_format_calls() { + for header in ["for await", "for /* gap */ await"] { + let js = format!("async function f() {{ {header} (const x of /AFPercent_Format(0, 0)/g) {{}} }} AFPercent_Format(2, 1);"); + assert_eq!(parse_format(&js), Some(Format::Percent { decimals: 2, sep: 1 })); + let body = format!("async function f() {{ {header} (const x of xs) /AFPercent_Format(0, 0)/.test(x); }} AFPercent_Format(2, 1);"); + assert_eq!(parse_format(&body), Some(Format::Percent { decimals: 2, sep: 1 })); + } + assert_eq!(parse_format("obj.await(x) / 2; AFPercent_Format(2, 1); var z = 1 / 2;"), Some(Format::Percent { decimals: 2, sep: 1 })); +} + +#[test] +fn identifier_of_in_conditions_keeps_division_and_real_format_calls() { + for header in [ + "if (of / 2)", + "while (of / 2)", + "with (of / 2)", + "for (of / 2; ; )", + "for (var x = of / 2; ; )", + "for (var x = 0; of / 2; )", + "for (x of of / 2)", + "for (const x of of / 2)", + "for (x in of / 2)", + "for (var of / 2; ; )", + "for (obj.of / 2; ; )", + ] { + let js = format!("{header} AFPercent_Format(2, 1); var z = 1 / 2;"); + assert_eq!(parse_format(&js), Some(Format::Percent { decimals: 2, sep: 1 }), "{header}"); + } +} + +#[test] +fn destructured_for_of_targets_keep_regex_iterables_opaque() { + for header in ["for", "for await"] { + for target in ["const [x]", "const {x}", "(x)", "const {x = of / 2}", "obj.in", "obj.of", "const of"] { + let js = format!("async function f() {{ {header} ({target} of /AFPercent_Format(0, 0)/g) {{}} }} AFPercent_Format(2, 1);"); + assert_eq!(parse_format(&js), Some(Format::Percent { decimals: 2, sep: 1 }), "{header} {target}"); + } + } +} diff --git a/crates/forms/src/author/join.rs b/crates/forms/src/author/join.rs index e9fedbd655..b8c2520dd6 100644 --- a/crates/forms/src/author/join.rs +++ b/crates/forms/src/author/join.rs @@ -5,16 +5,17 @@ use std::collections::{HashMap, HashSet}; use super::lifecycle::{drop_from_calculation_order, split_merged, split_triggers}; -use super::tree::{detach, list_of}; +use super::tree::{ancestor_sets, detach, list_of}; use super::{Dict, Document, Field, FormError, ObjRef, Object, invalid}; /// The keys a field that is also its own widget sheds when it becomes only a widget. Its `/AA` /// keeps only the widget's own triggers. const FIELD_ONLY_KEYS: [&[u8]; 15] = [b"T", b"FT", b"Kids", b"Ff", b"V", b"DV", b"Opt", b"TU", b"TM", b"MaxLen", b"Q", b"DS", b"RV", b"TI", b"I"]; -/// Whether `field` holds `/Opt`, which its buttons' export values index (ISO 32000-2 §12.7.5.2). +/// Whether `field` holds or inherits `/Opt`, which its buttons' export values index (ISO 32000-2 §12.7.5.2). fn has_opt(doc: &Document, field: ObjRef) -> bool { - doc.get(field).as_dict().is_some_and(|d| d.contains(b"Opt")) + doc.get(field).as_dict().and_then(|d| d.get(b"Opt")).is_some_and(|v| !matches!(*doc.resolve(v), Object::Null)) + || ancestor_sets(doc, field, b"Opt") } /// Refuse what [`join_radio_group`] can't do, before anything is written. @@ -127,3 +128,26 @@ fn rename_state(doc: &Document, d: &mut Dict, old: &str, new: &str) -> bool { d.set(b"AP".to_vec(), Object::Dict(ap)); true } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn has_opt_ignores_null_and_detects_inherited_opt() { + let mut doc = Document::new_empty(); + let mut parent = Dict::new(); + parent.set(b"Opt".to_vec(), Object::Array(vec![])); + let parent_ref = doc.add(Object::Dict(parent)); + + let mut f_null = Dict::new(); + f_null.set(b"Opt".to_vec(), Object::Null); + let f_null_ref = doc.add(Object::Dict(f_null)); + assert!(!has_opt(&doc, f_null_ref)); + + let mut f_inherited = Dict::new(); + f_inherited.set(b"Parent".to_vec(), Object::Ref(parent_ref)); + let f_inherited_ref = doc.add(Object::Dict(f_inherited)); + assert!(has_opt(&doc, f_inherited_ref)); + } +} diff --git a/crates/forms/src/author/tree.rs b/crates/forms/src/author/tree.rs index ce4ee687bb..55b84fa390 100644 --- a/crates/forms/src/author/tree.rs +++ b/crates/forms/src/author/tree.rs @@ -112,6 +112,11 @@ pub(crate) fn form_fonts_as_objects(doc: &mut Document, all: &[crate::Field]) -> Ok(()) } +/// Whether `d` has a non-null entry `key` (a `null` entry is absent per ISO 32000-2 §7.3.9). +fn dict_has(doc: &Document, d: &Dict, key: &[u8]) -> bool { + d.get(key).is_some_and(|v| !matches!(*doc.resolve(v), Object::Null)) +} + /// Whether the field `r`, or the nearest ancestor that has one, holds a `/V` (the entry is /// inheritable, ISO 32000-2 §12.7.4). The `/Parent` chain is followed at most 64 deep, as deep /// as the field walk goes, so a cyclic chain ends. @@ -120,7 +125,7 @@ fn holds_value(doc: &Document, r: ObjRef) -> bool { for _ in 0..=64 { let obj = doc.get(at); let Some(d) = obj.as_dict() else { return false }; - if d.contains(b"V") { + if dict_has(doc, d, b"V") { return true; } match d.reference(b"Parent") { @@ -160,8 +165,8 @@ fn add_to_fields(doc: &mut Document, af: ObjRef, field: ObjRef) -> Result<(), Fo /// The most parts a dotted field name may have ("a.b.c" has three). const MAX_NAME_PARTS: usize = 32; -/// The field attributes a field inherits from its parents (ISO 32000-2 §12.7.4.1). -const INHERITED_KEYS: [&[u8]; 7] = [b"FT", b"Ff", b"V", b"DV", b"DA", b"Q", b"MaxLen"]; +/// The field attributes a field inherits from its parents (ISO 32000-2 §12.7.4.1, §12.7.5.2). +const INHERITED_KEYS: [&[u8]; 8] = [b"FT", b"Ff", b"V", b"DV", b"DA", b"Q", b"MaxLen", b"Opt"]; /// The parts of a full field name: `a.b` is the field `b` under the parent field `a`, since a /// field's own (partial) name can't contain a period (ISO 32000-2 §12.7.4.2) (#2696). @@ -234,7 +239,7 @@ pub(super) fn check_place(doc: &Document, af: ObjRef, field: ObjRef, parts: &[&s break; }; // A value, reset value or length limit held by a parent would pass to this field. - let held = doc.get(r).as_dict().is_some_and(|d| PASSED_VALUES.iter().any(|k| d.contains(k) && !owned.contains(k))); + let held = doc.get(r).as_dict().is_some_and(|d| PASSED_VALUES.iter().any(|k| dict_has(doc, d, k) && !dict_has(doc, &owned, k))); if held { return invalid(format!("{part:?} holds a value for the fields under it, so {:?} can't be put there", parts.join("."))); } @@ -259,7 +264,7 @@ pub(super) fn attach(doc: &mut Document, af: ObjRef, field: ObjRef, parts: &[&st for part in parents { let next = match child_named(doc, af, parent, part) { Some(r) => { - inherits |= doc.get(r).as_dict().is_some_and(|d| d.contains(b"Ff") || d.contains(b"Q")); + inherits |= doc.get(r).as_dict().is_some_and(|d| dict_has(doc, d, b"Ff") || dict_has(doc, d, b"Q")); r } None => { @@ -308,12 +313,20 @@ pub(super) fn inherited(doc: &Document, field: ObjRef) -> Dict { } let Some(d) = doc.get(p).as_dict().cloned() else { break }; for k in INHERITED_KEYS { - if let Some(v) = d.get(k).filter(|_| !own.contains(k) && !add.contains(k)) { + if let Some(v) = d.get(k).filter(|_| dict_has(doc, &d, k) && !dict_has(doc, &own, k) && !add.contains(k)) { add.set(k.to_vec(), v.clone()); } } parent = d.reference(b"Parent"); } + // /Opt is inheritable for check boxes and radio buttons (Table 230), not choice + // fields (Table 234). Classify by effective /FT and /Ff before materializing it. + let effective = |key: &[u8]| if dict_has(doc, &own, key) { own.get(key) } else { add.get(key) }; + let button = effective(b"FT").is_some_and(|v| doc.resolve(v).as_name() == Some(b"Btn")); + let flags = effective(b"Ff").and_then(|v| doc.resolve(v).as_int()).and_then(|v| u32::try_from(v).ok()).unwrap_or(0); + if !button || flags & crate::flags::PUSH_BUTTON != 0 { + add.remove(b"Opt"); + } add } @@ -327,7 +340,7 @@ pub(crate) fn ancestor_sets(doc: &Document, field: ObjRef, key: &[u8]) -> bool { return false; } let Some(d) = doc.get(p).as_dict().cloned() else { return false }; - if d.contains(key) { + if dict_has(doc, &d, key) { return true; } parent = d.reference(b"Parent"); @@ -396,3 +409,162 @@ pub(super) fn top_names(doc: &Document) -> Vec { let list = af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default(); list.iter().filter_map(|o| doc.resolve(o).as_dict().and_then(|d| d.get(b"T").cloned())).filter_map(|t| crate::text_of(&doc.resolve(&t))).collect() } + +#[cfg(test)] +mod tests { + use super::*; + + /// /Opt is inheritable only for check boxes and radio buttons, using effective FT/Ff. + #[test] + fn inherited_options_are_limited_to_checkboxes_and_radios() { + for (ft, flags, expected) in + [("Btn", 0, true), ("Btn", crate::flags::RADIO, true), ("Btn", crate::flags::PUSH_BUTTON, false), ("Ch", 0, false), ("Tx", 0, false)] + { + for own_type in [false, true] { + let mut doc = Document::new_empty(); + let mut parent = Dict::new(); + parent.set(b"FT".to_vec(), Object::name(ft)); + parent.set(b"Ff".to_vec(), Object::Int(i64::from(flags))); + parent.set(b"Opt".to_vec(), Object::Array(vec![Object::String(PdfString::text("export"))])); + parent.set(b"DA".to_vec(), PdfString::text("/Helv 10 Tf 0 g")); + let p = doc.add(parent); + let mut child = Dict::new(); + child.set(b"Parent".to_vec(), Object::Ref(p)); + if own_type { + child.set(b"FT".to_vec(), Object::name(ft)); + child.set(b"Ff".to_vec(), Object::Int(i64::from(flags))); + } + let c = doc.add(child); + let inherited = inherited(&doc, c); + assert_eq!(inherited.contains(b"Opt"), expected, "{ft}, flags {flags}, own {own_type}"); + assert!(inherited.contains(b"DA")); + } + } + } + + fn one_page() -> Document { + let mut doc = Document::new_empty(); + let pages = doc.get(doc.root().unwrap()).as_dict().unwrap().reference(b"Pages").unwrap(); + let mut page = Dict::new(); + page.set(b"Type".to_vec(), Object::name("Page")); + page.set(b"Parent".to_vec(), Object::Ref(pages)); + page.set(b"MediaBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 600.into(), 800.into()])); + let page = doc.add(page); + doc.update_dict(pages, |d| { + d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(page)])); + d.set(b"Count".to_vec(), Object::Int(1)); + }) + .unwrap(); + doc + } + + fn reopen(doc: &Document) -> Document { + let bytes = pdfcraft_cos::write_incremental(doc, &pdfcraft_cos::SaveOptions::default()).unwrap(); + hayro_syntax::Pdf::new(bytes.clone()).unwrap(); + Document::open(std::sync::Arc::new(bytes)).unwrap() + } + + /// Moving a choice field must not turn its ancestor's unrelated /Opt into local choices. + #[test] + fn moving_choice_fields_does_not_copy_ancestor_options() { + for combo in [false, true] { + for own_options in [false, true] { + let mut doc = one_page(); + let options = if own_options { vec!["local".into()] } else { Vec::new() }; + let kind = if combo { crate::NewField::Combo { options, editable: false } } else { crate::NewField::List { options, multi: false } }; + crate::add_field(&mut doc, 0, [50.0, 700.0, 250.0, 720.0], &kind, Some("old.pick")).unwrap(); + let f = crate::fields(&doc).into_iter().find(|f| f.name == "old.pick").unwrap(); + let parent = doc.get(f.obj).as_dict().unwrap().reference(b"Parent").unwrap(); + if !own_options { + doc.update_dict(f.obj, |d| { + d.remove(b"Opt"); + }) + .unwrap(); + } + doc.update_dict(parent, |d| d.set(b"Opt".to_vec(), Object::Array(vec![Object::String(PdfString::text("borrowed"))]))).unwrap(); + let before = crate::fields(&doc).into_iter().find(|f| f.name == "old.pick").unwrap().options; + crate::set_props(&mut doc, "old.pick", &crate::FieldProps { name: Some("new.pick".into()), ..Default::default() }).unwrap(); + let after = crate::fields(&doc).into_iter().find(|f| f.name == "new.pick").unwrap(); + assert_eq!(after.options, before); + assert_eq!(doc.get(after.obj).as_dict().unwrap().contains(b"Opt"), own_options); + let reopened = reopen(&doc); + let after = crate::fields(&reopened).into_iter().find(|f| f.name == "new.pick").unwrap(); + assert_eq!(after.options, before); + assert_eq!(reopened.get(after.obj).as_dict().unwrap().contains(b"Opt"), own_options); + } + } + } + + #[test] + fn moving_buttons_preserves_inherited_export_mapping() { + for kind in [crate::NewField::CheckBox, crate::NewField::Radio { group: None, export: "0".into() }] { + let mut doc = one_page(); + crate::add_field(&mut doc, 0, [50.0, 700.0, 70.0, 720.0], &kind, Some("old.button")).unwrap(); + let field = crate::fields(&doc).into_iter().find(|f| f.name == "old.button").unwrap(); + let state = field.widgets.first().unwrap().on_state.clone().unwrap(); + let parent = doc.get(field.obj).as_dict().unwrap().reference(b"Parent").unwrap(); + let opt = Object::Array(vec![Object::String(PdfString::text("exported"))]); + doc.update_dict(parent, |d| d.set(b"Opt".to_vec(), opt.clone())).unwrap(); + doc.update_dict(field.obj, |d| { + d.remove(b"Opt"); + }) + .unwrap(); + crate::set_props(&mut doc, "old.button", &crate::FieldProps { name: Some("new.button".into()), ..Default::default() }).unwrap(); + for doc in [&doc, &reopen(&doc)] { + let after = crate::fields(doc).into_iter().find(|f| f.name == "new.button").unwrap(); + assert_eq!(doc.get(after.obj).as_dict().unwrap().get(b"Opt"), Some(&opt)); + assert_eq!(after.export_of(0), Some("exported")); + assert_eq!(after.state_for("exported"), Some(state.as_str())); + } + } + } + + #[test] + fn null_entries_on_parents_and_unsigned_signatures_are_treated_as_absent() { + let mut doc = Document::new_empty(); + let mut sig = Dict::new(); + sig.set(b"FT".to_vec(), Object::name("Sig")); + sig.set(b"T".to_vec(), PdfString::text("sig")); + sig.set(b"V".to_vec(), Object::Null); + let sig_ref = doc.add(Object::Dict(sig)); + assert!(!holds_value(&doc, sig_ref)); + + let mut grand = Dict::new(); + grand.set(b"FT".to_vec(), Object::name("Tx")); + grand.set(b"MaxLen".to_vec(), Object::Int(10)); + let grand_ref = doc.add(Object::Dict(grand)); + + let mut parent = Dict::new(); + parent.set(b"Parent".to_vec(), Object::Ref(grand_ref)); + parent.set(b"T".to_vec(), PdfString::text("group")); + parent.set(b"V".to_vec(), Object::Null); + parent.set(b"MaxLen".to_vec(), Object::Null); + let parent_ref = doc.add(Object::Dict(parent)); + + let mut child = Dict::new(); + child.set(b"Parent".to_vec(), Object::Ref(parent_ref)); + child.set(b"T".to_vec(), PdfString::text("item")); + let child_ref = doc.add(Object::Dict(child)); + + let inh = inherited(&doc, child_ref); + assert_eq!(inh.get(b"MaxLen"), Some(&Object::Int(10))); + assert!(!inh.contains(b"V")); + assert!(!ancestor_sets(&doc, child_ref, b"V")); + assert!(ancestor_sets(&doc, child_ref, b"MaxLen")); + + // Button /Opt is inherited, text /Opt is not; null kids and /Parent cycles are handled. + let opt_val = Object::Array(vec![Object::String(PdfString::text("Choice1"))]); + doc.update_dict(grand_ref, |d| { + d.set(b"Opt".to_vec(), opt_val.clone()); + d.set(b"Parent".to_vec(), Object::Ref(parent_ref)); + d.set(b"Kids".to_vec(), Object::Array(vec![Object::Null, Object::Ref(parent_ref)])); + }) + .unwrap(); + assert!(!inherited(&doc, child_ref).contains(b"Opt")); + doc.update_dict(grand_ref, |d| d.set(b"FT".to_vec(), Object::name("Btn"))).unwrap(); + let inh_cycle = inherited(&doc, child_ref); + assert_eq!(inh_cycle.get(b"Opt"), Some(&opt_val)); + assert!(!holds_value(&doc, child_ref)); + assert_eq!(child_named(&doc, grand_ref, Some(grand_ref), "group"), Some(parent_ref)); + } +} diff --git a/crates/forms/src/tab_order.rs b/crates/forms/src/tab_order.rs index 5b60e3927b..5c46293303 100644 --- a/crates/forms/src/tab_order.rs +++ b/crates/forms/src/tab_order.rs @@ -137,23 +137,34 @@ pub fn tab_orders(doc: &Document) -> Vec { /// annotations keep their places. pub fn move_in_tab_order(doc: &mut Document, name: &str, earlier: bool) -> Result<(), FormError> { let all = fields(doc); - let f = all.iter().find(|f| f.name == name).ok_or_else(|| FormError::NoSuchField(name.into()))?; - let page = f.widgets.iter().filter_map(|w| w.page).min().ok_or_else(|| FormError::Invalid(format!("{name} is not on a page")))?; + if !all.iter().any(|f| f.name == name) { + return Err(FormError::NoSuchField(name.into())); + } + let (target_fi, page) = all + .iter() + .enumerate() + .filter(|(_, f)| f.name == name) + .find_map(|(fi, f)| Some((fi, f.widgets.iter().filter_map(|w| w.page).min()?))) + .ok_or_else(|| FormError::Invalid(format!("{name} is not on a page")))?; // The page's fields in their current tab order (a field counts once, at its first widget). - let mut order: Vec<(usize, &str)> = - all.iter().filter_map(|g| g.widgets.iter().filter(|w| w.page == Some(page)).map(|w| w.tab).min().map(|t| (t, g.name.as_str()))).collect(); + let mut order: Vec<(usize, usize)> = all + .iter() + .enumerate() + .filter_map(|(fi, g)| g.widgets.iter().filter(|w| w.page == Some(page)).map(|w| w.tab).min().map(|t| (t, fi))) + .collect(); order.sort(); - let names: Vec<&str> = order.iter().map(|(_, n)| *n).collect(); - let Some(i) = names.iter().position(|n| *n == name) else { return invalid(format!("{name} is not on a page")) }; - let j = if earlier { i.checked_sub(1) } else { Some(i + 1).filter(|j| *j < names.len()) }; + let Some(i) = order.iter().position(|&(_, fi)| fi == target_fi) else { return invalid(format!("{name} is not on a page")) }; + let j = if earlier { i.checked_sub(1) } else { Some(i + 1).filter(|j| *j < order.len()) }; let Some(j) = j else { return Ok(()) }; - let mut names = names.into_iter().map(str::to_string).collect::>(); - names.swap(i, j); - // The widgets of this page in the new order. + order.swap(i, j); + // The widgets of this page in the new order, preserving each field's relative widget tab order. let mut widgets: Vec = Vec::new(); - for n in &names { - if let Some(g) = all.iter().find(|g| &g.name == n) { - widgets.extend(g.widgets.iter().filter(|w| w.page == Some(page)).map(|w| w.obj)); + let mut queued = std::collections::HashSet::new(); + for &(_, fi) in &order { + if let Some(g) = all.get(fi) { + let mut own: Vec<(usize, ObjRef)> = g.widgets.iter().filter(|w| w.page == Some(page)).map(|w| (w.tab, w.obj)).collect(); + own.sort_by_key(|&(tab, _)| tab); + widgets.extend(own.into_iter().map(|(_, obj)| obj).filter(|obj| queued.insert(*obj))); } } let pr = *page_refs(doc).get(page).ok_or_else(|| FormError::Invalid("no such page".into()))?; @@ -162,10 +173,11 @@ pub fn move_in_tab_order(doc: &mut Document, name: &str, earlier: bool) -> Resul let list = doc.resolve(&annots_obj).as_array().cloned().unwrap_or_default(); let mut next = widgets.into_iter(); let set: std::collections::HashSet = all.iter().flat_map(|g| g.widgets.iter().filter(|w| w.page == Some(page)).map(|w| w.obj)).collect(); + let mut seen_slots = std::collections::HashSet::new(); let new_list: Vec = list .iter() .map(|o| match o.as_ref() { - Some(r) if set.contains(&r) => next.next().map(Object::Ref).unwrap_or_else(|| o.clone()), + Some(r) if set.contains(&r) && seen_slots.insert(r) => next.next().map(Object::Ref).unwrap_or_else(|| o.clone()), _ => o.clone(), }) .collect(); @@ -178,3 +190,85 @@ pub fn move_in_tab_order(doc: &mut Document, name: &str, earlier: bool) -> Resul })?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use pdfcraft_cos::{Dict, PdfString}; + + #[test] + fn move_in_tab_order_preserves_multi_widget_relative_tab_order() { + let mut doc = Document::new_empty(); + let pages_ref = doc.add(Object::Null); + let page_ref = doc.add(Object::Null); + + let mut w1 = Dict::new(); + w1.set(b"Subtype".to_vec(), Object::name("Widget")); + w1.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(200), Object::Int(700), Object::Int(220), Object::Int(720)])); + let w1_ref = doc.add(Object::Dict(w1)); + + let mut w2 = Dict::new(); + w2.set(b"Subtype".to_vec(), Object::name("Widget")); + w2.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(50), Object::Int(700), Object::Int(70), Object::Int(720)])); + let w2_ref = doc.add(Object::Dict(w2)); + + let mut radio = Dict::new(); + radio.set(b"FT".to_vec(), Object::name("Btn")); + radio.set(b"Ff".to_vec(), Object::Int(1 << 15)); + radio.set(b"T".to_vec(), PdfString::text("choice")); + // /Kids order is [w1 (right), w2 (left)], opposite to /R (Row) left-to-right order. + radio.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(w1_ref), Object::Ref(w2_ref)])); + let radio_ref = doc.add(Object::Dict(radio)); + doc.update_dict(w1_ref, |d| d.set(b"Parent".to_vec(), Object::Ref(radio_ref))).unwrap(); + doc.update_dict(w2_ref, |d| d.set(b"Parent".to_vec(), Object::Ref(radio_ref))).unwrap(); + + let mut text = Dict::new(); + text.set(b"FT".to_vec(), Object::name("Tx")); + text.set(b"Subtype".to_vec(), Object::name("Widget")); + text.set(b"T".to_vec(), PdfString::text("note")); + text.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(50), Object::Int(600), Object::Int(150), Object::Int(620)])); + let text_ref = doc.add(Object::Dict(text)); + + let mut page = Dict::new(); + page.set(b"Type".to_vec(), Object::name("Page")); + page.set(b"Parent".to_vec(), Object::Ref(pages_ref)); + page.set(b"Tabs".to_vec(), Object::name("R")); + page.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(w1_ref), Object::Ref(w2_ref), Object::Ref(text_ref)])); + doc.set(page_ref, Object::Dict(page)); + + let mut pages = Dict::new(); + pages.set(b"Type".to_vec(), Object::name("Pages")); + pages.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(page_ref)])); + pages.set(b"Count".to_vec(), Object::Int(1)); + doc.set(pages_ref, Object::Dict(pages)); + + let mut af = Dict::new(); + af.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(radio_ref), Object::Ref(text_ref)])); + let af_ref = doc.add(Object::Dict(af)); + + let mut catalog = Dict::new(); + catalog.set(b"Type".to_vec(), Object::name("Catalog")); + catalog.set(b"Pages".to_vec(), Object::Ref(pages_ref)); + catalog.set(b"AcroForm".to_vec(), Object::Ref(af_ref)); + let root_ref = doc.root().unwrap(); + doc.set(root_ref, Object::Dict(catalog)); + + // Move "note" earlier (before "choice"): "choice"'s own buttons should stay in their + // /R tab order (w2 then w1), not revert to /Kids order (w1 then w2). + move_in_tab_order(&mut doc, "note", true).unwrap(); + let annots = doc.get(page_ref).as_dict().and_then(|d| d.get(b"Annots")).and_then(Object::as_array).cloned().unwrap(); + assert_eq!(annots, vec![Object::Ref(text_ref), Object::Ref(w2_ref), Object::Ref(w1_ref)]); + + // Missing page and unplaced field return errors cleanly. + assert!(set_tab_order(&mut doc, &[99], TabOrder::Row).is_err()); + let mut unplaced = Dict::new(); + unplaced.set(b"FT".to_vec(), Object::name("Tx")); + unplaced.set(b"T".to_vec(), PdfString::text("orphan")); + let unplaced_ref = doc.add(Object::Dict(unplaced)); + doc.update_dict(af_ref, |d| { + d.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(radio_ref), Object::Ref(text_ref), Object::Ref(unplaced_ref)])); + }) + .unwrap(); + assert!(move_in_tab_order(&mut doc, "orphan", true).is_err()); + } +} diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index a49d35b0f4..f00d1cad9a 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -1701,6 +1701,43 @@ fn a_script_that_spells_the_markers_in_a_string_runs_as_a_script() { assert_eq!(field(&fields(&doc), "total").value, ["ran"]); } +/// An unsupported first AF call keeps the entire script and reaches the caller's interpreter. +#[test] +fn unsupported_first_validation_call_preserves_custom_script_dispatch() { + const JS: &str = "if (event.value < 50) AFRange_Validate(true, 10 + 5, false, 0); else AFRange_Validate(false, 0, true, 100);"; + struct RejectingInterpreter { + calls: usize, + } + impl Scripts for RejectingInterpreter { + fn run(&mut self, event: FieldEvent, script: &str, target: &Field, value: &str, _fields: &[Field]) -> ScriptResult { + assert_eq!(event, FieldEvent::Validate); + assert_eq!(script, JS); + assert_eq!(target.name, "amount"); + self.calls += 1; + ScriptResult { rc: false, value: value.into(), changes: Vec::new(), message: Some("custom validation ran".into()) } + } + } + let mut doc = one_page(); + add_field(&mut doc, 0, [50.0, 700.0, 250.0, 720.0], &NewField::Text { multiline: false }, Some("amount")).unwrap(); + set_value(&mut doc, "amount", &FieldValue::Text("20".into())).unwrap(); + set_field_script(&mut doc, "amount", "validate", Some(JS)).unwrap(); + let reopened = reopen(&doc); + for mut doc in [doc, reopened] { + let all = fields(&doc); + let actions = &field(&all, "amount").actions; + assert_eq!(actions.validate, af::Validate::None); + assert_eq!(actions.scripts.validate.as_deref(), Some(JS)); + assert!(actions.unsupported.contains(&"validate")); + let mut scripts = RejectingInterpreter { calls: 0 }; + assert_eq!( + set_value_with(&mut doc, "amount", &FieldValue::Text("10".into()), &mut scripts), + Err(FormError::Invalid("custom validation ran".into())) + ); + assert_eq!(scripts.calls, 1); + assert_eq!(field(&fields(&doc), "amount").value, ["20"]); + } +} + /// Renaming a field renames it in the calculations that read it: a sum dropped it, notation /// read it as 0 and a script read a field that no longer existed. #[test] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 24b0e3e150..cc5231262b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4770,8 +4770,8 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props", "form_fill"] -evidence = ["crates/forms/src/af/tests.rs::validation_and_calculation", "crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat", "crates/automation/tests/it/automation.rs::form_formats_and_calculations_through_tools"] -notes = "AFRange_Validate with Acrobat's messages" +evidence = ["crates/forms/src/af/tests.rs::validation_and_calculation", "crates/forms/src/tests.rs::formats_validation_and_calculations_run_like_acrobat", "crates/forms/src/tests.rs::unsupported_first_validation_call_preserves_custom_script_dispatch", "crates/automation/tests/it/automation.rs::form_formats_and_calculations_through_tools"] +notes = "AFRange_Validate with Acrobat's messages; unsupported arguments in the first genuine call preserve the complete custom script for the supplied interpreter, rather than selecting a later literal call (save/reopen and validation dispatch covered)." [[feature]] id = "form.af-simple-calculate" @@ -5268,7 +5268,7 @@ status = "shipped" commands = ["form.prepare"] tools = ["form_set_props"] evidence = ["crates/forms/src/tests.rs::locked_fields_only_take_unlocking", "crates/ui-egui/tests/it/prepare.rs::preview_fills_the_form_and_locked_fields_keep_their_properties", "crates/forms/src/tests.rs::renaming_a_kid_is_checked_against_the_full_name_it_takes", "crates/forms/src/tests.rs::renaming_a_top_level_field_is_still_checked_against_top_level_names", "crates/forms/src/tests.rs::dotted_field_names_are_made_with_parent_fields"] -notes = "rename in Field Properties ▸ General, which shows the full name; a dotted name (MASTER.first) is a full name and makes or joins parent fields, as Acrobat does, moving a renamed field with what it inherited (#2696); Prepare a form's table tool still refuses a period in the cell names it makes; a plain name renames the field within its parent, so the GUI cannot yet move a nested field to the top level; Locked sets the widgets' annotation Locked flag (/F bit 8) and greys out the properties until unlocked; values can still be filled in" +notes = "rename in Field Properties ▸ General, which shows the full name; a dotted name (MASTER.first) is a full name and makes or joins parent fields, as Acrobat does, moving a renamed field with what it inherited (#2696); inherited checkbox/radio export options survive a move and save/reopen, while choice fields keep only their own options; Prepare a form's table tool still refuses a period in the cell names it makes; a plain name renames the field within its parent, so the GUI cannot yet move a nested field to the top level; Locked sets the widgets' annotation Locked flag (/F bit 8) and greys out the properties until unlocked; values can still be filled in" [[feature]] id = "form.tab-order"