From ef03e1b1dfd30d299e52d3079c49cae534d5799f Mon Sep 17 00:00:00 2001 From: Bra1nZ <95569966+Bra1nZ@users.noreply.github.com> Date: Sat, 14 Nov 2026 01:19:29 +0300 Subject: [PATCH 1/3] fix(print): trust one side of a legacy V3 driver's paper metric Legacy V3 GDI drivers (HP LaserJet P1102) keep the paper metric in their DEVMODE extension and report MediaSizeWidth = 0 while the height is right, so verify_ticket refused with "a paper without a size (a roll?)" and nothing was sent (storytold/craft-libs#7230). verify_ticket now fails closed only when neither side is known or the request itself names no sheet; otherwise it goes on with the requested size, but only while the side the driver did report agrees with the request to a quarter of a millimetre. matching_paper gains a partial record as a candidate of last resort, and driver_ticket takes the wanted size from the sheet the job asked for, not from the zero record. The fallback is logged; log moves to the common dependencies so the warning compiles on Windows too (caught by the windows-gnu check). --- crates/print/Cargo.toml | 8 +-- crates/print/src/ticket.rs | 111 ++++++++++++++++++++++++++++++++++-- crates/print/src/windows.rs | 23 +++++++- 3 files changed, 132 insertions(+), 10 deletions(-) diff --git a/crates/print/Cargo.toml b/crates/print/Cargo.toml index 50fbb43d6..485fc8a93 100644 --- a/crates/print/Cargo.toml +++ b/crates/print/Cargo.toml @@ -13,14 +13,14 @@ description = "System PDF print spooler for the Craft apps: CUPS and Windows" # downstream GDI values a different Rust type identity even when the code is unchanged. # PDF loader updates below are independent of this public transport API. craft-winprint-gdi = { git = "https://github.com/storytold/winprint.rs", rev = "64e7afd4d1483512385012c340ecaaaf0f3beb7f" } +# Says why the printer list is empty when lpstat can't run (e.g. a Flatpak without CUPS tools), +# and why a job went on with the paper the request names while the driver reported only one side +# of it (storytold/craft-libs#7230). +log = "0.4" [lints] workspace = true -[target.'cfg(unix)'.dependencies] -# Says why the printer list is empty when lpstat can't run (e.g. a Flatpak without CUPS tools). -log = "0.4" - [target.'cfg(windows)'.dependencies] # Printing without starting anything (storytold/pdfcraft#1107): the printer list (EnumPrinters) # and the job (Windows.Data.Pdf drawn with Direct2D into the XPS print API, options as a diff --git a/crates/print/src/ticket.rs b/crates/print/src/ticket.rs index ddf2c3955..55a9c50ae 100644 --- a/crates/print/src/ticket.rs +++ b/crates/print/src/ticket.rs @@ -218,6 +218,11 @@ pub(crate) struct Paper<'a> { /// The driver paper for a sheet of `width` × `height` points (either way round): within a /// quarter of a millimetre, preferring the one named like the standard size, then a Print Schema /// keyword, then the closest; on a tie the driver's own order. Its index in `papers`. +/// +/// An entry the driver keeps only one side of (the other reads zero, a legacy host-based driver's +/// DEVMODE metric, storytold/craft-libs#7230) is a candidate of last resort (INV-4): only when no +/// full entry matched, only a Print Schema keyword entry, and only while its known side agrees +/// with the sheet, either way round. pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> Option { let (w, h) = (micrometres(width.min(height))?, micrometres(width.max(height))?); let names = paper_names(width, height); @@ -231,7 +236,13 @@ pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> O }) .collect(); candidates.sort_by_key(|(_, p, off)| (!names.iter().any(|n| names_paper(p.name, n)), !p.keyword, *off)); - candidates.first().map(|(i, _, _)| *i) + if let Some(index) = candidates.first().map(|(i, _, _)| *i) { + return Some(index); + } + let agrees = |side: u32| side.abs_diff(w) <= PAPER_TOLERANCE || side.abs_diff(h) <= PAPER_TOLERANCE; + papers + .iter() + .position(|p| p.keyword && (p.width == 0) != (p.height == 0) && (p.width == 0 || agrees(p.width)) && (p.height == 0 || agrees(p.height))) } fn named(node: roxmltree::Node<'_, '_>, local: &str) -> bool { @@ -287,14 +298,35 @@ fn integer(node: roxmltree::Node<'_, '_>, tag: &str, name: &str) -> Option /// paper (when `paper` is given, that one, to a quarter of a millimetre). Returns the paper's /// width and height in micrometres as the driver lists it. Only those schema fields are read: /// some drivers return empty vendor parameters that a stricter parser would refuse. +/// +/// A zero or unreadable side (`MediaSizeWidth` gone) is roll-like only while the request itself +/// names no sheet (INV-2): a legacy host-based driver keeps the paper metric in its DEVMODE +/// extension and reads back a zero side for a real sheet (HP LaserJet P1102, storytold/craft-libs#7230), so +/// with a sheet named the job falls back to the size the request asked for (INV-1, +/// fail-closed: every refusal here is before anything reaches the spooler). A side the driver +/// did report must agree with the sheet asked for, either way round (INV-3); both sides gone is +/// refused whatever the request names (INV-5). The fallback is logged (INV-7). pub(crate) fn verify_ticket(xml: &str, paper: Option<(u32, u32)>, landscape: bool) -> Result<(u32, u32), Error> { let document = crate::xml::parse(xml).map_err(spool_error)?; let feature = |name: &str| document.root_element().children().find(|n| n.has_tag_name((NS_PSF, "Feature")) && named(*n, name)); let media = feature("PageMediaSize").ok_or_else(|| spool_error("the printer's driver did not confirm the paper size; nothing was sent"))?; - let size = integer(media, "ScoredProperty", "MediaSizeWidth") - .zip(integer(media, "ScoredProperty", "MediaSizeHeight")) - .filter(|(w, h)| *w > 0 && *h > 0) - .ok_or_else(|| spool_error("the printer's driver gave a paper without a size (a roll?); nothing was sent"))?; + let width = integer(media, "ScoredProperty", "MediaSizeWidth").filter(|w| *w > 0); + let height = integer(media, "ScoredProperty", "MediaSizeHeight").filter(|h| *h > 0); + let without_a_size = spool_error("the printer's driver gave a paper without a size (a roll?); nothing was sent"); + let size = match (width, height, paper) { + (Some(w), Some(h), _) => (w, h), + (None, None, _) | (_, _, None) => return Err(without_a_size), + (Some(reported), None, Some(wanted)) | (None, Some(reported), Some(wanted)) => { + let (w, h) = wanted; + // Either way round: the one side the driver did report against the sheet's width or height. + let agrees = |other: u32| reported.abs_diff(other) <= PAPER_TOLERANCE; + if !agrees(w) && !agrees(h) { + return Err(spool_error("the printer's driver changed the paper size; nothing was sent (choose a paper size it has)")); + } + log::warn!("the printer's driver reported only {reported} µm of the paper; going on with the requested {w} × {h} µm"); + (w, h) + } + }; if let Some((w, h)) = paper { let same = |a: (u32, u32), b: (u32, u32)| a.0.abs_diff(b.0) <= PAPER_TOLERANCE && a.1.abs_diff(b.1) <= PAPER_TOLERANCE; if !same(size, (w, h)) && !same(size, (h, w)) { @@ -1058,6 +1090,55 @@ mod tests { assert!(verify_ticket(", height: Option) -> String { + let side = |name: &str, value: Option| { + value.map_or(String::new(), |v| { + format!(r#"{v}"#) + }) + }; + format!( + r#" +{}{} + + +"#, + side("MediaSizeWidth", width), + side("MediaSizeHeight", height) + ) + } + + /// storytold/craft-libs#7230: the driver reports one side of the paper as zero for a real + /// sheet. With the sheet the request names, the job goes on with the size it asked for; a + /// side the driver did report must still agree with it, and a request that names no sheet is + /// refused as before. + #[test] + fn a_legacy_v3_drivers_zero_side_falls_back_to_the_requested_paper() { + let a4 = (210_000, 297_000); + assert_eq!(verify_ticket(&validated_partial(None, Some(297_000)), Some(a4), false), Ok(a4)); + assert_eq!(verify_ticket(&validated_partial(Some(0), Some(297_000)), Some(a4), false), Ok(a4), "a zero side is a missing one"); + assert_eq!(verify_ticket(&validated_partial(Some(297_000), None), Some(a4), false), Ok(a4), "either way round"); + // 0.5 pt (176 µm) inside the tolerance still agrees. + assert_eq!(verify_ticket(&validated_partial(Some(0), Some(297_176)), Some(a4), false), Ok(a4)); + assert_eq!(verify_ticket(&validated_partial(Some(0), Some(297_250)), Some(a4), false), Ok(a4), "exactly the tolerance"); + // A side that disagrees means the driver changed the paper, not that it has a roll. + let changed = verify_ticket(&validated_partial(Some(0), Some(279_400)), Some(a4), false).unwrap_err(); + assert!(changed.to_string().contains("changed the paper size"), "{changed}"); + assert!(!changed.to_string().contains("roll"), "{changed}"); + assert!(verify_ticket(&validated_partial(Some(0), Some(297_251)), Some(a4), false).is_err(), "one µm past the tolerance"); + // Without a sheet named the zero side stays roll-like. + assert!(verify_ticket(&validated_partial(Some(0), Some(297_000)), None, false).is_err()); + assert!(verify_ticket(&validated_partial(None, Some(297_000)), None, false).is_err()); + // Both sides gone is refused whatever the request names. + for wanted in [Some(a4), None] { + for (width, height) in [(None, None), (Some(0), Some(0))] { + let refused = verify_ticket(&validated_partial(width, height), wanted, false).unwrap_err(); + assert!(refused.to_string().contains("roll"), "{refused}"); + } + } + } + /// The namespace of the Brother driver's own keywords. const BROTHER: &str = "http://schemas.brother.info/mfc/printing/2006/11/printschemakeywords"; @@ -1548,6 +1629,26 @@ mod tests { assert!(!names_paper(None, "A4")); } + /// A paper the driver keeps one side of (storytold/craft-libs#7230) is a candidate of last + /// resort only: no full entry matched, a Print Schema keyword entry, and its known side + /// agrees with the sheet. + #[test] + fn a_partially_known_paper_is_a_candidate_of_last_resort_only() { + let paper = |width, height, name, keyword| Paper { width, height, name, keyword }; + let partial = paper(0, 297_000, Some("A4"), true); + let alone = [partial.clone()]; + assert_eq!(matching_paper(&alone, 595.28, 841.89), Some(0), "no full entry: the partial one is taken"); + // A full candidate, however it spells its option, wins over the partial one. + let with_full = [partial, paper(210_100, 297_000, Some("A4"), false)]; + assert_eq!(matching_paper(&with_full, 595.28, 841.89), Some(1), "the full entry wins"); + // A vendor's own entry is no partial candidate. + assert_eq!(matching_paper(&[paper(0, 297_000, Some("A4"), false)], 595.28, 841.89), None); + // A partial entry whose known side disagrees is no candidate. + assert_eq!(matching_paper(&[paper(0, 279_400, Some("Letter"), true)], 595.28, 841.89), None); + // Both sides gone: nothing known to agree with. + assert_eq!(matching_paper(&[paper(0, 0, Some("A4"), true)], 595.28, 841.89), None); + } + #[test] fn the_unprintable_margin_comes_from_the_capabilities() { let caps = |x: u32, y: u32, w: u32, h: u32| { diff --git a/crates/print/src/windows.rs b/crates/print/src/windows.rs index d43e3981a..7ba238808 100644 --- a/crates/print/src/windows.rs +++ b/crates/print/src/windows.rs @@ -224,7 +224,28 @@ fn driver_ticket(device: &PrinterDevice, job: &Job, sheet: (f64, f64), landscape .collect(); let media = ticket::matching_paper(&papers, sheet.0, sheet.1).and_then(|i| medias.get(i)); let wanted = match (media, job.scaling) { - (Some(m), _) => Some((m.size().width_in_micron(), m.size().height_in_micron())), + (Some(m), _) => { + let size = (m.size().width_in_micron(), m.size().height_in_micron()); + // INV-6: a driver that keeps one side of the paper metric in its DEVMODE extension + // (storytold/craft-libs#7230) reports a zero side; the wanted size then is the sheet + // the job asked for, not the zero record, which would poison the cross-check and hand + // `sheets::plan` a width of zero. + if size.0 == 0 || size.1 == 0 { + match (ticket::micrometres(sheet.0), ticket::micrometres(sheet.1)) { + (Some(w), Some(h)) => Some((w, h)), + _ => { + return Err(spool_error(format!( + "{} has no {:.2} × {:.2} inch paper; choose a paper size it has, or add this one in its driver", + device.name(), + sheet.0 / 72.0, + sheet.1 / 72.0 + ))); + } + } + } else { + Some(size) + } + } (None, Scaling::ShrinkToFit) => None, (None, Scaling::Exact) => { return Err(spool_error(format!( From 82e3341018c65aeb1370cb6b6411148e69c96676 Mon Sep 17 00:00:00 2001 From: Bra1nZ <95569966+Bra1nZ@users.noreply.github.com> Date: Sat, 14 Nov 2026 12:55:59 +0300 Subject: [PATCH 2/3] fix(print): match the requested paper identity among partial records and resolve parameter references Two review findings on #1225: A partial paper record was accepted as a candidate of last resort on its known side alone, so a partial Letter and a partial Legal sharing a width both agreed with a Legal request and the first one in the driver's order won - the ticket could select Letter while the layout assumed Legal. Partial candidates now resolve ambiguity the way full ones do: one whose name matches the requested standard sheet wins; an ambiguity that leaves more than one (or none) is not resolved and nothing is selected. integer() read only a direct Value, so a paper dimension supplied through a ParameterRef read as missing and the fallback silently substituted the requested size for a ticket that named a different one. The resolution logic the driver's ticket reader already had moves into a shared resolved_value() helper, and integer() uses it: a referenced dimension now either agrees or refuses. --- crates/print/src/ticket.rs | 141 +++++++++++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 28 deletions(-) diff --git a/crates/print/src/ticket.rs b/crates/print/src/ticket.rs index 55a9c50ae..8e1171f71 100644 --- a/crates/print/src/ticket.rs +++ b/crates/print/src/ticket.rs @@ -222,7 +222,10 @@ pub(crate) struct Paper<'a> { /// An entry the driver keeps only one side of (the other reads zero, a legacy host-based driver's /// DEVMODE metric, storytold/craft-libs#7230) is a candidate of last resort (INV-4): only when no /// full entry matched, only a Print Schema keyword entry, and only while its known side agrees -/// with the sheet, either way round. +/// with the sheet, either way round. Several such entries that all agree (a Letter and a Legal +/// option share a width) are told apart by name, as full candidates are: the one named like the +/// requested standard sheet wins, and when that leaves more than one, or none, nothing is chosen — +/// a guess here would lay the sheets out for the wrong paper. pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> Option { let (w, h) = (micrometres(width.min(height))?, micrometres(width.max(height))?); let names = paper_names(width, height); @@ -240,9 +243,23 @@ pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> O return Some(index); } let agrees = |side: u32| side.abs_diff(w) <= PAPER_TOLERANCE || side.abs_diff(h) <= PAPER_TOLERANCE; - papers + let partials: Vec = papers .iter() - .position(|p| p.keyword && (p.width == 0) != (p.height == 0) && (p.width == 0 || agrees(p.width)) && (p.height == 0 || agrees(p.height))) + .enumerate() + .filter(|(_, p)| p.keyword && (p.width == 0) != (p.height == 0) && (p.width == 0 || agrees(p.width)) && (p.height == 0 || agrees(p.height))) + .map(|(i, _)| i) + .collect(); + match partials.as_slice() { + [only] => Some(*only), + // Ambiguous by agreement alone: a partial named like the requested standard sheet wins. + _ => { + let named: Vec = partials.iter().copied().filter(|i| names.iter().any(|n| names_paper(papers[*i].name, n))).collect(); + match named.as_slice() { + [only] => Some(*only), + _ => None, + } + } + } } fn named(node: roxmltree::Node<'_, '_>, local: &str) -> bool { @@ -285,12 +302,32 @@ pub(crate) fn two_sided(xml: &str) -> Option { ) } -/// The integer value of the Print Schema element under `node` with this keyword name. -fn integer(node: roxmltree::Node<'_, '_>, tag: &str, name: &str) -> Option { +/// The `Value` of the Print Schema element `element`: its own, or one a `ParameterRef` names +/// through the `ParameterInit` under the document's root that sets it (a driver may keep a paper +/// side or a resolution this way). `None` when the element has neither, or names a parameter the +/// ticket does not set. +fn resolved_value<'a>(document: &'a roxmltree::Document<'a>, element: roxmltree::Node<'a, 'a>) -> Option> { + let value = element.children().find(|v| v.has_tag_name((NS_PSF, "Value")) || v.has_tag_name((NS_PSF, "ParameterRef")))?; + if value.has_tag_name((NS_PSF, "Value")) { + return Some(value); + } + let parameter = expanded(value, value.attribute("name")?); + document + .root_element() + .children() + .filter(|p| p.has_tag_name((NS_PSF, "ParameterInit"))) + .find(|p| p.attribute("name").is_some_and(|name| expanded(*p, name) == parameter))? + .children() + .find(|v| v.has_tag_name((NS_PSF, "Value"))) +} + +/// The integer value of the Print Schema element under `node` with this keyword name, direct or +/// through a `ParameterRef` the ticket resolves ([`resolved_value`]). +fn integer(document: &roxmltree::Document<'_>, node: roxmltree::Node<'_, '_>, tag: &str, name: &str) -> Option { node.descendants() .find(|n| n.has_tag_name((NS_PSF, tag)) && named(*n, name)) - .and_then(|n| n.children().find(|v| v.has_tag_name((NS_PSF, "Value")))) - .and_then(|n| n.text()) + .and_then(|element| resolved_value(document, element)) + .and_then(|value| value.text()) .and_then(|v| v.trim().parse::().ok()) } @@ -310,8 +347,8 @@ pub(crate) fn verify_ticket(xml: &str, paper: Option<(u32, u32)>, landscape: boo let document = crate::xml::parse(xml).map_err(spool_error)?; let feature = |name: &str| document.root_element().children().find(|n| n.has_tag_name((NS_PSF, "Feature")) && named(*n, name)); let media = feature("PageMediaSize").ok_or_else(|| spool_error("the printer's driver did not confirm the paper size; nothing was sent"))?; - let width = integer(media, "ScoredProperty", "MediaSizeWidth").filter(|w| *w > 0); - let height = integer(media, "ScoredProperty", "MediaSizeHeight").filter(|h| *h > 0); + let width = integer(&document, media, "ScoredProperty", "MediaSizeWidth").filter(|w| *w > 0); + let height = integer(&document, media, "ScoredProperty", "MediaSizeHeight").filter(|h| *h > 0); let without_a_size = spool_error("the printer's driver gave a paper without a size (a roll?); nothing was sent"); let size = match (width, height, paper) { (Some(w), Some(h), _) => (w, h), @@ -416,7 +453,9 @@ pub(crate) fn one_job_sides(xml: &str) -> Option { _ => return None, } } - if features(&["JobNUpAllDocumentsContiguously", "DocumentNUp"]).any(|f| integer(f, "ScoredProperty", "PagesPerSheet").is_some_and(|n| n > 1)) { + if features(&["JobNUpAllDocumentsContiguously", "DocumentNUp"]) + .any(|f| integer(&document, f, "ScoredProperty", "PagesPerSheet").is_some_and(|n| n > 1)) + { return None; } if features(&["JobBindAllDocuments", "DocumentBinding", "JobStapleAllDocuments", "DocumentStaple"]) @@ -453,7 +492,7 @@ pub(crate) fn verify_job_options(xml: &str, job: &Job, pages: usize, offered: Of }; let refused = |field: &str| spool_error(format!("the printer's driver did not retain the requested {field}; nothing was sent")); let copies = job.copies.clamp(1, 999); - if integer(root, "ParameterInit", "JobCopiesAllDocuments") != Some(copies) { + if integer(&document, root, "ParameterInit", "JobCopiesAllDocuments") != Some(copies) { return Err(refused("copy count")); } if copies > 1 && pages > 1 && !option("DocumentCollate").is_some_and(|n| named(n, if job.collate { "Collated" } else { "Uncollated" })) { @@ -529,19 +568,7 @@ fn feature_option(document: &roxmltree::Document<'_>, feature: &str) -> Option value, - reference => { - let parameter = expanded(reference, reference.attribute("name")?); - document - .root_element() - .children() - .filter(|p| p.has_tag_name((NS_PSF, "ParameterInit"))) - .find(|p| p.attribute("name").is_some_and(|name| expanded(*p, name) == parameter))? - .children() - .find(|v| v.has_tag_name((NS_PSF, "Value")))? - } - }; + let value = resolved_value(document, n)?; let text = value.text()?.trim(); let value = match text.parse::() { _ if is_qname_value(value) => expanded(value, text), @@ -798,11 +825,11 @@ pub(crate) fn prefixes_unlike_the_drivers(ticket: &str, driver: &str) -> Option< pub(crate) fn imageable_margin(capabilities_xml: &str) -> Option { let document = crate::xml::parse(capabilities_xml).ok()?; let imageable = document.root_element().children().find(|n| n.has_tag_name((NS_PSF, "Property")) && named(*n, "PageImageableSize"))?; - let width = integer(imageable, "Property", "ImageableSizeWidth")?; - let height = integer(imageable, "Property", "ImageableSizeHeight")?; + let width = integer(&document, imageable, "Property", "ImageableSizeWidth")?; + let height = integer(&document, imageable, "Property", "ImageableSizeHeight")?; let area = imageable.children().find(|n| n.has_tag_name((NS_PSF, "Property")) && named(*n, "ImageableArea"))?; - let (x, y) = (integer(area, "Property", "OriginWidth")?, integer(area, "Property", "OriginHeight")?); - let (w, h) = (integer(area, "Property", "ExtentWidth")?, integer(area, "Property", "ExtentHeight")?); + let (x, y) = (integer(&document, area, "Property", "OriginWidth")?, integer(&document, area, "Property", "OriginHeight")?); + let (w, h) = (integer(&document, area, "Property", "ExtentWidth")?, integer(&document, area, "Property", "ExtentHeight")?); let right = width.checked_sub(x)?.checked_sub(w)?; let bottom = height.checked_sub(y)?.checked_sub(h)?; Some(x.max(y).max(right).max(bottom)) @@ -1139,6 +1166,40 @@ mod tests { } } + /// A validated ticket whose width the option refers to a parameter the ticket sets + /// (`MediaSizeWidth` a `ParameterRef`, storytold/craft-libs#1225), as a driver writes one. + fn validated_referenced(width: u32, height: u32) -> String { + format!( + r#" +{width} + + +{height} + + + +"# + ) + } + + /// A paper side a driver keeps in a `ParameterRef` is resolved through the `ParameterInit` + /// the ticket sets, and checked like a direct one: a referenced width that disagrees with the + /// sheet the request names is refused, one that agrees is taken. + #[test] + fn a_referenced_paper_side_is_resolved_through_the_parameter_it_names() { + let a4 = (210_000, 297_000); + assert_eq!(verify_ticket(&validated_referenced(210_000, 297_000), Some(a4), false), Ok(a4)); + assert_eq!(verify_ticket(&validated_referenced(209_750, 297_000), Some(a4), false), Ok((209_750, 297_000)), "exactly the tolerance"); + // A referenced size that disagrees is refused, not rewritten from the request. + let changed = verify_ticket(&validated_referenced(400_000, 297_000), Some(a4), false).unwrap_err(); + assert!(changed.to_string().contains("changed the paper size"), "{changed}"); + // A reference to a parameter the ticket does not set is a missing side, which falls back + // to the requested paper like any unreadable one. + let unset = + validated_referenced(400_000, 297_000).replace(r#"name="psk:PageMediaSizeWidth"> Date: Sat, 14 Nov 2026 13:16:41 +0300 Subject: [PATCH 3/3] fix(print): reject a partial record whose known name contradicts the sheet A review finding on #1225: a single agreeing partial record was taken on its known side alone, so a Legal request could take a keyword entry named Letter reporting (215_900, 0) - the ticket then selects Letter while the layout assumes Legal. A single partial record now refuses when its known name doesn't match the requested standard sheet; an unnamed one is still taken on agreement alone. The name check exposed names_paper accepting a word modifier as a dimension: "A4 Extra" matched "A4" because the suffix rule only looked at the first character, a space. The rule now reads the word after the separator: a letter is a modifier ("A4 Extra" is another paper), a digit is the sheet's dimensions ("A4 210 x 297 mm"), a digit or letter right against the name is a different name ("ARCH E1", "A4Extra"). --- crates/print/src/ticket.rs | 69 +++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/print/src/ticket.rs b/crates/print/src/ticket.rs index 8e1171f71..e6224df69 100644 --- a/crates/print/src/ticket.rs +++ b/crates/print/src/ticket.rs @@ -197,11 +197,24 @@ pub(crate) fn paper_names(width: f64, height: f64) -> &'static [&'static str] { } /// Whether a driver's paper name is `name`: it starts with it, ignoring case, and the name ends -/// there ("A4 210 x 297 mm", not "A4 Extra" for "A4" or "ARCH E1" for "ARCH E"). +/// there — either nothing follows ("arch d"), or a separator and the sheet's dimensions ("A4 +/// 210 x 297 mm"). After a separator, dimensions are a digit: a word is a modifier, another +/// paper ("A4 Extra", "A4 (Extra)"), and anything joined to the name without a separator is +/// another paper too ("A4Extra", "ARCH E1" for "ARCH E"). pub(crate) fn names_paper(display: Option<&str>, name: &str) -> bool { let Some(display) = display else { return false }; - display.get(..name.len()).is_some_and(|start| start.eq_ignore_ascii_case(name)) - && display.get(name.len()..).is_some_and(|rest| rest.chars().next().is_none_or(|c| !c.is_alphanumeric())) + let Some(rest) = display.get(name.len()..) else { return false }; + if !display.get(..name.len()).is_some_and(|start| start.eq_ignore_ascii_case(name)) { + return false; + } + let after = rest.trim_start_matches(|c: char| !c.is_alphanumeric()); + match after.chars().next() { + // The name ends here, or a separator introduces the sheet's dimensions. + None => true, + Some(c) if !c.is_alphabetic() => after.len() < rest.len(), + // A word, directly after the name or after a separator, is a modifier: another paper. + Some(_) => false, + } } /// A driver paper offered for the job: its size in micrometres (as the driver lists it), its @@ -222,10 +235,13 @@ pub(crate) struct Paper<'a> { /// An entry the driver keeps only one side of (the other reads zero, a legacy host-based driver's /// DEVMODE metric, storytold/craft-libs#7230) is a candidate of last resort (INV-4): only when no /// full entry matched, only a Print Schema keyword entry, and only while its known side agrees -/// with the sheet, either way round. Several such entries that all agree (a Letter and a Legal -/// option share a width) are told apart by name, as full candidates are: the one named like the -/// requested standard sheet wins, and when that leaves more than one, or none, nothing is chosen — -/// a guess here would lay the sheets out for the wrong paper. +/// with the sheet, either way round. A single such entry is still refused when its own name is +/// known and contradicts the requested standard sheet (INV-4): a known conflicting name is +/// evidence against the choice, and failing closed is safer than a guess. Without a name there is +/// nothing contradicting, so the agreement alone is enough. Several such entries that all agree +/// (a Letter and a Legal option share a width) are told apart by name, as full candidates are: +/// the one named like the requested standard sheet wins, and when that leaves more than one, or +/// none, nothing is chosen — a guess here would lay the sheets out for the wrong paper. pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> Option { let (w, h) = (micrometres(width.min(height))?, micrometres(width.max(height))?); let names = paper_names(width, height); @@ -250,7 +266,13 @@ pub(crate) fn matching_paper(papers: &[Paper<'_>], width: f64, height: f64) -> O .map(|(i, _)| i) .collect(); match partials.as_slice() { - [only] => Some(*only), + [only] => { + // INV-4: a known name that contradicts the requested standard sheet is evidence + // against this single partial record — refuse it; without a name, the known side's + // agreement is all the evidence there is, and the record stands. + let contradicts = papers[*only].name.is_some_and(|name| !names.iter().any(|n| names_paper(Some(name), n))); + (!contradicts).then_some(*only) + } // Ambiguous by agreement alone: a partial named like the requested standard sheet wins. _ => { let named: Vec = partials.iter().copied().filter(|i| names.iter().any(|n| names_paper(papers[*i].name, n))).collect(); @@ -1686,6 +1708,10 @@ mod tests { assert!(!names_paper(Some("ANSI D"), "ARCH D")); assert!(!names_paper(Some("ARCH E1"), "ARCH E")); assert!(!names_paper(Some("A4Extra"), "A4")); + // storytold/craft-libs#1225: a word after a separator is a modifier, not dimensions. + assert!(!names_paper(Some("A4 Extra"), "A4")); + assert!(!names_paper(Some("A4 (Extra)"), "A4")); + assert!(names_paper(Some("A4 210 x 297 mm"), "A4")); assert!(!names_paper(Some("Ä"), "A4"), "never slices inside a character"); assert!(!names_paper(None, "A4")); } @@ -1711,12 +1737,37 @@ mod tests { } /// One partial paper whose known side agrees is still taken, named like the requested - /// standard sheet or not. + /// standard sheet or not. The known side agrees only to a quarter of a millimetre: exactly + /// the tolerance is taken, one micrometre past it is another paper. #[test] fn a_single_partially_known_paper_that_agrees_is_still_taken() { let paper = |width, height, name, keyword| Paper { width, height, name, keyword }; assert_eq!(matching_paper(&[paper(215_900, 0, Some("Legal 215.9 x 355.6 mm"), true)], 612.0, 1008.0), Some(0), "named like the sheet"); assert_eq!(matching_paper(&[paper(215_900, 0, None, true)], 612.0, 1008.0), Some(0), "or without a name at all"); + assert_eq!(matching_paper(&[paper(0, 297_250, None, true)], 595.28, 841.89), Some(0), "exactly the tolerance"); + assert_eq!(matching_paper(&[paper(0, 297_251, None, true)], 595.28, 841.89), None, "one µm past the tolerance"); + } + + /// storytold/craft-libs#1225 (Code Rabbit, second round): a single partial paper whose known + /// side agrees is still refused when its own name is known and contradicts the requested + /// standard sheet — the width the driver kept is shared by Letter and Legal, and a known + /// Letter name is evidence against choosing it for a Legal sheet (INV-4). "Extra" is a + /// modifier word, another paper: a known "Letter Extra" name contradicts a Letter sheet too. + /// Without a name there is nothing contradicting, and the agreeing entry is taken as before. + #[test] + fn a_single_partially_known_paper_whose_known_name_contradicts_the_sheet_is_refused() { + let paper = |width, height, name, keyword| Paper { width, height, name, keyword }; + assert_eq!( + matching_paper(&[paper(215_900, 0, Some("Letter 215.9 x 279.4 mm"), true)], 612.0, 1008.0), + None, + "a known Letter name is not a Legal sheet" + ); + assert_eq!( + matching_paper(&[paper(215_900, 0, Some("Letter Extra 215.9 x 431.8 mm"), true)], 612.0, 792.0), + None, + "a known \"Letter Extra\" name is not a Letter sheet" + ); + assert_eq!(matching_paper(&[paper(215_900, 0, None, true)], 612.0, 1008.0), Some(0), "an unnamed twin stands, the agreement alone"); } /// storytold/craft-libs#1225: two partial papers that both agree (Letter and Legal share a