From a709ac2ef515c9ca54acafe718da24b5166ecb4c Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Wed, 11 Nov 2026 10:51:13 -0500 Subject: [PATCH] Live-shape question: refused once the document changed under it (#12768) The question also stores the document's revision: an edit while it is open (an anchor added through the control channel, the question being modal on screen) shifts the stored knot index, so Yes is refused rather than deleting another anchor. Test: yes_after_the_path_changed_deletes_nothing (red before: Yes deleted an anchor of the edited path). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs/live_shape_prompt.rs | 10 ++++++---- .../anchor_tools/delete_anchor_live_shape_tests.rs | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/dialogs/live_shape_prompt.rs b/crates/ui-egui/src/dialogs/live_shape_prompt.rs index b963516bc4..6db96b6760 100644 --- a/crates/ui-egui/src/dialogs/live_shape_prompt.rs +++ b/crates/ui-egui/src/dialogs/live_shape_prompt.rs @@ -32,7 +32,7 @@ fn is_live(app: &PhotocraftApp, r: PathRef) -> bool { /// or "Don't show again" was ticked. pub fn ask(app: &mut PhotocraftApp, r: PathRef, params: &Value) -> bool { let PathRef::Layer(layer) = r else { return false }; - let Some(document) = app.session.active().map(|st| st.doc.id.0) else { return false }; + let Some((document, revision)) = app.session.active().map(|st| (st.doc.id.0, st.revision)) else { return false }; if !is_live(app, r) || dont_show_again::dismissed(app, PREF) { return false; } @@ -44,8 +44,10 @@ pub fn ask(app: &mut PhotocraftApp, r: PathRef, params: &Value) -> bool { f.insert("__command".into(), json!("path.deleteAnchor")); f.insert("__label".into(), json!(tl!("Delete Anchor Point"))); f.insert("layer".into(), json!(layer)); - // The layer id means something only in this document (another may become active meanwhile). + // The layer id means something only in this document (another may become active meanwhile), + // and the knot index only in this revision of it (an edit meanwhile can shift the anchors). f.insert("__document".into(), json!(document)); + f.insert("__revision".into(), json!(revision)); f.insert("params".into(), params.clone()); f.insert("__dontShowAgain".into(), json!(PREF)); f.insert("skip".into(), json!(false)); @@ -62,8 +64,8 @@ pub fn body(ui: &mut egui::Ui, fields: &mut Map) { pub fn confirm(app: &mut PhotocraftApp, fields: &Map) -> Result { let layer = fields.get("layer").and_then(Value::as_u64).ok_or(tl!("No anchor to delete"))?; let params = fields.get("params").cloned().ok_or(tl!("No anchor to delete"))?; - let document = fields.get("__document").and_then(Value::as_u64); - if document.is_none() || app.session.active().map(|st| st.doc.id.0) != document { + let asked = fields.get("__document").and_then(Value::as_u64).zip(fields.get("__revision").and_then(Value::as_u64)); + if asked.is_none() || app.session.active().map(|st| (st.doc.id.0, st.revision)) != asked { return Err(tl!("No anchor to delete").into()); } let result = app.run("path.deleteAnchor", params)?; diff --git a/crates/ui-egui/src/tools/anchor_tools/delete_anchor_live_shape_tests.rs b/crates/ui-egui/src/tools/anchor_tools/delete_anchor_live_shape_tests.rs index 015a4a7e04..fdccea07e0 100644 --- a/crates/ui-egui/src/tools/anchor_tools/delete_anchor_live_shape_tests.rs +++ b/crates/ui-egui/src/tools/anchor_tools/delete_anchor_live_shape_tests.rs @@ -104,6 +104,20 @@ fn yes_after_another_document_became_active_deletes_nothing() { assert_eq!(corners(&mut app, shape), 4, "the first document's shape keeps its corner"); } +/// The question's knot index belongs to the path as it was asked: an anchor added before the +/// corner meanwhile (through the control channel; the question is modal on screen) shifts it, so Yes +/// is refused instead of deleting another anchor. +#[test] +fn yes_after_the_path_changed_deletes_nothing() { + let (mut app, shape) = live(Tool::DeleteAnchorPoint); + click(&mut app, CORNER); + let id = question(&app).expect("the question"); + app.run("path.addAnchor", json!({"name": "layer", "layer": shape, "subpath": 0, "knot": 0, "t": 0.5})).unwrap(); + let (before, n) = (shape_path(&mut app, shape), steps(&app)); + assert!(crate::dialogs::confirm(&mut app, id).is_err()); + assert_eq!((shape_path(&mut app, shape), steps(&app)), (before, n)); +} + #[test] fn no_leaves_the_shape_its_parameters_and_history_unchanged() { let (mut app, shape) = live(Tool::DeleteAnchorPoint);