From 6f7192d294a2ecdc98cfee4076c9fb4c4699dedb Mon Sep 17 00:00:00 2001 From: DWAA1660 Date: Thu, 8 Oct 2026 16:28:50 -0600 Subject: [PATCH] =?UTF-8?q?Select=20=E2=80=BA=20Modify=20refuses=20a=20neg?= =?UTF-8?q?ative=20radius=20instead=20of=20clamping=20it=20(#993)=20(#1274?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit modify() rejected only non-finite radii and radii above the dialog's maximum, then clamped with r.max(0.0). A negative radius therefore reported success while leaving the selection alone (Expand, Contract) or clearing it (Border), against the function's own rule that out-of-range values are "refused, never clamped silently". Negative radii now get the same BadParams error, "radius must be a number in 0..max", and the selection is untouched. Test: modify_commands also tries -5, -0.5 and f64::MIN for all five Modify commands. Claude-Session: https://claude.ai/code/session_01K3vVft4VXvvy8SpbbUceTW Co-authored-by: Claude Opus 5.5 --- crates/engine/src/selection_cmds.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/selection_cmds.rs b/crates/engine/src/selection_cmds.rs index c138306c49..c18a742506 100644 --- a/crates/engine/src/selection_cmds.rs +++ b/crates/engine/src/selection_cmds.rs @@ -302,10 +302,11 @@ fn modify(s: &mut Session, p: &Value, op: &str) -> Result { _ => 500.0, }; let r = f(p, "radius", 1.0); - if !r.is_finite() || r > max { + // A negative radius is refused too (#993): it used to be clamped to 0, which left the + // selection alone (Expand, Contract) or cleared it (Border) while reporting success. + if !r.is_finite() || !(0.0..=max).contains(&r) { return Err(EngineError::BadParams { cmd: format!("select.modify.{op}"), msg: format!("radius must be a number in 0..{max}") }); } - let r = r.max(0.0); // Photoshop's "Apply effect at canvas bounds": when on, the canvas edge is a selection edge // (Select All then Contract shrinks from the edges); when off, the selection is taken to // continue past the canvas. Border has no such option: its band always follows the canvas edge, @@ -746,7 +747,7 @@ mod tests { assert!(!s.is_enabled("select.nothing")); // Out-of-range or non-finite radii are refused and leave the selection alone. for op in ["feather", "smooth", "expand", "contract", "border"] { - for r in [json!(1e300), json!(5000), json!(f64::MAX)] { + for r in [json!(1e300), json!(5000), json!(f64::MAX), json!(-5), json!(-0.5), json!(f64::MIN)] { assert!(s.execute(&format!("select.modify.{op}"), json!({"radius": r})).is_err(), "{op} {r}"); } }