mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-02 22:41:47 -06:00
fix(ui): respect clipboard authority and retain refused cut selections
Do not reuse stale native images for unsupported clipboard contents, while preserving web internal image paste. Observe text paste before focus gating and keep image-less key releases silent. Tie release fallback to V gestures. Check modification permission before copying and clear selection only after successful deletion. Co-Authored-By: Codex GPT-6.1 <noreply@openai.com>
This commit is contained in:
co-authored by
Codex GPT-6.1
parent
36a6a572b8
commit
a310d5a41a
@@ -25,6 +25,17 @@ impl PdfCraftApp {
|
||||
/// ⌘C / ⌘X / ⌘V copy, cut and paste text or images on canvas (unless a text field has focus).
|
||||
pub(super) fn clipboard_keys(&mut self, i: usize, ctx: &egui::Context, pasted: bool) {
|
||||
use egui::{Key, KeyboardShortcut, Modifiers};
|
||||
let paste_event_text = ctx.input(|input| {
|
||||
input.events.iter().find_map(|e| match e {
|
||||
egui::Event::Paste(text) if !text.is_empty() => Some(text.clone()),
|
||||
_ => None,
|
||||
})
|
||||
});
|
||||
if paste_event_text.is_some() {
|
||||
// A text field may own the paste; the canvas cache must still observe the new contents.
|
||||
self.last_copied_image = None;
|
||||
self.last_copied_file = None;
|
||||
}
|
||||
let free = self.dialog.is_none() && !self.palette_open && !ctx.egui_wants_keyboard_input() && !self.is_preparing();
|
||||
if !free || self.views.get(i).is_some_and(|v| v.forms.focus.is_some() || v.organize) {
|
||||
return;
|
||||
@@ -33,12 +44,6 @@ impl PdfCraftApp {
|
||||
|| ctx.input_mut(|input| input.consume_shortcut(&KeyboardShortcut::new(Modifiers::COMMAND, Key::C)));
|
||||
let cut = ctx.input(|input| input.events.iter().any(|e| matches!(e, egui::Event::Cut)))
|
||||
|| ctx.input_mut(|input| input.consume_shortcut(&KeyboardShortcut::new(Modifiers::COMMAND, Key::X)));
|
||||
let paste_event_text = ctx.input(|input| {
|
||||
input.events.iter().find_map(|e| match e {
|
||||
egui::Event::Paste(text) if !text.is_empty() => Some(text.clone()),
|
||||
_ => None,
|
||||
})
|
||||
});
|
||||
let paste = pasted
|
||||
|| ctx.input(|input| input.events.iter().any(|e| matches!(e, egui::Event::Paste(_))))
|
||||
|| ctx.input_mut(|input| input.consume_shortcut(&KeyboardShortcut::new(Modifiers::COMMAND, Key::V)));
|
||||
@@ -56,12 +61,14 @@ impl PdfCraftApp {
|
||||
&& let Some(id) = self.views.get(i).map(|v| v.id)
|
||||
&& self.may_copy(id)
|
||||
{
|
||||
if let Some(v) = self.views.get_mut(i) {
|
||||
v.image_selection = None;
|
||||
}
|
||||
self.cut_page_image(sel.page, sel.index);
|
||||
}
|
||||
if paste {
|
||||
// A release is only a fallback for image shortcuts whose press the platform swallowed.
|
||||
// Text paste already reached its owner; an empty image clipboard needs no second notice.
|
||||
if pasted && paste_event_text.is_none() && !self.can_paste_image() {
|
||||
return;
|
||||
}
|
||||
let has_override_image = matches!(self.clipboard_override, Some(crate::create_ui::Clip::Image { .. }));
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
let system_has_text = if !has_override_image && self.system_clipboard { crate::os_clipboard::has_text() } else { false };
|
||||
@@ -126,3 +133,55 @@ impl PdfCraftApp {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod review_tests {
|
||||
use super::*;
|
||||
#[test]
|
||||
fn review_an_image_less_release_is_silent_but_explicit_paste_explains() {
|
||||
let mut app = PdfCraftApp::new();
|
||||
app.system_clipboard = false;
|
||||
app.create_blank();
|
||||
app.toast = None;
|
||||
let ctx = egui::Context::default();
|
||||
ctx.run_ui(egui::RawInput::default(), |ui| app.clipboard_keys(0, ui.ctx(), true)).drop_without_applying_deltas();
|
||||
assert!(app.toast.is_none());
|
||||
assert!(!app.paste_image(0, None));
|
||||
assert!(app.toast.is_some(), "an explicit menu action still explains why there is no image");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_text_paste_release_is_silent_and_later_image_paste_still_works() {
|
||||
let mut app = PdfCraftApp::new();
|
||||
app.system_clipboard = false;
|
||||
app.create_blank();
|
||||
app.last_copied_image = Some((1, 1, vec![255, 0, 0, 255]));
|
||||
app.palette_open = true; // A text field owns the first event.
|
||||
let ctx = egui::Context::default();
|
||||
ctx.run_ui(egui::RawInput { events: vec![egui::Event::Paste("text".into())], ..Default::default() }, |ui| {
|
||||
app.clipboard_keys(0, ui.ctx(), false)
|
||||
})
|
||||
.drop_without_applying_deltas();
|
||||
assert!(app.last_copied_image.is_none());
|
||||
app.palette_open = false;
|
||||
app.toast = None;
|
||||
ctx.run_ui(egui::RawInput::default(), |ui| app.clipboard_keys(0, ui.ctx(), true)).drop_without_applying_deltas();
|
||||
assert!(app.toast.is_none(), "the text gesture's key release must be silent");
|
||||
app.clipboard_override = Some(crate::create_ui::Clip::Image { width: 1, height: 1, rgba: vec![0, 255, 0, 255] });
|
||||
ctx.run_ui(egui::RawInput::default(), |ui| app.clipboard_keys(0, ui.ctx(), true)).drop_without_applying_deltas();
|
||||
let id = app.active_ids().unwrap().1;
|
||||
assert_eq!(app.session.get(id).unwrap().page_images(0).len(), 1, "a later real image shortcut still works");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_failed_keyboard_cut_keeps_the_image_selection() {
|
||||
let mut app = PdfCraftApp::new();
|
||||
app.system_clipboard = false;
|
||||
app.create_blank();
|
||||
app.views[0].image_selection = Some(crate::edit_text_ui::ImageSelection::at(0, 0));
|
||||
let ctx = egui::Context::default();
|
||||
ctx.run_ui(egui::RawInput { events: vec![egui::Event::Cut], ..Default::default() }, |ui| app.clipboard_keys(0, ui.ctx(), false))
|
||||
.drop_without_applying_deltas();
|
||||
assert!(app.views[0].image_selection.is_some());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -946,10 +946,19 @@ impl crate::PdfCraftApp {
|
||||
}
|
||||
|
||||
/// Cut: copy the picture, and only when that worked take it off the page (one undo step).
|
||||
pub(crate) fn cut_page_image(&mut self, page: usize, index: usize) {
|
||||
if self.copy_page_image(page, index) {
|
||||
self.apply_edit(Edit::EditPageImage { page, index, change: pdfcraft_engine::ImageEdit::Delete });
|
||||
pub(crate) fn cut_page_image(&mut self, page: usize, index: usize) -> bool {
|
||||
let Some((view, id)) = self.active_ids() else { return false };
|
||||
if self.session.get(id).is_some_and(|doc| !doc.allows_modification()) {
|
||||
self.notify_error(tl!("The document's security settings don't allow this change"));
|
||||
return false;
|
||||
}
|
||||
if !self.copy_page_image(page, index) || !self.apply_edit(Edit::EditPageImage { page, index, change: pdfcraft_engine::ImageEdit::Delete }) {
|
||||
return false;
|
||||
}
|
||||
if let Some(view) = self.views.get_mut(view) {
|
||||
view.image_selection = None;
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Redact image: a redaction mark over the whole picture. It is applied, like any mark, from
|
||||
@@ -1027,13 +1036,12 @@ impl crate::PdfCraftApp {
|
||||
pub(crate) fn paste_key_released(&mut self, ctx: &egui::Context) -> bool {
|
||||
let mut released = false;
|
||||
ctx.input(|inp| {
|
||||
if inp.modifiers.command {
|
||||
self.last_command_time = Some(inp.time);
|
||||
}
|
||||
for e in &inp.events {
|
||||
if let egui::Event::Key { key: egui::Key::V, pressed, modifiers, .. } = e {
|
||||
if modifiers.command {
|
||||
self.last_command_time = Some(inp.time);
|
||||
} else if *pressed {
|
||||
self.last_command_time = None;
|
||||
}
|
||||
let command_recent = self.last_command_time.is_some_and(|t| inp.time >= t && inp.time - t < 0.35);
|
||||
let command_active = modifiers.command || command_recent;
|
||||
@@ -1058,11 +1066,7 @@ impl crate::PdfCraftApp {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
if self.system_clipboard {
|
||||
match crate::os_clipboard::for_image_paste() {
|
||||
Some(crate::create_ui::Clip::Image { .. }) => return true,
|
||||
Some(crate::create_ui::Clip::Text(_)) => return false,
|
||||
None => {}
|
||||
}
|
||||
return matches!(crate::os_clipboard::for_image_paste(), Some(crate::create_ui::Clip::Image { .. }));
|
||||
}
|
||||
}
|
||||
self.last_copied_image.is_some() || self.last_copied_file.is_some()
|
||||
@@ -1083,10 +1087,17 @@ impl crate::PdfCraftApp {
|
||||
let system = if self.clipboard_override.is_none() && self.system_clipboard { crate::os_clipboard::for_image_paste() } else { None };
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
let system = None;
|
||||
self.paste_image_with_system(page, rect, at, system)
|
||||
self.paste_image_with_system(page, rect, at, system, cfg!(not(target_arch = "wasm32")) && self.system_clipboard)
|
||||
}
|
||||
|
||||
fn paste_image_with_system(&mut self, page: usize, rect: Option<[f64; 4]>, at: Option<[f64; 2]>, system: Option<crate::create_ui::Clip>) -> bool {
|
||||
fn paste_image_with_system(
|
||||
&mut self,
|
||||
page: usize,
|
||||
rect: Option<[f64; 4]>,
|
||||
at: Option<[f64; 2]>,
|
||||
system: Option<crate::create_ui::Clip>,
|
||||
system_authoritative: bool,
|
||||
) -> bool {
|
||||
let Some((i, id)) = self.active_ids() else {
|
||||
return false;
|
||||
};
|
||||
@@ -1111,7 +1122,7 @@ impl crate::PdfCraftApp {
|
||||
}
|
||||
Some(crate::create_ui::Clip::Text(_)) => None,
|
||||
None => {
|
||||
if matches!(system, Some(crate::create_ui::Clip::Text(_))) {
|
||||
if matches!(system, Some(crate::create_ui::Clip::Text(_))) || (system_authoritative && system.is_none()) {
|
||||
self.last_copied_image = None;
|
||||
self.last_copied_file = None;
|
||||
self.notify_tr("No image on clipboard to paste");
|
||||
@@ -1297,6 +1308,82 @@ mod tests {
|
||||
|
||||
use super::*;
|
||||
|
||||
fn review_v_release(app: &mut crate::PdfCraftApp, ctx: &egui::Context, time: f64, events: Vec<egui::Event>) -> bool {
|
||||
let mut paste = false;
|
||||
ctx.run_ui(egui::RawInput { time: Some(time), events, ..Default::default() }, |ui| {
|
||||
paste = app.paste_key_released(ui.ctx());
|
||||
})
|
||||
.drop_without_applying_deltas();
|
||||
paste
|
||||
}
|
||||
|
||||
fn review_key(key: egui::Key, pressed: bool, modifiers: egui::Modifiers) -> egui::Event {
|
||||
egui::Event::Key { key, physical_key: None, pressed, repeat: false, modifiers }
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_plain_v_after_another_shortcut_does_not_paste() {
|
||||
let mut app = crate::PdfCraftApp::new();
|
||||
let ctx = egui::Context::default();
|
||||
review_v_release(
|
||||
&mut app,
|
||||
&ctx,
|
||||
1.0,
|
||||
vec![egui::Event::ModifiersChanged(egui::Modifiers::COMMAND), review_key(egui::Key::C, true, egui::Modifiers::COMMAND)],
|
||||
);
|
||||
assert!(!review_v_release(
|
||||
&mut app,
|
||||
&ctx,
|
||||
1.1,
|
||||
vec![egui::Event::ModifiersChanged(egui::Modifiers::NONE), review_key(egui::Key::V, false, egui::Modifiers::NONE)]
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_web_can_paste_an_internal_image_without_a_native_clipboard() {
|
||||
let mut app = crate::PdfCraftApp::new();
|
||||
app.create_blank();
|
||||
app.system_clipboard = true;
|
||||
app.last_copied_image = Some((1, 1, vec![255, 0, 0, 255]));
|
||||
assert!(app.paste_image_with_system(0, None, None, None, false));
|
||||
let id = app.active_ids().unwrap().1;
|
||||
assert_eq!(app.session.get(id).unwrap().page_images(0).len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_system_without_supported_formats_does_not_paste_a_cached_image() {
|
||||
let mut app = crate::PdfCraftApp::new();
|
||||
app.create_blank();
|
||||
app.system_clipboard = true;
|
||||
app.last_copied_image = Some((1, 1, vec![255, 0, 0, 255]));
|
||||
assert!(!app.paste_image_with_system(0, None, None, None, true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_cut_checks_modify_permission_before_writing_the_clipboard() {
|
||||
let mut app = crate::PdfCraftApp::new();
|
||||
app.system_clipboard = false;
|
||||
app.create_blank();
|
||||
let png = crate::create_ui::png_from_rgba(1, 1, &[255, 0, 0, 255]).unwrap();
|
||||
app.add_image("test.png".into(), png);
|
||||
app.apply_edit(Edit::Protect(pdfcraft_engine::Protection {
|
||||
permissions_password: Some("owner".into()),
|
||||
changes: pdfcraft_engine::Changes::None,
|
||||
copy: true,
|
||||
..Default::default()
|
||||
}));
|
||||
let id = app.active_ids().unwrap().1;
|
||||
let bytes = app.session.save_bytes(id).unwrap();
|
||||
let mut locked = crate::PdfCraftApp::new();
|
||||
locked.system_clipboard = false;
|
||||
locked.open_bytes("locked.pdf", None, bytes.as_ref().clone()).unwrap();
|
||||
assert!(!locked.session.get(locked.active_ids().unwrap().1).unwrap().allows_modification());
|
||||
locked.last_copied_image = Some((1, 1, vec![0, 255, 0, 255]));
|
||||
let before = locked.last_copied_image.clone();
|
||||
assert!(!locked.cut_page_image(0, 0));
|
||||
assert_eq!(locked.last_copied_image, before);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn system_text_prevents_context_menu_paste_of_a_cached_image() {
|
||||
let mut app = crate::PdfCraftApp::new();
|
||||
@@ -1305,7 +1392,7 @@ mod tests {
|
||||
let id = app.active_ids().unwrap().1;
|
||||
app.last_copied_image = Some((1, 1, vec![255, 0, 0, 255]));
|
||||
let undo = app.session.get(id).unwrap().can_undo().map(str::to_owned);
|
||||
assert!(!app.paste_image_with_system(0, None, Some([20.0, 20.0]), Some(crate::create_ui::Clip::Text("new text".into()))));
|
||||
assert!(!app.paste_image_with_system(0, None, Some([20.0, 20.0]), Some(crate::create_ui::Clip::Text("new text".into())), true));
|
||||
assert_eq!(app.session.get(id).unwrap().can_undo().map(str::to_owned), undo);
|
||||
assert!(app.session.get(id).unwrap().page_images(0).is_empty());
|
||||
assert!(app.last_copied_image.is_none());
|
||||
|
||||
Reference in New Issue
Block a user