Merge pull request #6225 from slafebre/fix/6135-ai-open-without-import-dialog

Fix #6135: opening a .ai with editing data skips the Import PDF dialog

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

@@ -114,17 +114,19 @@ pub(super) fn show_preview(ui: &mut egui::Ui, tex: Option<&egui::TextureHandle>,
 
 /// Open `bytes` (or place them with the `file.place` params `place`) through the dialog when the
 /// file is a PDF (or `.ai`/`.ait`) with several pages or a password; false when there's nothing to
-/// ask (the caller goes on), including a file that carries its VectorCraft document when opening.
+/// ask (the caller goes on), including, when opening, a file that carries its VectorCraft document
+/// and a `.ai`/`.ait` with its editing data.
 pub fn offer(app: &mut VectorcraftApp, name: &str, bytes: &[u8], path: Option<String>, place: Option<&Value>) -> bool {
-    if !fileio::detect(name, bytes).is_some_and(|f| matches!(f.id, "pdf" | "ai" | "ait")) {
+    let Some(format) = fileio::detect(name, bytes).map(|f| f.id).filter(|id| matches!(*id, "pdf" | "ai" | "ait")) else {
         return false;
-    }
+    };
     // A file carrying its VectorCraft document (a saved .ai, or a PDF saved with Preserve Editing)
-    // opens as that document, every artboard and layer: there are no pages to pick. Placing still
-    // asks, since a place takes one page.
+    // opens as that document, every artboard and layer: there are no pages to pick. So does a .ai
+    // or .ait with its editing data, which holds them all (#6135); a PDF with editing data still
+    // asks, as its pages are what it is. Placing still asks, since a place takes one page.
     // The file is read once for both checks.
     let opened = vectorcraft_pdf::open_pdf(bytes, None);
-    if place.is_none() && opened.as_ref().is_ok_and(|o| o.editing().is_some()) {
+    if place.is_none() && opened.as_ref().is_ok_and(|o| o.editing().is_some() || (format != "pdf" && o.has_illustrator_data())) {
         return false;
     }
     let (pages, locked) = match opened.map(|o| o.page_count()) {

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

@@ -3,7 +3,8 @@
 use serde_json::json;
 use vectorcraft_doc::NodeKind;
 use vectorcraft_engine::Session;
-use vectorcraft_testkit::pdf::{PdfPage, pdf};
+use vectorcraft_testkit::ai;
+use vectorcraft_testkit::pdf::{PdfPage, first_extra, pdf, pdf_with};
 
 use super::*;
 use crate::io;
@@ -197,3 +198,72 @@ fn placing_a_saved_ai_still_asks_for_the_page() {
     assert!(import_pdf::offer(&mut app, "two.ai", &bytes, None, Some(&json!({}))));
     assert_eq!(app.ui.dialog.as_ref().unwrap().str("mode"), "place");
 }
+
+/// Two pages, 100 and 200 pt wide; with `editing_data` the first page has the keys of an editor's
+/// copy of the art.
+fn two_pages(editing_data: bool, password: Option<&str>) -> Vec<u8> {
+    let mut first = PdfPage::new(100.0, 100.0, "0 g 0 0 10 10 re f");
+    if editing_data {
+        first.entries = "/PieceInfo << /Illustrator << /Private << /AIPrivateData1 7 /NumBlock 1 >> >> >>".into();
+    }
+    pdf(&[first, PdfPage::new(200.0, 100.0, "1 0 0 rg 0 0 30 30 re f")], password)
+}
+
+/// Two 100 pt pages and, on the first, editing data with two artboards to match and a layer of its
+/// own, which draws the square the first page draws.
+fn two_pages_with_a_layer_in_the_editing_data(layer: &str) -> Vec<u8> {
+    let point = |x: i32, y: i32, n: u8| format!("%_{x} {y} /RealPointRelToROrigin\n%_ (PositionPoint{n}) ,\n");
+    let board = |top: i32| format!("%_/Dictionary :\n{}{}%_; ,\n", point(0, top, 1), point(100, top - 100, 2));
+    let boards = format!("%_/Document :\n%_/Dictionary :\n%_/Array :\n{}{}%_; (ArtboardArray) ,\n%_; /NotRecorded ,\n%_;\n", board(100), board(-50));
+    let art = format!("0 0 0 1 k\n{}f\n", ai::rect(10.0, 10.0, 30.0, 30.0));
+    let data = ai::editing_data(100.0, 100.0, &format!("{boards}{}", ai::layer(layer, &art)));
+    let mut first = PdfPage::new(100.0, 100.0, "0 0 0 1 k 10 10 20 20 re f");
+    first.entries = format!("/PieceInfo << /Illustrator << /Private << /AIPrivateData1 {} 0 R /NumBlock 1 >> >> >>", first_extra(2));
+    let stream = format!("<< /Length {} >>\nstream\n{data}\nendstream", data.len());
+    pdf_with(&[first, PdfPage::new(100.0, 100.0, "")], &[&stream], None)
+}
+
+/// #6135: the editing data of a `.ai` holds every artboard, so opening one asks nothing, opens
+/// them all and reads the art from that data: the layer is in nothing else.
+#[test]
+fn opening_an_ai_with_editing_data_reads_it_without_asking() {
+    let mut app = app();
+    let bytes = two_pages_with_a_layer_in_the_editing_data("Only in the editing data");
+    io::open_bytes(&mut app, "two.ai", &bytes, Some("/tmp/two.ai".into())).unwrap();
+    assert!(app.ui.dialog.is_none(), "no Import PDF dialog");
+    assert_eq!(app.session.documents().len(), 2);
+    assert_eq!(artboard_widths(&app), [100.0, 100.0]);
+    let layers = &app.session.active().unwrap().doc.layers;
+    assert!(layers.iter().any(|l| l.name.as_deref() == Some("Only in the editing data")), "read from the editing data");
+    assert!(!import_pdf::offer(&mut app, "two.ait", &bytes, None, None), "a template too");
+    assert!(app.ui.dialog.is_none());
+}
+
+/// A `.ai` whose editing data can't be read asks nothing either, and every page opens from the PDF
+/// part. Asking at that point is #6134.
+#[test]
+fn opening_an_ai_with_unreadable_editing_data_opens_every_page_without_asking() {
+    let mut app = app();
+    io::open_bytes(&mut app, "two.ai", &two_pages(true, None), Some("/tmp/two.ai".into())).unwrap();
+    assert!(app.ui.dialog.is_none(), "no Import PDF dialog");
+    assert_eq!(artboard_widths(&app), [100.0, 200.0]);
+}
+
+/// #6135: the dialog stays for what still has something to ask: a PDF, even one with editing data
+/// keys, a `.ai` that is only a PDF, a password, and Place.
+#[test]
+fn the_import_dialog_stays_where_there_is_something_to_ask() {
+    for (name, bytes) in [("two.pdf", two_pages(true, None)), ("two.ai", two_pages(false, None))] {
+        let mut app = app();
+        assert!(import_pdf::offer(&mut app, name, &bytes, None, None), "{name}");
+        assert_eq!((app.ui.dialog.as_ref().unwrap().str("mode"), field(&app, "pageCount")), ("open".into(), json!(2)), "{name}");
+    }
+
+    let mut app = app();
+    assert!(import_pdf::offer(&mut app, "locked.ai", &two_pages(true, Some("pw")), None, None));
+    assert_eq!(field(&app, "__locked"), true);
+
+    let mut app = self::app();
+    assert!(import_pdf::offer(&mut app, "two.ai", &two_pages(true, None), None, Some(&json!({}))));
+    assert_eq!(app.ui.dialog.as_ref().unwrap().str("mode"), "place");
+}

docs/control-protocol.md

@@ -126,7 +126,7 @@ the two can still win that race.
 | `engine.execute` `file.exportSelection` | `{multiple?}` | File › Export Selection…: collects the selected objects as one asset (`assets.add`; one per object with `multiple: true`) and opens `exportForScreens` on its Assets tab with just them checked → `{assets}`. `document.exportSelection` still exports the selection in one go, without the dialog |
 | CSS Properties panel | `window.panel {panel: "cssProperties"}`; `engine.execute` `css.copy` / `css.exportFile` | the panel shows `css.selection` of the selection (after Generate CSS, while nothing is selected, `css.generate`); its menu holds the options the commands take (`units`, `position`, `dimensions`, `unnamed`, `rasterize`). `css.copy {scope?, …options}` copies the CSS to the clipboard → `{css, rules}`; `css.exportFile {path?, scope?, …options}` writes `css.export` to `path` (else a picked file; the web downloads it) with the pictures of rasterized art next to it → `{path, rules, images, warnings}`, and the status bar shows the first of `warnings` as Export As shows an export's notes |
 
-| `ui.dialog.*` on `importPdf` | `{field: "page" \| "allPages" \| "range" \| "cropTo" \| "password", value}` | opening (`app.open`, `file.open`) a PDF or `.ai` with several pages or a password, or placing one (`file.place` without `page`), opens the Import PDF dialog (`mode`: `open` or `place`; `pageCount`). While `__locked` is true, set `password` and `ui.dialog.confirm` to unlock it (a one-page file then opens or is placed). Then set `allPages: false` and `range` ("2-3, 5") to open some pages, or `page` to place one, and `cropTo` (`bounding`, `art`, `crop`, `trim`, `bleed`, `media`); `ui.dialog.confirm` opens or places them (placing keeps the other `file.place` params, such as `link`, `template` and `at`, in `__place`). `document.open {pages, cropTo, password}` and `file.place {page, crop, password}` do the same without the dialog |
+| `ui.dialog.*` on `importPdf` | `{field: "page" \| "allPages" \| "range" \| "cropTo" \| "password", value}` | opening (`app.open`, `file.open`) a PDF, `.ai` or `.ait` with a password, or one with several pages unless it is a `.ai` or `.ait` with its editing data or carries its VectorCraft document (those open whole), or placing (`file.place` without `page`) any of them with several pages or a password, opens the Import PDF dialog (`mode`: `open` or `place`; `pageCount`). While `__locked` is true, set `password` and `ui.dialog.confirm` to unlock it (a one-page file then opens or is placed). Then set `allPages: false` and `range` ("2-3, 5") to open some pages, or `page` to place one, and `cropTo` (`bounding`, `art`, `crop`, `trim`, `bleed`, `media`); `ui.dialog.confirm` opens or places them (placing keeps the other `file.place` params, such as `link`, `template` and `at`, in `__place`). `document.open {pages, cropTo, password}` and `file.place {page, crop, password}` do the same without the dialog |
 
 Swatch editors: `engine.execute` with `ui.swatchOptions {name}` opens the `swatchOptions` dialog for a colour swatch
 (fields `name`, `spot`, `global`, `mode`: `gray`/`rgb`/`hsb`/`lab`/`cmyk`/`web`, `color`: `"#rrggbb"` or a colour object,