fix(ui): header, footer and watermark dialogs refuse undrawable text before OK (#2013) (#2107)

The dialog previews in the interface font, which draws Cyrillic, while
applying draws with the standard fonts and refused it after OK.
HeaderFooter::first_undrawable and Watermark::first_undrawable are now
the one check: applying refuses through them, and the dialog shows the
Edit Text warning and disables OK on the same answer.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
jonathanwxh-cell
2026-10-15 02:44:49 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 178d9a5882
commit b655208aaf
4 changed files with 101 additions and 25 deletions
+27 -15
View File
@@ -73,6 +73,17 @@ impl Default for HeaderFooter {
}
}
impl HeaderFooter {
/// The first character [`add_header_footer`] would refuse, because the standard fonts can't
/// draw it (#125). Tokens only add ASCII (Bates prefixes and suffixes are part of the
/// template), so checking the templates, split as `expand`'s output is, covers every page
/// (and a `\r` that would be drawn is refused too). The dialog asks this too, so its preview
/// never offers text that applying refuses (#2013).
pub fn first_undrawable(&self) -> Option<char> {
self.text.iter().flat_map(|t| t.lines()).find_map(pdfcraft_fonts::first_non_win_ansi)
}
}
/// A picture for a background or watermark (Acrobat: Source ▸ File): an XObject already in
/// the document, an image (drawn into a unit square) or a form (a page of a PDF).
#[derive(Clone, Copy, Debug, PartialEq)]
@@ -104,6 +115,17 @@ pub struct Watermark {
pub offset: [f64; 2],
}
impl Watermark {
/// The first character of a text watermark [`add_watermark`] would refuse, because the
/// standard fonts can't draw it (#125). A picture watermark draws no text.
pub fn first_undrawable(&self) -> Option<char> {
match self.source {
Some(_) => None,
None => self.text.trim().lines().find_map(pdfcraft_fonts::first_non_win_ansi),
}
}
}
impl Default for Watermark {
fn default() -> Self {
Self {
@@ -383,16 +405,6 @@ fn begin(kind: MarkKind, subtype: &str, matrix: [f64; 6]) -> String {
const END: &str = "EMC\nQ\n";
/// Refuses page text the standard-14 fonts (WinAnsiEncoding) can't draw: written anyway, each
/// such character would become a `?` on the page (#125). `lines` must split the text the way the
/// caller draws it, so a `\r` that would be drawn is refused too.
pub(crate) fn drawable<'a>(lines: impl IntoIterator<Item = &'a str>) -> Result<(), EditError> {
match lines.into_iter().find_map(pdfcraft_fonts::first_non_win_ansi) {
Some(c) => Err(undrawable(c)),
None => Ok(()),
}
}
/// The refusal for page text with `c`, which nothing here can draw.
pub(crate) fn undrawable(c: char) -> EditError {
EditError::Invalid(undrawable_message(c))
@@ -418,9 +430,9 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter,
if hf.text.iter().all(|t| t.trim().is_empty()) {
return Err(EditError::Invalid("type the header or footer text first".into()));
}
// Tokens only add ASCII (Bates prefixes and suffixes are part of the template), so checking
// the templates, split as `expand`'s output is, covers every page.
hf.text.iter().try_for_each(|t| drawable(t.lines()))?;
if let Some(c) = hf.first_undrawable() {
return Err(undrawable(c));
}
if !(hf.font_size.is_finite() && hf.font_size > 0.0 && hf.font_size <= 200.0 && hf.margins.iter().all(|m| m.is_finite() && *m >= 0.0)) {
return Err(EditError::Invalid("invalid font size or margins".into()));
}
@@ -498,8 +510,8 @@ pub fn add_watermark(doc: &mut Document, pages: &[usize], wm: &Watermark, replac
if text.is_empty() && wm.source.is_none() {
return Err(EditError::Invalid("type the watermark text first".into()));
}
if wm.source.is_none() {
drawable(text.lines())?;
if let Some(c) = wm.first_undrawable() {
return Err(undrawable(c));
}
if !(wm.opacity.is_finite()
&& wm.rotation.is_finite()
+31
View File
@@ -2318,6 +2318,37 @@ fn hebrew_letters_the_page_font_lacks_use_the_hebrew_face() {
assert_eq!(text::text_blocks(&reopen(&doc), 0).unwrap()[0].text, "עברית מלאה");
}
/// #2013: the dialog asks `first_undrawable` before offering OK, so it must refuse exactly what
/// applying refuses: a Cyrillic footer was previewed (in the interface font) and then failed.
#[test]
fn header_and_watermark_drawability_matches_what_applying_refuses() {
let footer = |t: &str| HeaderFooter {
text: [String::new(), String::new(), String::new(), String::new(), String::new(), t.into()],
..HeaderFooter::default()
};
for (text, refused) in [
("Страница <<1>> из 1 / Page <<1>> of 1", Some('С')),
("TEST-001 <<Page 1 of n>>", None),
("Café — 5€ ™ <<Bates Number#6#1#ACME-#>>", None),
("Left\r\nRight", None),
("A\rB", Some('\r')),
("line one\nΕλληνικά", Some('Ε')),
] {
let hf = footer(text);
assert_eq!(hf.first_undrawable(), refused, "{text:?}");
let mut doc = fixture();
let applied = add_header_footer(&mut doc, &[0], &hf, false, &cx());
match refused {
Some(c) => assert_eq!(applied.unwrap_err().to_string(), undrawable_message(c), "{text:?}"),
None => applied.unwrap(),
}
let wm = Watermark { text: text.into(), ..Watermark::default() };
assert_eq!(wm.first_undrawable(), refused, "{text:?}");
let applied = add_watermark(&mut fixture(), &[0], &wm, false);
assert_eq!(applied.err().map(|e| e.to_string()), refused.map(undrawable_message), "{text:?}");
}
}
/// #125: page-content tools refuse text the standard fonts can't draw instead of writing `?`,
/// and leave the document untouched.
#[test]
+1 -1
View File
@@ -12,7 +12,7 @@ use crate::theme::Tokens;
use crate::widgets;
/// The colour of the warning under text the standard fonts can't draw.
const UNDRAWABLE: Color32 = Color32::from_rgb(0xD7, 0x37, 0x3F);
pub(crate) const UNDRAWABLE: Color32 = Color32::from_rgb(0xD7, 0x37, 0x3F);
/// The first character of a draft that can't be drawn, as the engine decides it (it refuses
/// such text, #125): outside the standard fonts and not shaped with the Arabic face.
+42 -9
View File
@@ -384,20 +384,29 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M
}
});
});
// Text the standard fonts can't draw is refused on apply (#125), so it's refused here, with the
// preview (drawn in the interface font, which can draw it) no longer the only answer (#2013).
let undrawable = undrawable(kind, d);
if let Some(c) = undrawable {
ui.add_space(6.0);
ui.label(egui::RichText::new(crate::content_ui::undrawable_message(c)).small().color(crate::content_ui::UNDRAWABLE));
}
ui.add_space(10.0);
let (mut apply, mut cancel) = (false, false);
ui.with_layout(Layout::right_to_left(Align::Center), |ui| {
let ready = match kind {
MarkKind::HeaderFooter => d.hf.text.iter().any(|x| !x.trim().is_empty()),
MarkKind::Watermark => {
if d.use_file {
d.file.is_some()
} else {
!d.wm.text.trim().is_empty()
let ready = undrawable.is_none()
&& match kind {
MarkKind::HeaderFooter => d.hf.text.iter().any(|x| !x.trim().is_empty()),
MarkKind::Watermark => {
if d.use_file {
d.file.is_some()
} else {
!d.wm.text.trim().is_empty()
}
}
MarkKind::Background => !d.use_file || d.file.is_some(),
}
MarkKind::Background => !d.use_file || d.file.is_some(),
} && !d.range.pages(count).is_empty();
&& !d.range.pages(count).is_empty();
if ui.add_enabled_ui(ready, |ui| widgets::pill_button(ui, tl!("OK"), true)).inner.clicked() {
apply = true;
}
@@ -425,6 +434,16 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M
(apply, cancel)
}
/// The first character of the dialog's text that applying would refuse (the standard fonts can't
/// draw it), as the engine decides it. Pictures draw no text.
fn undrawable(kind: MarkKind, d: &MarksDraft) -> Option<char> {
match kind {
MarkKind::HeaderFooter => d.hf.first_undrawable(),
MarkKind::Watermark if !d.use_file => d.wm.first_undrawable(),
MarkKind::Watermark | MarkKind::Background => None,
}
}
fn mark_file(d: &MarksDraft) -> Option<pdfcraft_engine::MarkFile> {
let (name, bytes) = d.file.clone().filter(|_| d.use_file)?;
Some(pdfcraft_engine::MarkFile { name, bytes, page: d.file_page.max(1) - 1 })
@@ -478,6 +497,20 @@ pub fn edit(d: &MarksDraft, kind: MarkKind, count: usize) -> Edit {
mod tests {
use super::*;
#[test]
fn text_applying_would_refuse_is_caught_in_the_dialog() {
let mut d = MarksDraft::default();
d.hf.text[5] = "Страница <<1>> из <<n>> / Page <<1>> of <<n>>".into();
assert_eq!(undrawable(MarkKind::HeaderFooter, &d), Some('С'), "#2013: refused before OK, not after");
d.hf.text[5] = "Page <<1>> of <<n>> — café".into();
assert_eq!(undrawable(MarkKind::HeaderFooter, &d), None);
d.wm.text = "ΑΠΟΡΡΗΤΟ".into();
assert_eq!(undrawable(MarkKind::Watermark, &d), Some('Α'));
d.use_file = true;
assert_eq!(undrawable(MarkKind::Watermark, &d), None, "a picture watermark draws no text");
assert_eq!(undrawable(MarkKind::Background, &d), None);
}
#[test]
fn page_ranges_with_subsets() {
let r = PageRange::default();