diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 47cf1ff24..351ca29dc 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -219,7 +219,7 @@ fn neighbour(s: &mut Session, p: &Value, forward: bool) -> Result { let cmd = if forward { "comment.next" } else { "comment.prev" }; let (threads, slide, index) = { let st = s.doc()?; - (comment_threads(&st.doc), usize_param(p, "slide").unwrap_or(st.selection.slide), usize_param(p, "index")) + (comment_threads(&st.doc), usize_param(p, "slide").unwrap_or(st.selection.slide), super::opt_usize_param(p, "index", cmd)?) }; if threads.is_empty() { return Err(bad(cmd, "no comments")); @@ -862,6 +862,40 @@ mod tests { assert_eq!(here["text"], "on first"); } + #[test] + fn malformed_comment_navigation_index_does_not_change_selection() { + let mut s = Session::with_new(); + s.execute("comment.add", &json!({"text": "first"})).unwrap(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [10.0, 10.0, 40.0, 40.0]})).unwrap(); + s.execute("slide.new", &json!({})).unwrap(); + s.execute("comment.add", &json!({"text": "second"})).unwrap(); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + let shape = s.doc().unwrap().doc.slides.first().and_then(|slide| slide.shapes.first()).map(|shape| shape.id).unwrap(); + s.doc_mut().unwrap().selection.shapes = vec![shape]; + + for (command, value) in [ + ("comment.next", json!(-1)), + ("comment.next", json!(1.5)), + ("comment.next", json!("bad")), + ("comment.prev", json!(-1)), + ("comment.prev", json!(1.5)), + ("comment.prev", json!("bad")), + ] { + let before = s.doc().unwrap().selection.clone(); + let error = s.execute(command, &json!({"index": value})).unwrap_err(); + assert!(error.to_string().contains("valid index"), "{command} {value}: {error}"); + assert_eq!(s.doc().unwrap().selection, before, "{command} {value}"); + } + + let zero = s.execute("comment.next", &json!({"slide": 0, "index": 0})).unwrap(); + assert_eq!(zero["slide"], 1); + assert_eq!(zero["index"], 0); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + let omitted = s.execute("comment.next", &json!({})).unwrap(); + assert_eq!(omitted["slide"], 0); + assert_eq!(omitted["index"], 0); + } + #[test] fn timestamps_are_iso_utc() { assert_eq!(iso_utc(0), "1970-01-01T00:00:00Z");