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.
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-03 22:24:44 -05:00
1 parent 417ad412a2
commit 3b4de34efe
2 files changed
+20 -2

No files matched your search

+6 -2
View File
@@ -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<NodeId> = 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<NodeId> = 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);
+14
View File
@@ -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");