Fix #5369: every way of opening a file focuses its open tab

Squashed from #5420 onto main.

crates/ui-egui/src/canvas/home.rs

@@ -74,7 +74,7 @@ fn recent(app: &mut VectorcraftApp, ui: &mut Ui) {
     for path in files {
         if recent_row(ui, &path).clicked() {
             // A failure is reported to the user there.
-            let _ = crate::io::open_recent_reporting(app, &path);
+            let _ = crate::io::open_reporting(app, &path);
         }
     }
 }

crates/ui-egui/src/dialogs/find_links/tests.rs

@@ -63,6 +63,8 @@ fn opened_without(app: &mut VectorcraftApp, root: &Path, names: &[&str]) -> Vec<
     std::fs::create_dir_all(doc.parent().unwrap()).unwrap();
     app.run("document.save", json!({"path": doc})).unwrap();
     std::fs::remove_dir_all(root.join("orig")).unwrap();
+    // Reopened: an open file would only come to the front (#3455).
+    app.run("file.close", json!({})).unwrap();
     crate::io::open_path(app, &doc.to_string_lossy()).unwrap();
     files
 }
@@ -271,6 +273,7 @@ fn copy_leaves_placed_documents_where_they_are() {
     std::fs::create_dir_all(doc.parent().unwrap()).unwrap();
     app.run("document.save", json!({"path": doc})).unwrap();
     std::fs::remove_dir_all(root.join("orig")).unwrap();
+    app.run("file.close", json!({})).unwrap();
     crate::io::open_path(&mut app, &doc.to_string_lossy()).unwrap();
     super::super::cancel(&mut app);
     app.run("ui.findLinksInFolder", json!({"folder": root.join("found")})).unwrap();

crates/ui-egui/src/dialogs/missing_fonts/tests.rs

@@ -49,6 +49,8 @@ fn save_doc(app: &mut VectorcraftApp, path: &Path, font: &str, link: bool) {
     if link {
         std::fs::remove_file(&pic).unwrap();
     }
+    // Opened afresh by `open`: an open file would only come to the front (#3455).
+    app.run("file.close", json!({})).unwrap();
 }
 
 fn open(app: &mut VectorcraftApp, path: &Path) {
@@ -135,6 +137,8 @@ fn find_fonts_opens_find_font_on_the_missing_font() {
     app.run("text.create", json!({"x": 10, "y": 80, "text": "Gone", "font": "Zz Missing Family"})).unwrap();
     let doc = root.join("doc.vectorcraft");
     app.run("document.save", json!({"path": doc.to_string_lossy()})).unwrap();
+    // Opened afresh: an open file would only come to the front (#5369).
+    app.run("file.close", json!({})).unwrap();
     open(&mut app, &doc);
     assert_eq!(kind(&app), Some(KIND));
     app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true));
@@ -223,6 +227,7 @@ fn one_dialog_at_a_time_document_by_document() {
     super::super::cancel(&mut app);
     assert!(app.ui.dialog.is_none());
     // A document closed before its turn is passed over.
+    app.run("file.closeAll", json!({})).unwrap();
     open(&mut app, &a);
     open(&mut app, &b);
     let index = app.session.documents().iter().rposition(|d| d.path.as_deref() == Some(&*a.to_string_lossy())).unwrap();
@@ -295,6 +300,24 @@ fn a_search_folder_that_fails_is_not_tried_again() {
     assert_eq!(super::super::confirm(&mut app).err().as_deref(), Some("choose the font files to add"));
 }
 
+/// Opening a file that is already open only brings its tab to the front: Missing Fonts, closed
+/// once, is not asked again (#5369).
+#[test]
+fn reopening_an_open_document_does_not_ask_about_its_fonts_again() {
+    let root = folder("reopen");
+    let mut app = app_for(&root);
+    let doc = root.join("doc.vectorcraft");
+    save_doc(&mut app, &doc, "No Such Font Family", false);
+    open(&mut app, &doc);
+    assert_eq!(kind(&app), Some(KIND));
+    super::super::cancel(&mut app);
+    app.run("file.new", json!({})).unwrap();
+    open(&mut app, &doc);
+    frame(&mut app);
+    assert_eq!((app.session.documents().len(), app.session.active_index()), (2, Some(0)));
+    assert_eq!(kind(&app), None);
+}
+
 /// A placed document that Links › Edit Original opens asks about its fonts, as File › Open does.
 #[test]
 fn edit_original_asks_about_the_placed_documents_fonts() {
@@ -302,7 +325,6 @@ fn edit_original_asks_about_the_placed_documents_fonts() {
     let mut app = app_for(&root);
     let badge = root.join("badge.vectorcraft");
     save_doc(&mut app, &badge, "Badge Missing Family", false);
-    app.session.execute("file.close", &json!({})).unwrap();
     app.run("file.new", json!({"width": 400, "height": 300})).unwrap();
     app.run("file.place", json!({"path": badge.to_string_lossy(), "link": true})).unwrap();
     let tabs = app.session.documents().len();

crates/ui-egui/src/dialogs/missing_links.rs

@@ -249,6 +249,8 @@ mod tests {
         for n in ["a.png", "b.png"] {
             std::fs::remove_file(dir.join(n)).unwrap();
         }
+        // Reopened: an open file would only come to the front (#3455).
+        app.run("file.close", json!({})).unwrap();
         crate::io::open_path(&mut app, &doc).unwrap();
         (app, dir, doc)
     }
@@ -344,6 +346,8 @@ mod tests {
         };
         std::fs::write(&pic, png([0, 0, 200])).unwrap();
         mark_modified(2);
+        // Reopened: an open file would only come to the front (#3455).
+        app.run("file.close", json!({})).unwrap();
         crate::io::open_path(&mut app, &doc).unwrap();
         let d = app.ui.dialog.as_ref().expect("asks");
         assert_eq!((d.kind.as_str(), d.str("__command")), (super::super::confirm::KIND, "links.update".to_string()));
@@ -353,6 +357,7 @@ mod tests {
         app.run("prefs.set", json!({"key": "updateLinks", "value": "manually"})).unwrap();
         std::fs::write(&pic, png([0, 200, 0])).unwrap();
         mark_modified(4);
+        app.session.execute("file.close", &json!({})).unwrap();
         crate::io::open_path(&mut app, &doc).unwrap();
         assert!(app.ui.dialog.is_none());
         assert_eq!(app.run("links.check", json!({})).unwrap()["modified"], 1);

crates/ui-egui/src/io.rs

@@ -24,7 +24,7 @@ pub(crate) use export::{
 pub(crate) use open::{EXPORT_PRESETS, import_presets, opened};
 pub use open::{
     MAX_RECENT_FILES, OpensAs, ask_revert, install_module, install_plugin, new_from_template, note_recent, open_bytes, open_dialog, open_document,
-    open_path, open_recent_reporting, open_reporting, opens_as, place_bytes, recent_files, report_open_error,
+    open_path, open_reporting, opens_as, place_bytes, recent_files, report_open_error,
 };
 #[cfg(test)]
 pub(crate) use open::{open_file, opened_with_notes};

crates/ui-egui/src/io/open.rs

@@ -229,15 +229,9 @@ pub fn open_reporting(app: &mut VectorcraftApp, path: &str) -> Result<Value, Str
     r
 }
 
-/// Home › Recent Files and File › Open Recent: focus an already-open tab for that path instead of
-/// calling `document.open` again (#3455).
-pub fn open_recent_reporting(app: &mut VectorcraftApp, path: &str) -> Result<Value, String> {
-    if let Some(r) = try_activate_open_document(app, path)? {
-        return Ok(r);
-    }
-    open_reporting(app, path)
-}
-
+/// A file already open in a tab is not opened again: its tab comes to the front, unsaved edits
+/// and all (#3455), without the questions asked after an open (missing links and fonts).
+/// → `{activated: true, …}` as `document.open`'s result, or None.
 fn try_activate_open_document(app: &mut VectorcraftApp, path: &str) -> Result<Option<Value>, String> {
     let Some(index) = document_index_for_path(app, path) else {
         return Ok(None);
@@ -252,7 +246,7 @@ fn try_activate_open_document(app: &mut VectorcraftApp, path: &str) -> Result<Op
         "activated": true,
     });
     app.run("document.activate", json!({ "index": index }))?;
-    opened(app, &r);
+    app.sync_views();
     Ok(Some(r))
 }
 
@@ -279,6 +273,14 @@ pub fn open_path(app: &mut VectorcraftApp, path: &str) -> Result<Value, String>
 /// [`open_path`]; with `background`, a document (not a file that installs, imports or asks
 /// first) is decoded off the UI thread ([`crate::opening::start`]).
 pub(crate) fn open_file(app: &mut VectorcraftApp, path: &str, background: bool) -> Result<Value, String> {
+    // File › Open, Open Recent, the Home screen, a drop and the Finder all focus a document that
+    // is already open, or still being read, instead of opening a second copy.
+    if let Some(r) = try_activate_open_document(app, path)? {
+        return Ok(r);
+    }
+    if app.opening.pending.iter().any(|p| same_open_path(&p.path, path)) {
+        return Ok(Value::Null);
+    }
     // A shortcut set is read by Import Shortcut Set, up to its reader's cap.
     if dialogs::shortcut_import::is_kys(path) {
         dialogs::shortcut_import::command(app, &json!({ "path": path }))?;

crates/ui-egui/src/menus/run.rs

@@ -126,7 +126,7 @@ fn run_ui_part1(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Result<
         "file.cancelExport" => Ok(io::cancel_exports(app)),
         "file.exportInBackground" => Ok(io::hide_exports(app)),
         id if id.starts_with("file.openRecent") => match recent_slot(app, id).cloned() {
-            Some(path) => io::open_recent_reporting(app, &path),
+            Some(path) => io::open_reporting(app, &path),
             None => Err("no such recent file".into()),
         },
         "type.findFont" => {

crates/ui-egui/src/opening/tests.rs

@@ -332,3 +332,19 @@ fn scope_dir() -> std::path::PathBuf {
     std::fs::create_dir_all(&dir).unwrap();
     dir
 }
+
+/// A file asked for again while it is still being read is not read a second time (#3455).
+#[test]
+fn a_file_asked_for_again_while_it_is_read_opens_once() {
+    let mut app = VectorcraftApp::new(Session::new(), Services::default());
+    let bytes = vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false);
+    start(&mut app, "/art/Poster.vectorcraft", bytes).unwrap();
+    assert_eq!(crate::io::open_file(&mut app, "/art/Poster.vectorcraft", true).unwrap(), Value::Null);
+    assert_eq!(app.opening.pending.len(), 1);
+    let t0 = std::time::Instant::now();
+    while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 {
+        poll(&mut app);
+        std::thread::yield_now();
+    }
+    assert_eq!(app.session.documents().len(), 1);
+}

crates/ui-egui/src/tests/home.rs

@@ -299,10 +299,11 @@ fn home_scrolls_in_a_short_window() {
     assert_eq!(app.session.documents().len(), 1, "the card opened its preset");
 }
 
-/// Home › Recent Files and File › Open Recent focus an open tab instead of opening a second copy
-/// (#3455); `document.open` is unchanged.
+/// A file that is already open is not opened again, however it is asked for: Home › Recent Files,
+/// File › Open Recent, File › Open, the Finder and a drop all focus its tab, unsaved edits and all
+/// (#3455).
 #[test]
-fn home_recent_reopen_focuses_an_already_open_document() {
+fn reopening_an_open_file_focuses_its_tab() {
     let services = crate::Services {
         read: Some(Box::new(|p: &str| std::fs::read(p).map_err(|e| e.to_string()))),
         write: Some(Box::new(|p: &str, b: &[u8]| std::fs::write(p, b).map_err(|e| e.to_string()))),
@@ -316,13 +317,35 @@ fn home_recent_reopen_focuses_an_already_open_document() {
     app.run("file.new", json!({"width": 100, "height": 100})).unwrap();
     app.run("shape.rectangle", json!({"x": 1, "y": 2, "width": 10, "height": 10})).unwrap();
     app.run("file.saveAs", json!({"path": path})).unwrap();
-    app.run("file.new", json!({})).unwrap();
-    assert_eq!(app.session.documents().len(), 2);
-    assert_eq!(app.session.active_index(), Some(1));
-    let r = crate::io::open_recent_reporting(&mut app, &path).unwrap();
+    // An edit not saved yet.
+    app.run("shape.rectangle", json!({"x": 20, "y": 2, "width": 10, "height": 10})).unwrap();
+    let back = |app: &mut VectorcraftApp| {
+        app.run("file.new", json!({})).unwrap();
+        assert_eq!(app.session.documents().len(), 2);
+    };
+    let focused = |app: &VectorcraftApp, how: &str| {
+        assert_eq!(app.session.documents().len(), 2, "{how}: no second copy");
+        assert_eq!(app.session.active_index(), Some(0), "{how}");
+        assert_eq!(app.session.doc().unwrap().doc.layers[0].children().unwrap().len(), 2, "{how}: the unsaved edit is kept");
+        app.session.doc().unwrap().uid
+    };
+    back(&mut app);
+    // Home › Recent Files, File › Open Recent, File › Open… and the Finder.
+    let r = crate::io::open_reporting(&mut app, &path).unwrap();
     assert_eq!(r["activated"], true);
-    assert_eq!(app.session.documents().len(), 2);
-    assert_eq!(app.session.active_index(), Some(0));
-    assert_eq!(app.session.doc().unwrap().doc.layers[0].children().unwrap().len(), 1);
+    let uid = focused(&app, "open_reporting");
+    app.run("document.activate", json!({"index": 1})).unwrap();
+    app.run("file.close", json!({})).unwrap();
+    back(&mut app);
+    // File › Open with a path (an open dialog's answer, `app.open`), and a path spelled differently.
+    let other = d.join(".").join("doc.vectorcraft").to_string_lossy().to_string();
+    assert_eq!(app.run("file.open", json!({"path": other})).unwrap()["activated"], true);
+    assert_eq!(focused(&app, "file.open"), uid);
+    app.run("document.activate", json!({"index": 1})).unwrap();
+    app.run("file.close", json!({})).unwrap();
+    back(&mut app);
+    // A drop.
+    crate::place::drop_files(&mut app, vec![(crate::place::DropTarget::Open, ("doc.vectorcraft".into(), Some(path.clone()), vec![]))]);
+    assert_eq!(focused(&app, "drop"), uid);
     let _ = std::fs::remove_dir_all(d);
 }

crates/ui-egui/src/tests/save.rs

@@ -232,6 +232,8 @@ fn a_document_reopens_at_its_saved_view() {
     // Moving around afterwards isn't an edit.
     app.view_mut().unwrap().zoom = 0.5;
     assert!(!app.session.active().unwrap().is_dirty());
+    // Reopened: an open file would only come to the front (#3455).
+    app.run("file.close", json!({})).unwrap();
     io::open_path(&mut app, &path).unwrap();
     let v = *app.view().unwrap();
     assert_eq!((v.zoom, v.center, v.rotation, v.fitted), (3.0, Point::new(70.0, 40.0), 30.0, true));