From 0595d4d3976492471bd24152367ffcc746dd9310 Mon Sep 17 00:00:00 2001
From: FredApps <153173854+FredApps@users.noreply.github.com>
Date: Sun, 1 Nov 2026 14:57:42 +0100
Subject: [PATCH] fix(ui): preserve author clips and route crop controls before
 page content

Offer Remove Crop only for editor-owned clips, reset draft cropping to the author-visible bounds, and remove only an owned crop when confirming the full image. Active crop mode owns presses on its page so underlying text and marquee selection cannot steal crop controls. Label the painted controls for accessibility and automation.

The author-clip regression opens the real menu, resets a draft crop and confirms it while preserving the authored clip. It failed before ownership checks and again before the router fix.

Co-Authored-By: Codex GPT-6.1 <noreply@openai.com>
---
 crates/ui-egui/src/canvas.rs                  | 13 +++++++
 .../ui-egui/src/tools/edit_text_ui/image.rs   | 17 +++++---
 crates/ui-egui/tests/it/editing.rs            | 39 +++++++++++++++++++
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs
index 8c91ba0cb3..13d6da88df 100644
--- a/crates/ui-egui/src/canvas.rs
+++ b/crates/ui-egui/src/canvas.rs
@@ -1791,6 +1791,19 @@ impl PageHits<'_> {
             images,
             clip,
         } = *self;
+        // Crop mode owns presses on its page, including the controls beside its
+        // frame. Underlying paragraphs and the empty-page marquee must not take
+        // a crop button's press before the image module receives it.
+        if tool == QuickTool::EditText
+            && can_modify
+            && let Some(selection) = view.image_selection.as_ref().filter(|s| s.page == page && s.crop.is_some())
+        {
+            return vec![crate::page_router::Hit::new(
+                crate::page_router::Owner::Picture { index: selection.index, part: crate::page_router::PicturePart::Body },
+                crate::page_cursor::Layer::Handle,
+                egui::CursorIcon::Default,
+            )];
+        }
         let fields = if preparing {
             crate::prepare::hit(xf, info, page, form, field_tool, prepare_allowed, &view.prepare, p)
         } else if tool == QuickTool::Select {
diff --git a/crates/ui-egui/src/tools/edit_text_ui/image.rs b/crates/ui-egui/src/tools/edit_text_ui/image.rs
index 2be091c1e6..caef96330a 100644
--- a/crates/ui-egui/src/tools/edit_text_ui/image.rs
+++ b/crates/ui-egui/src/tools/edit_text_ui/image.rs
@@ -286,8 +286,8 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
     let font = egui::FontId::proportional(12.5);
     let done_w = painter.layout_no_wrap(tl!("Done").to_string(), font.clone(), Color32::WHITE).size().x + 22.0;
     let cancel_w = painter.layout_no_wrap(tl!("Cancel").to_string(), font.clone(), Color32::WHITE).size().x + 22.0;
-    let is_full_rect = same_rect(mode.keep, image.rect);
-    let has_crop = !is_full_rect || image.visible_rect.is_some_and(|v| v != image.rect);
+    let initial_keep = image.visible_rect.unwrap_or(image.rect);
+    let has_crop = image.has_editor_crop() || !same_rect(mode.keep, initial_keep);
     let reset_w =
         if has_crop { Some(painter.layout_no_wrap(tl!("Remove Crop").to_string(), font.clone(), Color32::WHITE).size().x + 22.0) } else { None };
     let spacing = 6.0;
@@ -319,6 +319,12 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
     let done_resp = ui.interact(done, done_id, egui::Sense::click());
     let cancel_resp = ui.interact(cancel, cancel_id, egui::Sense::click());
     let reset_resp = reset.map(|r| ui.interact(r, reset_id, egui::Sense::click()));
+    done_resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, allow_interaction, tl!("Done")));
+    cancel_resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, allow_interaction, tl!("Cancel")));
+    if let Some(response) = &reset_resp {
+        response.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, allow_interaction, tl!("Remove Crop")));
+    }
+
     let t = crate::theme::Tokens::get(ui.ctx());
 
     // Done button (accent colored)
@@ -377,7 +383,7 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
             .is_some_and(|r| reset_resp.as_ref().is_some_and(|res| res.clicked()) || (resp.clicked() && hover_pos.is_some_and(|p| r.contains(p))));
 
     if reset_clicked {
-        mode.keep = image.rect;
+        mode.keep = if image.has_editor_crop() { image.rect } else { initial_keep };
         mode.drag = None;
     } else if done_clicked || keys(egui::Key::Enter) {
         let keep = mode.keep;
@@ -386,7 +392,7 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
         }
         let is_full = same_rect(keep, image.rect);
         if is_full {
-            if image.visible_rect.is_some_and(|v| v != image.rect) {
+            if image.has_editor_crop() {
                 view.pending_edit = Some(Edit::EditPageImage { page, index: i, change: pdfcraft_engine::ImageEdit::RemoveCrop });
             }
         } else if image.visible_rect != Some(keep) {
@@ -1018,8 +1024,7 @@ pub(crate) fn image_context_menu(
 
     ui.separator();
 
-    let is_cropped = img.visible_rect.is_some() && img.visible_rect != Some(img.rect);
-    if raster && item(ui, is_cropped, tl!("Remove crop")) {
+    if raster && img.has_editor_crop() && item(ui, true, tl!("Remove crop")) {
         view.pending_edit = edit(E::RemoveCrop);
     }
     if item(ui, true, tl!("Flip horizontal")) {
diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs
index ae32d2864e..a0f822ee89 100644
--- a/crates/ui-egui/tests/it/editing.rs
+++ b/crates/ui-egui/tests/it/editing.rs
@@ -9365,3 +9365,42 @@ fn outward_image_grip_drags_grow_on_rotated_pages() {
         }
     }
 }
+
+#[test]
+fn author_image_clip_is_not_offered_as_a_removable_editor_crop() {
+    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 1100.0)).build_eframe(|_cc| {
+        let mut app = PdfCraftApp::new();
+        app.set_option("language", "en").unwrap();
+        app.open_bytes("author-clip.pdf", None, clipped_artwork_with_labels()).unwrap();
+        app
+    });
+    h.run_steps(4);
+    assert!(h.state_mut().execute("edit.edit_text"));
+    h.state_mut().views[0].image_selection = Some(pdfcraft_ui_egui::ImageSelection::at(0, 0));
+    h.run_steps(3);
+    let view = &h.state().views[0];
+    let doc = h.state().session.get(view.id).unwrap();
+    let image = doc.page_images(0)[0].clone();
+    let visible = image.visible_rect.unwrap();
+    let xf = view.page_xform(0).unwrap();
+    let bounds = pdfcraft_ui_egui::canvas::image_screen_rect(&image, &xf, &doc.info, 0);
+    let p = bounds.center();
+    h.hover_at(p);
+    h.event(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Secondary, pressed: true, modifiers: Default::default() });
+    h.event(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Secondary, pressed: false, modifiers: Default::default() });
+    h.run_steps(2);
+    assert!(h.query_all_by_label("Remove crop").next().is_none(), "author clips are not editor crops");
+    h.get_by_label("Crop").click();
+    h.run_steps(2);
+    assert!(h.query_all_by_label("Remove Crop").next().is_none(), "the initial author clip has no removable draft crop");
+    let mut draft = visible;
+    draft[0] += (draft[2] - draft[0]) * 0.2;
+    h.state_mut().views[0].image_selection.as_mut().unwrap().crop.as_mut().unwrap().keep = draft;
+    h.run_steps(3);
+    h.get_by_label("Remove Crop").click();
+    h.run_steps(1);
+    assert_eq!(h.state().views[0].image_selection.as_ref().unwrap().crop.as_ref().unwrap().keep, visible, "draft reset preserves the author's clip");
+    h.get_by_label("Done").click();
+    h.run_steps(2);
+    assert_eq!(h.state().session.get(h.state().views[0].id).unwrap().page_images(0)[0].visible_rect, Some(visible));
+}