diff --git a/crates/plugins/src/manifest.rs b/crates/plugins/src/manifest.rs index 34b1d44138..d8523358c9 100644 --- a/crates/plugins/src/manifest.rs +++ b/crates/plugins/src/manifest.rs @@ -48,6 +48,17 @@ impl Unit { } } +/// Reads `"unit"` leniently: `"px"` is [`Unit::Px`]; anything else (an unknown string such as +/// `"pt"`, a number, `null`, an array…) is unitless, as when the field is absent. Manifests from +/// before `unit` existed could carry any value there (unknown fields were ignored), and they must +/// keep installing. +fn lenient_unit<'de, D: serde::Deserializer<'de>>(d: D) -> std::result::Result { + Ok(match Value::deserialize(d)? { + Value::String(s) if s == "px" => Unit::Px, + _ => Unit::None, + }) +} + /// One declared parameter. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "camelCase")] @@ -57,7 +68,7 @@ pub enum ParamSpec { min: f64, max: f64, default: f64, - #[serde(default, skip_serializing_if = "Unit::is_none")] + #[serde(default, deserialize_with = "lenient_unit", skip_serializing_if = "Unit::is_none")] unit: Unit, }, /// A whole number in `min..=max`. @@ -68,7 +79,7 @@ pub enum ParamSpec { max: i64, #[serde(default)] default: i64, - #[serde(default, skip_serializing_if = "Unit::is_none")] + #[serde(default, deserialize_with = "lenient_unit", skip_serializing_if = "Unit::is_none")] unit: Unit, }, /// A checkbox. @@ -495,12 +506,19 @@ mod tests { } #[test] - fn malformed_units_are_manifest_errors() { + fn unknown_units_are_unitless() { + // Unknown fields were always ignored, so a manifest may already carry any `unit`: it must + // keep installing, unscaled, exactly as when the field is absent. for unit in [r#""pt""#, r#""PX""#, r#""""#, "5", "true", "null", r#"["px"]"#, r#"{"px":1}"#] { - let bad = format!(r#"{{"id":"a","name":"x","kind":"filter","params":{{"r":{{"type":"number","min":0,"max":9,"default":1,"unit":{unit}}}}}}}"#); - assert!(matches!(Manifest::parse(bad.as_bytes()), Err(Error::Manifest(_))), "{bad}"); - let bad = bad.replace(r#""type":"number""#, r#""type":"int""#); - assert!(matches!(Manifest::parse(bad.as_bytes()), Err(Error::Manifest(_))), "{bad}"); + let text = format!(r#"{{"id":"a","name":"x","kind":"filter","params":{{"r":{{"type":"number","min":0,"max":9,"default":1,"unit":{unit}}}}}}}"#); + for text in [text.clone(), text.replace(r#""type":"number""#, r#""type":"int""#)] { + let m = Manifest::parse(text.as_bytes()).unwrap_or_else(|e| panic!("{text}: {e}")); + assert!(!m.is_pixel_param("r"), "{text}"); + let mut p = json!({"r": 8}).as_object().unwrap().clone(); + m.scale_for_preview(&mut p, 4); + assert_eq!(Value::Object(p), json!({"r": 8}), "{text}"); + assert!(serde_json::to_value(&m).unwrap()["params"]["r"].get("unit").is_none(), "{text}"); + } } } diff --git a/docs/plugins.md b/docs/plugins.md index 1e3b007850..11139151c1 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -105,7 +105,8 @@ exports. All integers are `i32` unless noted; pointers are offsets into the modu clamped to `min..max` (ints rounded), a wrong type or an unknown choice is an error. - `unit` (optional, on `number` and `int` params): `"px"` marks a length in document pixels (a radius, distance, shift or size); `"none"`, the default, is everything else (levels, factors, - counts, percentages). Any other value is a manifest error. A filter dialog's live preview on a + counts, percentages). Any other value (an unknown string, a number…) is treated as `"none"`, as + when the field is absent, so older manifests keep installing. A filter dialog's live preview on a large document runs on a reduced copy (Preferences › Performance › Low Resolution Previews), and the host divides each `px` param by that reduction (ints rounded, kept within `min..max`) so the preview shows the same lengths OK applies; when a `px` param with a positive `min` would drop below