From 0e25cf4c2651db9395024cff75b7e1c5958bc7b8 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:47:07 +0000 Subject: [PATCH 1/3] Refuse pages in edit.copy and edit.cut Found while testing crates/engine/src/cmd/edit.rs:499 (copy_to_clipboard, called from copy at :536 and cut at :542) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/edit.rs | 11 +++++++++-- crates/engine/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 2c767b93..ab503a02 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -499,7 +499,12 @@ fn bump_name(n: &str) -> String { pub fn copy_to_clipboard(s: &mut Session, ids: &[NodeId]) -> Result<()> { let ds = s.ds()?; let doc = &ds.doc; - let ids = z_sorted(doc, &roots(doc, ids)); + // Pages are not layers: they can't be pasted anywhere. + let layers: Vec = ids.iter().copied().filter(|i| !doc.node(*i).is_some_and(Node::is_page)).collect(); + if layers.is_empty() && !ids.is_empty() { + return Err(EngineError::Disabled("edit.copy".into(), "pages can't be copied; use page.duplicate".into())); + } + let ids = z_sorted(doc, &roots(doc, &layers)); let mut nodes = Vec::new(); let mut images = BTreeMap::new(); for i in &ids { @@ -542,7 +547,9 @@ fn copy(s: &mut Session, p: &Value) -> Result { fn cut(s: &mut Session, p: &Value) -> Result { let ids = req_targets("edit.cut", s, p)?; copy_to_clipboard(s, &ids)?; - delete(s, &json!({"ids": ids})) + let doc = s.doc()?; + let layers: Vec = ids.into_iter().filter(|i| !doc.node(*i).is_some_and(Node::is_page)).collect(); + delete(s, &json!({"ids": layers})) } /// Paste the clipboard under `parent` with the copied top-left moved to `at` (page coords). diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index dbf432d4..6010559e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -621,3 +621,20 @@ fn find_replace_changing_only_the_case_replaces_every_occurrence() { assert_eq!(s.doc().unwrap().node(t).unwrap().kind.text().unwrap().characters, "FigCraft and FigCraft, FigCraft"); assert_eq!(s.doc().unwrap().node(f).unwrap().name, "FigCraft / FigCraft"); } + +#[test] +fn copy_and_cut_ignore_pages_so_paste_keeps_the_document_valid() { + for cmd in ["edit.copy", "edit.cut"] { + let mut s = sess(); + let page = s.page().unwrap(); + s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + let r = s.execute(cmd, &json!({"ids": [page.0]})); + if let Ok(v) = &r { + assert!(v.get("deleted").is_none_or(|d| d == &json!([])), "{cmd} reported the page as deleted: {v}"); + } + let _ = s.execute("edit.paste", &json!({})); + let doc = s.doc().unwrap(); + assert!(doc.validate().is_empty(), "{cmd}: {:?}", doc.validate()); + assert!(doc.node(page).is_some(), "{cmd} removed the page"); + } +} From 01ce23ca765fabba5ec22bd83de5684614e259d3 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 17 Oct 2026 15:58:23 +0700 Subject: [PATCH 2/3] #680: page refusal names the calling command (edit.copy / edit.cut) --- crates/engine/src/cmd/edit.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index ab503a02..22610934 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -496,13 +496,13 @@ fn bump_name(n: &str) -> String { } /// The copied form of the selection. -pub fn copy_to_clipboard(s: &mut Session, ids: &[NodeId]) -> Result<()> { +pub fn copy_to_clipboard(s: &mut Session, cmd: &str, ids: &[NodeId]) -> Result<()> { let ds = s.ds()?; let doc = &ds.doc; // Pages are not layers: they can't be pasted anywhere. let layers: Vec = ids.iter().copied().filter(|i| !doc.node(*i).is_some_and(Node::is_page)).collect(); if layers.is_empty() && !ids.is_empty() { - return Err(EngineError::Disabled("edit.copy".into(), "pages can't be copied; use page.duplicate".into())); + return Err(EngineError::Disabled(cmd.into(), "pages can't be copied; use page.duplicate".into())); } let ids = z_sorted(doc, &roots(doc, &layers)); let mut nodes = Vec::new(); @@ -540,13 +540,13 @@ pub fn copy_to_clipboard(s: &mut Session, ids: &[NodeId]) -> Result<()> { fn copy(s: &mut Session, p: &Value) -> Result { let ids = req_targets("edit.copy", s, p)?; - copy_to_clipboard(s, &ids)?; + copy_to_clipboard(s, "edit.copy", &ids)?; Ok(json!({"copied": s.clipboard.nodes.len()})) } fn cut(s: &mut Session, p: &Value) -> Result { let ids = req_targets("edit.cut", s, p)?; - copy_to_clipboard(s, &ids)?; + copy_to_clipboard(s, "edit.cut", &ids)?; let doc = s.doc()?; let layers: Vec = ids.into_iter().filter(|i| !doc.node(*i).is_some_and(Node::is_page)).collect(); delete(s, &json!({"ids": layers})) From 5ea5590673bff20d96097b37cc6e2177e9c69348 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sat, 17 Oct 2026 15:58:24 +0700 Subject: [PATCH 3/3] #680: move the page clipboard test to its own module, check the error id --- crates/engine/src/lib.rs | 2 ++ crates/engine/src/page_clipboard_tests.rs | 36 +++++++++++++++++++++++ crates/engine/src/tests.rs | 17 ----------- 3 files changed, 38 insertions(+), 17 deletions(-) create mode 100644 crates/engine/src/page_clipboard_tests.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 6e0e8b12..9f00467a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1592,6 +1592,8 @@ mod open_issues_tests; #[cfg(test)] mod outline_options_tests; #[cfg(test)] +mod page_clipboard_tests; +#[cfg(test)] mod page_copy_navigation_tests; #[cfg(test)] mod page_target_tests; diff --git a/crates/engine/src/page_clipboard_tests.rs b/crates/engine/src/page_clipboard_tests.rs new file mode 100644 index 00000000..79ff7419 --- /dev/null +++ b/crates/engine/src/page_clipboard_tests.rs @@ -0,0 +1,36 @@ +//! `edit.copy` / `edit.cut` skip pages (they aren't layers and can't be pasted), and refuse a +//! selection of only pages under their own command id. + +use serde_json::json; + +use crate::{EngineError, Session}; + +#[test] +fn copy_and_cut_ignore_pages_so_paste_keeps_the_document_valid() { + for cmd in ["edit.copy", "edit.cut"] { + let mut s = Session::new(); + let page = s.page().unwrap(); + s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + match s.execute(cmd, &json!({"ids": [page.0]})) { + Err(EngineError::Disabled(id, _)) => assert_eq!(id, cmd), + r => panic!("{cmd} on a page: {r:?}"), + } + let _ = s.execute("edit.paste", &json!({})); + let doc = s.doc().unwrap(); + assert!(doc.validate().is_empty(), "{cmd}: {:?}", doc.validate()); + assert!(doc.node(page).is_some(), "{cmd} removed the page"); + } +} + +#[test] +fn cut_of_a_page_and_a_layer_moves_only_the_layer() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let r = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + let rect = crate::params::id_of(&r["id"]).unwrap(); + s.execute("edit.cut", &json!({"ids": [page.0, rect.0]})).unwrap(); + assert!(s.doc().unwrap().node(page).is_some()); + assert!(s.doc().unwrap().node(rect).is_none()); + s.execute("edit.paste", &json!({})).unwrap(); + assert!(s.doc().unwrap().validate().is_empty()); +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6010559e..dbf432d4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -621,20 +621,3 @@ fn find_replace_changing_only_the_case_replaces_every_occurrence() { assert_eq!(s.doc().unwrap().node(t).unwrap().kind.text().unwrap().characters, "FigCraft and FigCraft, FigCraft"); assert_eq!(s.doc().unwrap().node(f).unwrap().name, "FigCraft / FigCraft"); } - -#[test] -fn copy_and_cut_ignore_pages_so_paste_keeps_the_document_valid() { - for cmd in ["edit.copy", "edit.cut"] { - let mut s = sess(); - let page = s.page().unwrap(); - s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); - let r = s.execute(cmd, &json!({"ids": [page.0]})); - if let Ok(v) = &r { - assert!(v.get("deleted").is_none_or(|d| d == &json!([])), "{cmd} reported the page as deleted: {v}"); - } - let _ = s.execute("edit.paste", &json!({})); - let doc = s.doc().unwrap(); - assert!(doc.validate().is_empty(), "{cmd}: {:?}", doc.validate()); - assert!(doc.node(page).is_some(), "{cmd} removed the page"); - } -}