From f6dbc82e6e3e62e72f7ddd4a49313e5e7c25cfe0 Mon Sep 17 00:00:00 2001 From: Christian Lappin <36496308+christianlappin@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:00:24 -0500 Subject: [PATCH] M3: validate paragraph-only Eyedropper bounds (#6404) Paragraph justification can move point text past the canvas limit even when character copying is disabled. Check those targets before keeping the edit or remembered sample, and cover the explicit-target case with a regression. --- crates/engine/src/cmd/appearance.rs | 14 +++++++++---- crates/engine/src/tests/eyedropper.rs | 29 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 1f8d1d6967..1f64e582ad 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -1282,12 +1282,12 @@ fn copy_from(s: &mut Session, p: &Value) -> Result { Some((id, start, end)) if targets.contains(&id) => vec![(id, Some((start, end)))], _ => vec![], }; + let target_ids = || targets.iter().copied().filter(|_| ranges.is_empty()).chain(ranges.iter().map(|(id, _)| *id)); // Prepare every dimensional copy before changing defaults or the remembered sample. A bad // source or target transform must refuse the entire pick, including a multi-frame range. let characters = if attrs.character && !append { let d = &s.doc()?.doc; - let ids = targets.iter().copied().filter(|_| ranges.is_empty()).chain(ranges.iter().map(|(id, _)| *id)); - copied_character_styles(&node, ids.filter_map(|id| d.node(id)))? + copied_character_styles(&node, target_ids().filter_map(|id| d.node(id)))? } else { HashMap::new() }; @@ -1305,8 +1305,14 @@ fn copy_from(s: &mut Session, p: &Value) -> Result { } } // The session's bounds check follows the selection. Check text targets reached through - // explicit IDs, a reverse pick, or a highlighted story as well. - let text_ids: Vec<_> = characters.keys().copied().collect(); + // explicit IDs, a reverse pick, or a highlighted story as well. Paragraph changes can + // move point text even when character copying is disabled. + let text_ids: Vec<_> = if attrs.paragraph && !append && picked.text.is_some() { + let d = &s.doc()?.doc; + target_ids().filter(|id| d.node(*id).is_some_and(|n| matches!(n.kind, NodeKind::Text(_)))).collect() + } else { + characters.keys().copied().collect() + }; let check_text_bounds = |d: &Document| { if !text_ids.is_empty() && !crate::sanity::objects_in_range(d, &text_ids) { return Err(bad(C, "sampled text would exceed the canvas (coordinates out of range)")); diff --git a/crates/engine/src/tests/eyedropper.rs b/crates/engine/src/tests/eyedropper.rs index 5204995d68..b3a9012e06 100644 --- a/crates/engine/src/tests/eyedropper.rs +++ b/crates/engine/src/tests/eyedropper.rs @@ -442,6 +442,35 @@ fn scaled_type_pick_canvas_rejection_preserves_sample_and_defaults() { } } +/// Changing justification can move explicitly targeted point text outside the canvas even +/// when character copying is disabled. +#[test] +fn paragraph_only_pick_rejects_out_of_bounds_explicit_target() { + let mut s = session(); + let saved = id_of(s.execute("text.create", &json!({"x": 40, "y": 40, "text": "Saved", "color": "#0000ff"})).unwrap()); + let src = id_of(s.execute("text.create", &json!({"x": 100, "y": 100, "text": "Source", "color": "#ff0000"})).unwrap()); + s.execute("text.setStyle", &json!({"justify": "left"})).unwrap(); + let dst = id_of(s.execute("text.create", &json!({"x": 100, "y": 200, "text": "WWWW", "size": 16})).unwrap()); + s.execute("text.setStyle", &json!({"justify": "right"})).unwrap(); + s.execute("object.transform", &json!({"matrix": [1, 0, 0, 1, MAX_COORD - 101.0, 0]})).unwrap(); + s.execute("select.none", &json!({})).unwrap(); + s.execute("appearance.copyFrom", &json!({"source": saved.0, "ids": []})).unwrap(); + let doc = s.doc().unwrap().doc.clone(); + let selection = s.doc().unwrap().selection.clone(); + let paint = s.paint.clone(); + let sample = s.eyedropper_sample.clone(); + let history = undo_len(&s); + + let pick = json!({"source": src.0, "ids": [dst.0], "apply": {"character": false}}); + let error = s.execute("appearance.copyFrom", &pick).unwrap_err(); + assert!(error.to_string().contains("coordinates out of range"), "{error}"); + assert_eq!(s.doc().unwrap().doc, doc); + assert_eq!(s.doc().unwrap().selection, selection); + assert_eq!(s.paint, paint); + assert_eq!(s.eyedropper_sample, sample); + assert_eq!(undo_len(&s), history); +} + /// Disabled character copying and append preserve typography; reverse and cached picks convert it. #[test] fn scaled_type_pick_respects_options_and_reverse_sampling() {