Never crash & persist: Make Current Properties Default and page-display choices survive a restart (#340, #266) (#365)

* Never crash & persist: Make Current Properties Default and page display choices survive a restart

#340:
- Expose per-tool comment styles via CommentPrefs::styles().
- persist() writes a `comment_styles` array (tool, color, opacity, width, fill).
- restore() reads each entry through the clamping setters; a new rgb_of helper
  at the read boundary refuses malformed colors whole (string, wrong length,
  null, non-finite) and clamps finite out-of-range values into 0..=1, so a
  hostile settings file can't smuggle a NaN or saturated color.

#266:
- The page display chosen in a document (view.layout.*, view.fit_one_page,
  view.fit_width_scrolling) now mirrors into view_defaults, which the existing
  default-layout/default-zoom keys already persist. New documents and the next
  launch open in the persisted display; transient zooms do not rewrite the
  default (matches Acrobat).

Tests:
- make_current_properties_default: persist + fresh app reuses the style.
- the_page_display_you_choose_becomes_the_default: choice mirrors, persists,
  new document opens in the persisted display; Acrobat fit modes persist too.
- comment_style_settings_clamp_and_refuse_hostile_values: hostile JSON is
  clamped or refused.

cargo fmt --check and cargo clippy -p pdfcraft-ui-egui --all-targets are clean.

* Fix clippy: chunks_exact(4) -> as_chunks::<4>() in signature_image.rs

* 365: keep the comment-style persistence (#340) only; a saved no-fill clears a default fill

The #266 half (every View choice rewriting the default page display)
is dropped: Preferences already sets that default explicitly. rgb_triple
moves out of the module declarations with one doc comment.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* Fill & Sign dates: Italian names, since Italian is now an interface language (#494)

date_languages_are_the_interface_languages requires every interface
language to have date names; the unknown-language example in the
session test moves from it to xx.

---------

Co-authored-by: Mawfyy <[email protected]>
Co-authored-by: Brandon Thomas <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>

crates/engine/src/dates.rs

@@ -53,7 +53,7 @@ const ZH_DAYS: [&str; 7] = ["星期日", "星期一", "星期二", "星期三",
 
 /// The languages month and weekday names can be written in (Preferences ▸ Date format ▸ Language):
 /// the interface languages, in the same order and with the same names.
-pub const DATE_LANGUAGES: [DateLanguage; 14] = [
+pub const DATE_LANGUAGES: [DateLanguage; 15] = [
     DateLanguage {
         code: "en",
         name: "English",
@@ -219,6 +219,15 @@ pub const DATE_LANGUAGES: [DateLanguage; 14] = [
         days: ["неділя", "понеділок", "вівторок", "середа", "четвер", "пʼятниця", "субота"],
         days_short: ["нд", "пн", "вт", "ср", "чт", "пт", "сб"],
     },
+    DateLanguage {
+        code: "it",
+        name: "Italiano",
+        months: ["gennaio", "febbraio", "marzo", "aprile", "maggio", "giugno", "luglio", "agosto", "settembre", "ottobre", "novembre", "dicembre"],
+        months_with_day: None,
+        months_short: ["gen", "feb", "mar", "apr", "mag", "giu", "lug", "ago", "set", "ott", "nov", "dic"],
+        days: ["domenica", "lunedì", "martedì", "mercoledì", "giovedì", "venerdì", "sabato"],
+        days_short: ["dom", "lun", "mar", "mer", "gio", "ven", "sab"],
+    },
 ];
 
 /// The date language for `code` (any case), or `None` when there's none.
@@ -527,7 +536,7 @@ mod tests {
         assert_eq!(s.today_text(None, None).as_deref(), Ok("terça-feira 14 novembro"));
         assert_eq!(s.today_text(Some("d mmmm"), Some("uk")).as_deref(), Ok("14 листопада"), "Ukrainian months after a day");
         assert_eq!(s.today_text(Some("dddd d mmmm"), Some("de")).as_deref(), Ok("Dienstag 14 November"));
-        assert!(s.set_date_language(Some("it")).unwrap_err().contains("zh-hant"), "only the interface languages");
+        assert!(s.set_date_language(Some("xx")).unwrap_err().contains("zh-hant"), "only the interface languages");
         assert_eq!(s.date_language(), Some("pt-br"), "an unknown language keeps the previous one");
         s.set_date_language(None).unwrap();
         assert_eq!(s.date_language(), None);

crates/ui-egui/src/comments.rs

@@ -263,6 +263,11 @@ impl CommentPrefs {
         self.styles.iter().find(|(t, _)| *t == tool).map(|(_, s)| s.clone()).unwrap_or_default()
     }
 
+    /// Every tool with its current default style (the persisted settings read this).
+    pub fn styles(&self) -> impl Iterator<Item = (CommentTool, &Style)> {
+        self.styles.iter().map(|(t, s)| (*t, s))
+    }
+
     pub fn set_color(&mut self, tool: CommentTool, c: Rgb) {
         if let Some((_, s)) = self.styles.iter_mut().find(|(t, _)| *t == tool) {
             s.color = c;

crates/ui-egui/src/lib.rs

@@ -101,6 +101,7 @@ pub(crate) fn date_text_for_pdf(session: &Session) -> Result<String, String> {
 
 /// The longest author name kept (Preferences ▸ Identity, restored settings).
 pub(crate) const MAX_AUTHOR_CHARS: usize = 200;
+
 pub mod portable;
 mod protect;
 mod recovery;
@@ -624,6 +625,25 @@ impl Default for PdfCraftApp {
     }
 }
 
+/// A restored colour: an `[r, g, b]` array of finite numbers, each clamped to 0–1 (restored
+/// settings are untrusted). Anything else (a string, a wrong length, a null or a non-finite
+/// number) is refused whole, so the caller keeps its default.
+fn rgb_triple(v: &serde_json::Value) -> Option<[f64; 3]> {
+    let a = v.as_array()?;
+    if a.len() != 3 {
+        return None;
+    }
+    let mut out = [0.0f64; 3];
+    for (slot, x) in out.iter_mut().zip(a.iter()) {
+        let n = x.as_f64()?;
+        if !n.is_finite() {
+            return None;
+        }
+        *slot = n.clamp(0.0, 1.0);
+    }
+    Some(out)
+}
+
 impl PdfCraftApp {
     pub fn new() -> Self {
         Self {
@@ -1273,6 +1293,19 @@ impl PdfCraftApp {
             // Null follows the interface language.
             "date_language": self.session.date_language(),
             "author": self.comment_prefs.author,
+            // Per-tool comment defaults (colours, opacity, widths), so Make Current
+            // Properties Default survives a restart (#340).
+            "comment_styles": self
+                .comment_prefs
+                .styles()
+                .map(|(t, s)| serde_json::json!({
+                    "tool": t.command(),
+                    "color": s.color,
+                    "opacity": s.opacity,
+                    "width": s.width,
+                    "fill": s.fill,
+                }))
+                .collect::<Vec<_>>(),
             // Drawn signatures keep their original form (older settings read the same).
             "signature": match &self.signature { Some(fill_sign::SavedSig::Drawn(s)) => Some(s), _ => None },
             "signature_text": match &self.signature { Some(fill_sign::SavedSig::Typed(t)) => Some(t), _ => None },
@@ -1341,6 +1374,34 @@ impl PdfCraftApp {
         if let Some(author) = v["author"].as_str().map(str::trim).filter(|a| !a.is_empty()) {
             self.comment_prefs.author = author.chars().take(MAX_AUTHOR_CHARS).collect();
         }
+        // Per-tool comment styles (#340): each field goes through its clamping setter, so a
+        // malformed or hostile entry is clamped or ignored rather than trusted.
+        if let Some(styles) = v["comment_styles"].as_array() {
+            for e in styles {
+                let Some(tool) = e["tool"].as_str().and_then(crate::comments::CommentTool::from_command) else { continue };
+                if let Some(c) = rgb_triple(&e["color"]) {
+                    self.comment_prefs.set_color(tool, c);
+                }
+                if let Some(o) = e["opacity"].as_f64() {
+                    self.comment_prefs.set_opacity(tool, o);
+                }
+                if let Some(w) = e["width"].as_f64() {
+                    self.comment_prefs.set_width(tool, w);
+                }
+                // `"fill": null` is a saved "no fill", which clears a tool's default fill; a
+                // missing or malformed fill keeps it.
+                let fill = match e.get("fill") {
+                    Some(serde_json::Value::Null) => Some(None),
+                    Some(v) => rgb_triple(v).map(Some),
+                    None => None,
+                };
+                if let Some(fill) = fill {
+                    let mut s = self.comment_prefs.style(tool);
+                    s.fill = fill;
+                    self.comment_prefs.set_style(tool, s);
+                }
+            }
+        }
         if let Ok(s) = serde_json::from_value::<Vec<Vec<[f32; 2]>>>(v["signature"].clone())
             && s.iter().all(|st| st.iter().all(|p| p.iter().all(|x| x.is_finite())))
         {

crates/ui-egui/tests/comments.rs

@@ -537,6 +537,13 @@ fn make_current_properties_default() {
     h.run_steps(3);
     let st = h.state().comment_prefs.style(pdfcraft_ui_egui::comments::CommentTool::Rectangle);
     assert_eq!((st.color, st.opacity, st.width), ([0.0, 0.47, 0.84], 0.5, 5.0));
+    // The default survives a restart (#340): a fresh app reads the persisted settings and its
+    // next rectangle still takes the style.
+    let json = h.state().persist();
+    let mut fresh = PdfCraftApp::new();
+    fresh.restore(&json);
+    let st = fresh.comment_prefs.style(pdfcraft_ui_egui::comments::CommentTool::Rectangle);
+    assert_eq!((st.color, st.opacity, st.width), ([0.0, 0.47, 0.84], 0.5, 5.0), "the tool default persisted");
     // The next rectangle takes it.
     h.state_mut().set_option("quick", "square").unwrap();
     drag_pt(&mut h, (160.0, 100.0), (260.0, 40.0));

crates/ui-egui/tests/ui.rs

@@ -380,6 +380,52 @@ fn persistence_round_trips_and_tolerates_garbage() {
     assert_eq!(b.theme, pdfcraft_ui_egui::theme::ThemeKind::Dark);
 }
 
+/// A tool's saved "no fill" (`"fill": null`) clears a fill it had as its default; a missing or
+/// malformed fill keeps it (#340).
+#[test]
+fn a_saved_no_fill_clears_a_comment_tool_default_fill() {
+    use pdfcraft_ui_egui::comments::CommentTool;
+    let filled = |a: &mut PdfCraftApp| {
+        let mut s = a.comment_prefs.style(CommentTool::Rectangle);
+        s.fill = Some([0.2, 0.4, 0.6]);
+        a.comment_prefs.set_style(CommentTool::Rectangle, s);
+    };
+    let mut a = PdfCraftApp::new();
+    filled(&mut a);
+    a.restore(r#"{"comment_styles":[{"tool":"comment.square","fill":null}]}"#);
+    assert_eq!(a.comment_prefs.style(CommentTool::Rectangle).fill, None, "null clears the fill");
+    let mut b = PdfCraftApp::new();
+    filled(&mut b);
+    b.restore(r#"{"comment_styles":[{"tool":"comment.square","color":[0,0,1]},{"tool":"comment.square","fill":"red"}]}"#);
+    assert_eq!(b.comment_prefs.style(CommentTool::Rectangle).fill, Some([0.2, 0.4, 0.6]), "missing or malformed keeps it");
+    // A round trip keeps a cleared fill cleared.
+    let mut c = PdfCraftApp::new();
+    c.restore(&a.persist());
+    assert_eq!(c.comment_prefs.style(CommentTool::Rectangle).fill, None);
+}
+
+/// Persisted per-tool comment styles go through their clamping setters: hostile or malformed
+/// values are clamped or ignored, never trusted (#340).
+#[test]
+fn comment_style_settings_clamp_and_refuse_hostile_values() {
+    use pdfcraft_ui_egui::comments::CommentTool;
+    let mut a = PdfCraftApp::new();
+    a.restore(
+        r#"{"comment_styles":[
+            {"tool":"comment.highlight","color":[2,-1,0.5],"opacity":99,"width":-4},
+            {"tool":"comment.note","color":"yellow","opacity":"none"},
+            {"tool":"comment.note","color":[0.1,null,0.3]},
+            {"tool":"bogus.command","color":[0,0,0]}
+        ]}"#,
+    );
+    // Out-of-range numbers are clamped into range.
+    let st = a.comment_prefs.style(CommentTool::Highlight);
+    assert_eq!((st.color, st.opacity, st.width), ([1.0, 0.0, 0.5], 1.0, 0.5));
+    // A malformed colour is refused whole: the tool keeps its default.
+    let note = a.comment_prefs.style(CommentTool::Note);
+    assert_eq!(note.color, [1.0, 0.82, 0.0], "the default colour stays");
+}
+
 #[test]
 fn zoom_keeps_the_point_under_the_cursor_still() {
     let mut h = harness(|app| {

parity/acrobat-features.toml

@@ -3819,7 +3819,7 @@ status = "shipped"
 commands = ["comment.list"]
 tools = []
 evidence = ["crates/ui-egui/tests/comments.rs::make_current_properties_default"]
-notes = "colour, opacity and line width become the tool's defaults (canvas and Comments panel menus)"
+notes = "colour, opacity and line width become the tool's defaults (canvas and Comments panel menus) and survive a restart: the per-tool styles are persisted with the settings and restored through their clamping setters (#340)"
 
 [[feature]]
 id = "comment.author-identity"