fix(text): reject unknown character fill swatches

Reject unknown character fill swatches before modifying text or undo history. Registered swatch names remain valid, including names that resemble hex colours.

Validation: invalid-name atomicity and save/reopen regressions, plus all six cargo xtask ci gates on main after #514 and #523 (1,186 native tests passed, 3 ignored; 15 crates passed WASM compilation checks).

Task: #403
This commit is contained in:
KnottyDyes authored and GitHub committed 2026-10-11 21:30:21 -06:00
1 parent a21e8db14f
commit fae9bc84ce
3 files changed
+166

No files matched your search

+1
View File
@@ -181,6 +181,7 @@ fn change(s: &mut Session, p: &Value) -> Result<Value> {
over.set_json(k, v).map_err(|e| bad("find.change", e))?;
}
}
super::text::validate_char_fill(&s.doc()?.doc, &over, "find.change")?;
let targets = scope(s, p)?;
s.edit(|d, sel| {
let mut count = 0usize;
+152
View File
@@ -444,6 +444,7 @@ fn create_para(s: &mut Session, p: &Value) -> Result<Value> {
chars = full;
}
}
super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.create")?;
let based_on = str_param(p, "basedOn").map(str::to_string);
let next = str_param(p, "nextStyle").map(str::to_string);
s.edit(|d, _| {
@@ -470,6 +471,7 @@ fn CharProps_to_attrs(p: &designcraft_doc::CharProps) -> CharAttrs {
fn edit_para(s: &mut Session, p: &Value) -> Result<Value> {
let name = str_param(p, "name").ok_or_else(|| bad("style.paragraph.edit", "missing name"))?.to_string();
let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?;
super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.edit")?;
let rename = str_param(p, "rename").map(str::to_string);
let based = p.get("basedOn").cloned();
s.edit(|d, _| {
@@ -621,6 +623,7 @@ fn delete_para(s: &mut Session, p: &Value) -> Result<Value> {
fn create_char(s: &mut Session, p: &Value) -> Result<Value> {
let base = str_param(p, "name").unwrap_or("Character Style 1").to_string();
let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?;
super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.create")?;
// Based on [None] is based on nothing.
let based_on = str_param(p, "basedOn").filter(|b| *b != designcraft_doc::NO_CHAR_STYLE).map(str::to_string);
s.edit(|d, _| {
@@ -641,6 +644,7 @@ fn edit_char(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad("style.character.edit", "[None] can't be edited"));
}
let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?;
super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.edit")?;
// `null` unsets an attribute (the style no longer sets it).
let unset: Vec<String> = p
.get("chars")
@@ -1045,6 +1049,154 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) {
mod color_tests {
use super::*;
fn fill_session() -> Session {
let mut session = Session::new();
session.execute("file.new", &json!({})).unwrap();
let frame = session.execute("frame.create", &json!({"rect": [0, 0, 200, 100], "content": "text", "text": "Hi"})).unwrap();
session.execute("text.select", &json!({"story": frame["story"], "anchor": 0, "focus": 2})).unwrap();
session.execute("style.paragraph.create", &json!({"name": "Existing"})).unwrap();
session.execute("style.character.create", &json!({"name": "Existing"})).unwrap();
session.execute("type.char", &json!({"size": 18})).unwrap();
session.execute("edit.undo", &json!({})).unwrap();
session
}
fn assert_fill_rejected(session: &mut Session, command: &str, params: Value) {
let before = session.doc().unwrap().clone();
let journal = session.journal.clone();
let fill = params
.pointer("/chars/fill")
.or_else(|| params.pointer("/attrs/fill"))
.or_else(|| params.get("fill"))
.and_then(Value::as_str)
.unwrap_or("Missing Ink");
let result = session.execute(command, &params);
match result {
Err(crate::EngineError::BadParams { cmd, msg }) => {
assert_eq!(cmd, command);
assert!(msg.contains(fill), "{msg}");
assert!(msg.contains("swatch.create") && msg.contains("returned name"), "{msg}");
}
other => panic!("expected BadParams for {command}, got {other:?}"),
}
let after = session.doc().unwrap();
assert!(std::sync::Arc::ptr_eq(&before.doc, &after.doc));
assert_eq!(before.selection, after.selection);
assert_eq!(before.revision, after.revision);
assert_eq!(format!("{:?}", before.history), format!("{:?}", after.history));
assert_eq!(journal, session.journal);
}
#[test]
fn char_fill_rejects_unknown_flat_and_nested_names_when_formatting() {
for fill in ["Missing Ink", "#ff0000", "[black]", " [Black]"] {
for params in [json!({"fill": fill, "size": 24}), json!({"attrs": {"fill": fill, "size": 24}})] {
let mut session = fill_session();
assert_fill_rejected(&mut session, "type.char", params);
}
}
}
#[test]
fn char_fill_rejects_unknown_names_when_creating_or_editing_styles() {
for command in ["style.paragraph.create", "style.paragraph.edit", "style.character.create", "style.character.edit"] {
for fill in ["Missing Ink", "#ff0000"] {
let mut session = fill_session();
session.execute("style.paragraph.apply", &json!({"name": "Existing"})).unwrap();
session.execute("style.character.apply", &json!({"name": "Existing"})).unwrap();
assert_fill_rejected(&mut session, command, json!({"name": "Existing", "rename": "Renamed", "chars": {"fill": fill, "size": 24}}));
}
}
}
#[test]
fn char_fill_rejects_inherited_unknown_name_when_creating_from_selection() {
let mut session = fill_session();
session
.edit(|document, selection| {
let selected = selection.text.unwrap();
document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into()));
Ok(())
})
.unwrap();
assert_fill_rejected(&mut session, "style.paragraph.create", json!({"name": "Inherited", "fromSelection": true}));
}
#[test]
fn char_fill_accepts_valid_override_when_selection_has_unknown_name() {
let mut session = fill_session();
session
.edit(|document, selection| {
let selected = selection.text.unwrap();
document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into()));
Ok(())
})
.unwrap();
session.execute("style.paragraph.create", &json!({"name": "Override", "fromSelection": true, "chars": {"fill": "[Paper]"}})).unwrap();
assert_eq!(session.doc().unwrap().doc.styles.para("Override").unwrap().chars.fill.as_deref(), Some("[Paper]"));
}
#[test]
fn char_fill_rejects_unknown_name_when_changing_text_even_without_output() {
for (find, change) in [("Hi", "Bye"), ("Hi", ""), ("Absent", "Bye")] {
for fill in ["Missing Ink", "#ff0000"] {
let mut session = fill_session();
assert_fill_rejected(&mut session, "find.change", json!({"find": find, "change": change, "attrs": {"fill": fill}}));
}
}
}
#[test]
fn char_fill_preserves_exact_registered_names_when_used_at_all_boundaries() {
for fill in ["Brand Ink", "#ff0000", "[None]", "[Black]"] {
let mut session = fill_session();
if !fill.starts_with('[') {
let created = session.execute("swatch.create", &json!({"name": fill, "color": "#108080"})).unwrap();
assert_eq!(created["name"], fill);
}
for params in [json!({"fill": fill}), json!({"attrs": {"fill": fill}})] {
session.execute("type.char", &params).unwrap();
assert_eq!(session.execute("type.selectionAttrs", &json!({})).unwrap()["chars"]["fill"], fill);
}
for kind in ["paragraph", "character"] {
session.execute(&format!("style.{kind}.create"), &json!({"name": "Colored", "chars": {"fill": fill}})).unwrap();
session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": {"fill": fill}})).unwrap();
let document = &session.doc().unwrap().doc;
for name in ["Colored", "Existing"] {
let attrs = if kind == "paragraph" {
&document.styles.para(name).unwrap().chars
} else {
&document.styles.char_style(name).unwrap().chars
};
assert_eq!(attrs.fill.as_deref(), Some(fill));
}
}
assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": {"fill": fill}})).unwrap()["count"], 1);
let selected = session.doc().unwrap().selection.text.unwrap();
let story = session.doc().unwrap().doc.story(selected.story).unwrap();
assert_eq!(story.text, "Bye");
assert_eq!(story.format_after(0).over.fill.as_deref(), Some(fill));
}
}
#[test]
fn char_fill_accepts_omitted_and_null_attributes_when_used_at_all_boundaries() {
for chars in [json!({"size": 18}), json!({"fill": null})] {
let mut session = fill_session();
session.execute("type.char", &json!({"attrs": chars})).unwrap();
for kind in ["paragraph", "character"] {
session.execute(&format!("style.{kind}.create"), &json!({"name": "Nullable", "chars": chars})).unwrap();
session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": chars})).unwrap();
}
assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": chars})).unwrap()["count"], 1);
}
}
#[test]
fn percent_components_at_most_one_stay_percent() {
use designcraft_color::Color;
+13
View File
@@ -819,6 +819,18 @@ fn warichu_cmd(s: &mut Session, p: &Value) -> Result<Value> {
format_chars(s, &Value::Object(body))
}
pub(crate) fn validate_char_fill(document: &designcraft_doc::Document, attrs: &CharAttrs, command_id: &str) -> Result<()> {
if let Some(fill) = &attrs.fill
&& document.swatch(fill).is_none()
{
return Err(bad(
command_id,
format!("unknown character fill `{fill}`; create a swatch with `swatch.create` and use its returned name as `fill`"),
));
}
Ok(())
}
pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result<Value> {
let mut a = CharAttrs::default();
if let Some(o) = attrs.as_object() {
@@ -826,6 +838,7 @@ pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result<Value> {
a.set_json(k, v).map_err(|e| bad("type.char", e))?;
}
}
validate_char_fill(&s.doc()?.doc, &a, "type.char")?;
let cleared: Vec<String> = attrs.as_object().map(|o| o.iter().filter(|(_, v)| v.is_null()).map(|(k, _)| k.clone()).collect()).unwrap_or_default();
let targets = format_targets(s);
s.edit(|d, _| {