From 94da1af75326d89ecb12aba0cc9362e93ffdc766 Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Fri, 20 Dec 2024 20:46:04 -0500 Subject: [PATCH] Handle INCR selections properly Closes #82 --- src/xstate/mod.rs | 12 +- src/xstate/selection.rs | 266 +++++++++++++++++++++++++++++++++------- tests/integration.rs | 185 +++++++++++++++++++++++----- 3 files changed, 388 insertions(+), 75 deletions(-) diff --git a/src/xstate/mod.rs b/src/xstate/mod.rs index d1b5cb4..d430ee8 100644 --- a/src/xstate/mod.rs +++ b/src/xstate/mod.rs @@ -626,12 +626,15 @@ impl XState { } fn handle_property_change( - &self, + &mut self, event: x::PropertyNotifyEvent, server_state: &mut super::RealServerState, ) { if event.state() != x::Property::NewValue { - debug!("ignoring non newvalue for property {:?}", event.atom()); + debug!( + "ignoring non newvalue for property {:?}", + get_atom_name(&self.connection, event.atom()) + ); return; } @@ -663,7 +666,9 @@ impl XState { server_state.set_win_class(window, class); } _ => { - if log::log_enabled!(log::Level::Debug) { + if !self.handle_selection_property_change(&event, server_state) + && log::log_enabled!(log::Level::Debug) + { debug!( "changed property {:?} for {:?}", get_atom_name(&self.connection, event.atom()), @@ -698,6 +703,7 @@ xcb::atoms_struct! { pub multiple => b"MULTIPLE" only_if_exists = false, pub timestamp => b"TIMESTAMP" only_if_exists = false, pub selection_reply => b"_selection_reply" only_if_exists = false, + pub incr => b"INCR" only_if_exists = false, } } diff --git a/src/xstate/selection.rs b/src/xstate/selection.rs index 5e226dc..03723f7 100644 --- a/src/xstate/selection.rs +++ b/src/xstate/selection.rs @@ -32,6 +32,7 @@ impl MimeTypeData for SelectionTarget { fn data(&self) -> &[u8] { match &self.value { TargetValue::U8(v) => v, + TargetValue::U32(v) => unsafe { v.align_to().1 }, other => { warn!( "Unexpectedly requesting data from mime type with data type {} - nothing will be copied", @@ -43,13 +44,23 @@ impl MimeTypeData for SelectionTarget { } } +enum PendingMimeDataType { + Standard, + Incremental(TargetValue), +} + +struct PendingMimeData { + ty: PendingMimeDataType, + id: SelectionTargetId, + dest_property: x::Atom, +} + enum MimeTypes { Temporary { /// Temporary mime data, being built data: Vec, /// Mime types we still need to receive feedback on - /// 2nd field is the destination property - to_grab: Vec<(SelectionTargetId, x::Atom)>, + to_grab: Vec, }, /// Done grabbing mime data Complete(Rc>), @@ -118,7 +129,7 @@ impl XState { self.selection_data.mime_types = MimeTypes::Complete(Rc::new(types)); self.selection_data.foreign_data = Some(selection); - trace!("Clipboard set from Wayland"); + debug!("Clipboard set from Wayland"); } pub(crate) fn handle_selection_event( @@ -157,19 +168,23 @@ impl XState { return true; } + trace!( + "selection notify target: {}", + get_atom_name(&self.connection, e.target()) + ); match e.target() { x if x == self.atoms.targets => self.handle_target_list(e.property()), atom => self.handle_clipboard_data(atom), } - if let MimeTypes::Temporary { data, to_grab } = &mut self.selection_data.mime_types - { + if let MimeTypes::Temporary { to_grab, .. } = &self.selection_data.mime_types { if to_grab.is_empty() { - let data = Rc::new(std::mem::take(data)); - self.selection_data.mime_types = MimeTypes::Complete(data.clone()); - self.set_clipboard_owner(self.selection_data.clear_time.unwrap()); - server_state.set_copy_paste_source(data); - trace!("Clipboard set from X11"); + let MimeTypes::Temporary { data, .. } = + std::mem::take(&mut self.selection_data.mime_types) + else { + unreachable!() + }; + self.finish_mime_data(server_state, data); } } } @@ -320,26 +335,27 @@ impl XState { only_if_exists: false, })) .unwrap(); - let dest = reply.atom(); + let dest_property = reply.atom(); self.connection .send_and_check_request(&x::ConvertSelection { requestor: self.wm_window, selection: self.atoms.clipboard, target: target_atom, - property: dest, + property: dest_property, time: self.selection_data.clear_time.as_ref().copied().unwrap(), }) .unwrap(); let target_name = get_atom_name(&self.connection, target_atom); - ( - SelectionTargetId { + PendingMimeData { + ty: PendingMimeDataType::Standard, + id: SelectionTargetId { name: target_name, atom: target_atom, }, - dest, - ) + dest_property, + } }) .collect(); @@ -355,7 +371,10 @@ impl XState { return; }; - let Some(idx) = to_grab.iter().position(|(id, _)| id.atom == atom) else { + let Some(idx) = to_grab + .iter() + .position(|PendingMimeData { id, .. }| id.atom == atom) + else { warn!( "unexpected SelectionNotify type: {}", get_atom_name(&self.connection, atom) @@ -363,7 +382,11 @@ impl XState { return; }; - let (id, dest) = to_grab.swap_remove(idx); + let PendingMimeData { + ty, + id, + dest_property, + } = to_grab.swap_remove(idx); let value = match atom { x if x == self.atoms.timestamp => TargetValue::U32(vec![self @@ -373,36 +396,47 @@ impl XState { .copied() .unwrap()]), _ => { - let reply = self - .connection - .wait_for_reply(self.connection.send_request(&x::GetProperty { - delete: true, - window: self.wm_window, - property: dest, - r#type: x::ATOM_ANY, - long_offset: 0, - long_length: u32::MAX, - })) - .unwrap(); + let reply = get_property_any(&self.connection, self.wm_window, dest_property); - match reply.format() { - 8 => TargetValue::U8(reply.value().to_vec()), - 16 => TargetValue::U16(reply.value().to_vec()), - 32 => TargetValue::U32(reply.value().to_vec()), - other => { - if log::log_enabled!(log::Level::Debug) { - let target_name = &id.name; - let ty = if reply.r#type() == x::ATOM_NONE { - "None".to_string() - } else { - get_atom_name(&self.connection, reply.r#type()) - }; - let dest = get_atom_name(&self.connection, dest); - let value = reply.value::().to_vec(); - debug!("unexpected format: {other} (atom: {target_name}, type: {ty:?}, property: {dest}, value: {value:?})"); + trace!( + "got type {} for mime type {}", + get_atom_name(&self.connection, reply.r#type()), + get_atom_name(&self.connection, atom) + ); + + match reply.r#type() { + x if x == self.atoms.incr => { + assert!(matches!(ty, PendingMimeDataType::Standard)); + debug!( + "beginning incr process for {}", + get_atom_name(&self.connection, atom) + ); + if let Some(data) = + begin_incr(&self.connection, self.wm_window, reply, id, dest_property) + { + to_grab.push(data); } return; } + _ => match reply.format() { + 8 => TargetValue::U8(reply.value().to_vec()), + 16 => TargetValue::U16(reply.value().to_vec()), + 32 => TargetValue::U32(reply.value().to_vec()), + other => { + if log::log_enabled!(log::Level::Debug) { + let target_name = &id.name; + let ty = if reply.r#type() == x::ATOM_NONE { + "None".to_string() + } else { + get_atom_name(&self.connection, reply.r#type()) + }; + let dest = get_atom_name(&self.connection, dest_property); + let value = reply.value::().to_vec(); + debug!("unexpected format: {other} (atom: {target_name}, type: {ty:?}, property: {dest}, value: {value:?})"); + } + return; + } + }, } } }; @@ -410,4 +444,148 @@ impl XState { trace!("Selection data: {id:?} {value:?}"); data.push(SelectionTarget { id, value }); } + + pub(super) fn handle_selection_property_change( + &mut self, + event: &x::PropertyNotifyEvent, + server_state: &mut RealServerState, + ) -> bool { + if event.window() != self.wm_window { + return false; + } + + let MimeTypes::Temporary { data, to_grab } = &mut self.selection_data.mime_types else { + debug!("Got potential selection property change, but not awaiting mime data"); + return false; + }; + + let Some(idx) = to_grab.iter().position(|p| { + matches!(p.ty, PendingMimeDataType::Incremental(_)) && p.dest_property == event.atom() + }) else { + debug!( + "Changed non selection property: {}", + get_atom_name(&self.connection, event.atom()) + ); + return false; + }; + + let pending = &mut to_grab[idx]; + let reply = get_property_any(&self.connection, self.wm_window, pending.dest_property); + + if reply.r#type() != pending.id.atom { + warn!( + "wrong getproperty type: {}", + get_atom_name(&self.connection, reply.r#type()) + ); + return false; + } + + match reply.format() { + 8 => { + let value: &[u8] = reply.value(); + trace!("got incr data ({} bytes)", value.len()); + if value.is_empty() { + let pending = to_grab.swap_remove(idx); + let PendingMimeDataType::Incremental(value) = pending.ty else { + unreachable!() + }; + let atom = pending.id.atom; + data.push(SelectionTarget { + id: pending.id, + value, + }); + trace!( + "finalized incr for {}", + get_atom_name(&self.connection, atom) + ); + } else { + let PendingMimeDataType::Incremental(TargetValue::U8(data)) = &mut pending.ty + else { + unreachable!() + }; + data.extend_from_slice(value); + trace!("new incr len: {}", data.len()); + } + } + other => { + warn!("Got unexpected format {other} for INCR data - copy/pasting with mime type {} will fail!", get_atom_name(&self.connection, reply.r#type())); + to_grab.swap_remove(idx); + } + } + + if to_grab.is_empty() { + let MimeTypes::Temporary { data, .. } = + std::mem::take(&mut self.selection_data.mime_types) + else { + unreachable!() + }; + self.finish_mime_data(server_state, data); + } + + true + } + + fn finish_mime_data(&mut self, server_state: &mut RealServerState, data: Vec) { + self.connection + .send_and_check_request(&x::ChangeWindowAttributes { + window: self.wm_window, + value_list: &[x::Cw::EventMask(x::EventMask::empty())], + }) + .unwrap(); + let data = Rc::new(data); + self.selection_data.mime_types = MimeTypes::Complete(data.clone()); + self.set_clipboard_owner(self.selection_data.clear_time.unwrap()); + server_state.set_copy_paste_source(data); + debug!("Clipboard set from X11"); + } +} + +fn get_property_any( + connection: &xcb::Connection, + window: x::Window, + property: x::Atom, +) -> x::GetPropertyReply { + connection + .wait_for_reply(connection.send_request(&x::GetProperty { + delete: true, + window, + property, + r#type: x::ATOM_ANY, + long_offset: 0, + long_length: u32::MAX, + })) + .unwrap() +} +fn begin_incr( + connection: &xcb::Connection, + window: x::Window, + reply: x::GetPropertyReply, + id: SelectionTargetId, + dest_property: x::Atom, +) -> Option { + let size = match reply.format() { + 8 => reply.value::()[0] as usize, + 16 => reply.value::()[0] as usize, + 32 => reply.value::()[0] as usize, + other => { + warn!("unexpected incr format: {other}"); + return None; + } + }; + + connection + .send_and_check_request(&x::ChangeWindowAttributes { + window, + value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)], + }) + .unwrap(); + + // XXX: storing INCR property data in memory could significantly bloat memory depending on how + // much data is going to be stuck into the clipboard, but we'll cross that bridge when we get + // to it. + Some(PendingMimeData { + ty: PendingMimeDataType::Incremental(TargetValue::U8(Vec::with_capacity(size))), + id, + dest_property, + }) } diff --git a/tests/integration.rs b/tests/integration.rs index aa20a95..886b0f4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -268,6 +268,7 @@ xcb::atoms_struct! { wm_check => b"_NET_SUPPORTING_WM_CHECK", mime1 => b"text/plain" only_if_exists = false, mime2 => b"blah/blah" only_if_exists = false, + incr => b"INCR", } } @@ -383,6 +384,47 @@ impl Connection { { self.wait_for_reply(self.send_request(req)).unwrap() } + + #[track_caller] + fn set_selection_owner(&self, window: x::Window) { + self.send_and_check_request(&x::SetSelectionOwner { + owner: window, + selection: self.atoms.clipboard, + time: x::CURRENT_TIME, + }) + .unwrap(); + let owner = self + .wait_for_reply(self.send_request(&x::GetSelectionOwner { + selection: self.atoms.clipboard, + })) + .unwrap(); + assert_eq!(window, owner.owner()); + } + + #[track_caller] + fn send_selection_notify(&self, request: &x::SelectionRequestEvent) { + self.send_and_check_request(&x::SendEvent { + propagate: false, + destination: x::SendEventDest::Window(request.requestor()), + event_mask: x::EventMask::empty(), + event: &x::SelectionNotifyEvent::new( + request.time(), + request.requestor(), + request.selection(), + request.target(), + request.property(), + ), + }) + .unwrap(); + } + + #[track_caller] + fn atom_name(&self, atom: x::Atom) -> String { + self.get_reply(&x::GetAtomName { atom }) + .name() + .as_ascii() + .to_string() + } } #[test] @@ -683,20 +725,7 @@ fn copy_from_x11() { .expect("No surface created"); f.configure_and_verify_new_toplevel(&mut connection, window, surface); - // set data - connection - .send_and_check_request(&x::SetSelectionOwner { - owner: window, - selection: connection.atoms.clipboard, - time: x::CURRENT_TIME, - }) - .unwrap(); - let owner = connection - .wait_for_reply(connection.send_request(&x::GetSelectionOwner { - selection: connection.atoms.clipboard, - })) - .unwrap(); - assert_eq!(window, owner.owner()); + connection.set_selection_owner(window); // wait for requests to come through std::thread::sleep(std::time::Duration::from_millis(100)); @@ -712,20 +741,7 @@ fn copy_from_x11() { request.property(), &[connection.atoms.mime1, connection.atoms.mime2], ); - connection - .send_and_check_request(&x::SendEvent { - propagate: false, - destination: x::SendEventDest::Window(request.requestor()), - event_mask: x::EventMask::empty(), - event: &x::SelectionNotifyEvent::new( - request.time(), - request.requestor(), - request.selection(), - request.target(), - request.property(), - ), - }) - .unwrap(); + connection.send_selection_notify(&request); connection.await_event(); let mut mime_data = vec![ @@ -1197,3 +1213,116 @@ fn primary_output() { let reply = conn.get_reply(&xcb::randr::GetOutputPrimary { window: conn.root }); assert_eq!(reply.output(), output3); } + +// TODO: these sleeps are horrible. +#[test] +fn incr_copy_from_x11() { + let mut f = Fixture::new(); + let mut connection = Connection::new(&f.display); + + let window = connection.new_window(connection.root, 0, 0, 20, 20, false); + f.map_as_toplevel(&mut connection, window); + + connection.set_selection_owner(window); + std::thread::sleep(std::time::Duration::from_millis(10)); + let request = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r, + other => panic!("Didn't get selection request event, instead got {other:?}"), + }; + assert_eq!(request.target(), connection.atoms.targets); + connection.set_property( + request.requestor(), + x::ATOM_ATOM, + request.property(), + &[connection.atoms.targets, connection.atoms.mime1], + ); + connection.send_selection_notify(&request); + connection.await_event(); + + let request = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r, + other => panic!("Didn't get selection request event, instead got {other:?}"), + }; + assert_eq!(request.target(), connection.atoms.mime1); + let destination_property = request.property(); + + connection + .send_and_check_request(&x::ChangeWindowAttributes { + window: request.requestor(), + value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)], + }) + .unwrap(); + connection.set_property( + request.requestor(), + connection.atoms.incr, + destination_property, + &[3000u32], + ); + connection.send_selection_notify(&request); + // skip NewValue + let notify = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p, + other => panic!("Didn't get property notify event, instead got {other:?}"), + }; + assert_eq!(notify.atom(), request.property()); + assert_eq!(notify.state(), x::Property::NewValue); + + let data: Vec = std::iter::successors(Some(1u8), |n| Some(n.wrapping_add(1))) + .take(3000) + .collect(); + for (idx, chunk) in data.chunks(500).enumerate() { + std::thread::sleep(std::time::Duration::from_millis(10)); + let notify = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p, + other => panic!("Didn't get property notify event, instead got {other:?}"), + }; + assert_eq!(notify.atom(), destination_property, "chunk {idx}"); + assert_eq!(notify.state(), x::Property::Delete, "chunk {idx}"); + + connection.set_property( + request.requestor(), + connection.atoms.mime1, + destination_property, + chunk, + ); + // skip NewValue + let notify = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p, + other => panic!("Didn't get property notify event, instead got {other:?}"), + }; + assert_eq!(notify.atom(), destination_property, "chunk {idx}"); + assert_eq!(notify.state(), x::Property::NewValue, "chunk {idx}"); + } + + std::thread::sleep(std::time::Duration::from_millis(10)); + let notify = match connection.poll_for_event().unwrap() { + Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p, + other => panic!("Didn't get property notify event, instead got {other:?}"), + }; + assert_eq!(notify.atom(), destination_property); + assert_eq!(notify.state(), x::Property::Delete); + connection.set_property::( + request.requestor(), + connection.atoms.mime1, + destination_property, + &[], + ); + + std::thread::sleep(std::time::Duration::from_millis(100)); + let owner = connection + .wait_for_reply(connection.send_request(&x::GetSelectionOwner { + selection: connection.atoms.clipboard, + })) + .unwrap(); + assert_ne!(window, owner.owner()); + f.wait_and_dispatch(); + + assert_eq!(f.testwl.data_source_mimes(), vec!["text/plain"]); + let wl_data = f.testwl.paste_data(); + f.testwl.dispatch(); + let mut wl_data = wl_data.resolve(); + assert_eq!(wl_data.len(), 1); + let wl_data = wl_data.swap_remove(0); + assert_eq!(wl_data.mime_type, "text/plain"); + assert_eq!(&wl_data.data, &data); +}