Select › Modify refuses a radius below the dialog's minimum instead of clamping it to 0 (#993)

modify() rejected only a non-finite radius or one above the maximum, then
clamped with r.max(0.0). A negative or zero radius reported success: Border
cleared the selection, Expand and Contract did nothing, and Feather 0 was
accepted although its documented range starts at 0.1.

Each command now checks the range in its params string (Border 1..200,
Smooth/Expand/Contract 1..500, Feather 0.1..1000). A radius outside it,
including NaN and infinity, returns BadParams naming that range, and the
selection is left as it was. The clamp is removed.
This commit is contained in:
MarekSpodBiedry authored and Claude committed 2026-10-08 23:22:53 +00:00
1 parent b71991e15a
commit 627baa422a
1 file changed
+40 -8
+40 -8
View File
@@ -296,16 +296,17 @@ fn color_range(s: &mut Session, p: &Value) -> Result<Value> {
fn modify(s: &mut Session, p: &Value, op: &str) -> Result<Value> {
// The dialogs' ranges (Photoshop's limits); anything else is refused, never clamped silently.
let max = match op {
"border" => 200.0,
"feather" => 1000.0,
_ => 500.0,
let (min, max): (f32, f32) = match op {
"border" => (1.0, 200.0),
"feather" => (0.1, 1000.0),
_ => (1.0, 500.0),
};
let r = f(p, "radius", 1.0);
// 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}") });
// Below the dialog's minimum (a negative or zero radius, #993) or above its maximum is
// refused: a clamp to 0 left the selection alone (Expand, Contract) or cleared it (Border)
// while reporting success.
if !(min..=max).contains(&r) {
return Err(EngineError::BadParams { cmd: format!("select.modify.{op}"), msg: format!("radius must be a number in {min}..{max}") });
}
// 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
@@ -801,6 +802,37 @@ mod tests {
}
}
#[test]
fn modify_refuses_radii_below_the_minimum() {
let mut s = session();
s.execute("select.rect", json!({"x": 10, "y": 10, "width": 10, "height": 10})).unwrap();
// Below each dialog's minimum (1 px, Feather 0.1 px) is refused, not clamped to 0: a
// negative Border radius used to clear the selection and report success (#993).
let below = [
("border", -5.0),
("border", 0.5),
("smooth", -5.0),
("smooth", 0.0),
("expand", -5.0),
("expand", 0.0),
("contract", -5.0),
("contract", 0.9),
("feather", -5.0),
("feather", 0.0),
("feather", 0.05),
];
let before = current_mask(&s.active().unwrap().doc).1;
for (op, r) in below {
let err = s.execute(&format!("select.modify.{op}"), json!({"radius": r})).unwrap_err();
assert!(matches!(err, EngineError::BadParams { .. }), "{op} {r}: {err}");
assert!(current_mask(&s.active().unwrap().doc).1 == before, "{op} {r}: the selection changed");
}
// The minimum itself is still accepted.
s.execute("select.modify.feather", json!({"radius": 0.1})).unwrap();
s.execute("select.modify.expand", json!({"radius": 1})).unwrap();
assert_eq!(coverage(&s, 9, 15), 1.0);
}
#[test]
fn geometry_params_that_would_wrap_are_rejected() {
let mut s = session();