From 3b4de34efea0b433f63aa8ec85991d378c41b35f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 22:24:44 -0500 Subject: [PATCH] fix(doc): list a page once however often a file lists it (#1927) Loading accepted a page id repeated in the pages list and repair kept the repeats, so import, Fig and Penpot export and every page walk visited and retained that page's subtree once per listing. validate now reports a repeated page and repair drops the repeats. --- crates/doc/src/lib.rs | 8 ++++++-- crates/doc/src/tests.rs | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 0181ffd21..1349aac1d 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -1098,6 +1098,9 @@ impl Document { // lookups: a wide parent or a long page list doesn't make validation quadratic. let edges: std::collections::BTreeSet<(NodeId, NodeId)> = self.nodes.iter().flat_map(|(p, n)| n.children.iter().map(|c| (*p, *c))).collect(); let page_ids: std::collections::BTreeSet = self.pages.iter().copied().collect(); + if page_ids.len() != self.pages.len() { + note(&mut issues, || "a page is listed twice".to_string()); + } for (id, n) in self.nodes.iter() { if n.id != *id { note(&mut issues, || format!("node key {id} has id {}", n.id)); @@ -1191,9 +1194,10 @@ impl Document { if store != self.libraries.store { Arc::make_mut(&mut self.libraries).store = store; } - self.pages.retain(|p| self.nodes.get(p).is_some_and(|n| n.is_page()) && Some(*p) != store); + // A page listed twice would be imported, exported and walked once per listing. + let mut listed = std::collections::BTreeSet::new(); + self.pages.retain(|p| self.nodes.get(p).is_some_and(|n| n.is_page()) && Some(*p) != store && listed.insert(*p)); // Unlisted pages join the list; a set keeps many pages from making this quadratic (#1078). - let mut listed: std::collections::BTreeSet = self.pages.iter().copied().collect(); for (k, n) in self.nodes.iter() { if n.is_page() && Some(*k) != store && listed.insert(*k) { self.pages.push(*k); diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index a5c8e8e7a..6ac6f455b 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -348,6 +348,20 @@ fn repair_drops_cycles_and_repeated_children() { assert_eq!(bad.subtree(p), vec![p, r]); } +#[test] +fn repair_lists_a_page_once_however_often_the_file_lists_it() { + let mut d = Document::new("x"); + let p = page(&d); + let q = d.add_page("Two"); + let mut v = serde_json::to_value(&d).unwrap(); + v["pages"] = serde_json::json!([p, q, p, q, p]); + let mut bad: Document = serde_json::from_value(v).unwrap(); + assert!(bad.validate().iter().any(|i| i.contains("listed twice")), "{:?}", bad.validate()); + bad.repair(); + assert!(bad.validate().is_empty(), "{:?}", bad.validate()); + assert_eq!(bad.pages, vec![p, q]); +} + #[test] fn validate_caps_the_issue_list() { let mut d = Document::new("x");