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>
This commit is contained in:
FredApps
2026-11-01 14:58:21 +01:00
co-authored by Codex GPT-6.1
parent d423dd03ec
commit 0595d4d397
3 changed files with 63 additions and 6 deletions
+13
View File
@@ -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 {
+11 -6
View File
@@ -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")) {
+39
View File
@@ -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));
}