From b962a0f33b503aa39c9cf6919f488b664e5b79b4 Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Sun, 15 Sep 2024 01:28:32 -0400 Subject: [PATCH] Don't use MULTIPLE target atom for getting selections The ICCCM claims this is a "required" target for selection owners, however several GTK clients (zenity, winecfg) don't seem to support it. (So much for required.) Just manually grab all the supported targets individually from the selection owner instead. Fix for #50 --- src/xstate/mod.rs | 22 ++-- src/xstate/selection.rs | 267 +++++++++++++++++++++------------------- tests/integration.rs | 123 ++++++++---------- 3 files changed, 200 insertions(+), 212 deletions(-) diff --git a/src/xstate/mod.rs b/src/xstate/mod.rs index 23acd77..046d460 100644 --- a/src/xstate/mod.rs +++ b/src/xstate/mod.rs @@ -4,7 +4,7 @@ use selection::{SelectionData, SelectionTarget}; use crate::server::WindowAttributes; use bitflags::bitflags; use log::{debug, trace, warn}; -use std::ffi::{CString, CStr}; +use std::ffi::CString; use std::os::fd::{AsRawFd, BorrowedFd}; use std::sync::Arc; use xcb::{x, Xid, XidNew}; @@ -405,16 +405,6 @@ impl XState { } } - fn get_atom_name(&self, atom: x::Atom) -> String { - match self - .connection - .wait_for_reply(self.connection.send_request(&x::GetAtomName { atom })) - { - Ok(reply) => reply.name().to_string(), - Err(err) => format!(" {atom:?}"), - } - } - fn get_window_attributes(&self, window: x::Window) -> XResult { let geometry = self.connection.send_request(&x::GetGeometry { drawable: x::Drawable::Window(window), @@ -637,7 +627,7 @@ impl XState { if log::log_enabled!(log::Level::Debug) { debug!( "changed property {:?} for {:?}", - self.get_atom_name(event.atom()), + get_atom_name(&self.connection, event.atom()), window ); } @@ -665,6 +655,7 @@ xcb::atoms_struct! { pub utf8_string => b"UTF8_STRING" only_if_exists = false, pub clipboard => b"CLIPBOARD" only_if_exists = false, pub targets => b"TARGETS" only_if_exists = false, + pub save_targets => b"SAVE_TARGETS" only_if_exists = false, 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, @@ -864,3 +855,10 @@ impl super::FromServerState> for Atoms { state.atoms.as_ref().unwrap().clone() } } + +fn get_atom_name(connection: &xcb::Connection, atom: x::Atom) -> String { + match connection.wait_for_reply(connection.send_request(&x::GetAtomName { atom })) { + Ok(reply) => reply.name().to_string(), + Err(err) => format!(" {atom:?}"), + } +} diff --git a/src/xstate/selection.rs b/src/xstate/selection.rs index 13f2a18..5e226dc 100644 --- a/src/xstate/selection.rs +++ b/src/xstate/selection.rs @@ -1,4 +1,4 @@ -use super::XState; +use super::{get_atom_name, XState}; use crate::server::ForeignSelection; use crate::{MimeTypeData, RealServerState}; use log::{debug, trace, warn}; @@ -43,12 +43,28 @@ impl MimeTypeData for SelectionTarget { } } +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)>, + }, + /// Done grabbing mime data + Complete(Rc>), +} + +impl Default for MimeTypes { + fn default() -> Self { + Self::Complete(Default::default()) + } +} + #[derive(Default)] pub(crate) struct SelectionData { clear_time: Option, - // Selection ID and destination atom - tmp_mimes: Vec<(SelectionTargetId, x::Atom)>, - mime_types: Rc>, + mime_types: MimeTypes, foreign_data: Option, } @@ -100,7 +116,7 @@ impl XState { }) .collect(); - self.selection_data.mime_types = Rc::new(types); + self.selection_data.mime_types = MimeTypes::Complete(Rc::new(types)); self.selection_data.foreign_data = Some(selection); trace!("Clipboard set from Wayland"); } @@ -111,11 +127,13 @@ impl XState { server_state: &mut RealServerState, ) -> bool { match event { + // Someone else is the clipboard owner - get the data from them and then reestablish + // ourselves as the owner xcb::Event::X(x::Event::SelectionClear(e)) => { if e.selection() != self.atoms.clipboard { warn!( "Got SelectionClear for unexpected atom {}, ignoring", - self.get_atom_name(e.selection()) + get_atom_name(&self.connection, e.selection()) ); return true; } @@ -141,12 +159,17 @@ impl XState { match e.target() { x if x == self.atoms.targets => self.handle_target_list(e.property()), - x if x == self.atoms.multiple => self.handle_new_clipboard_data(server_state), - atom => { - warn!( - "unexpected SelectionNotify type: {}", - self.get_atom_name(atom) - ) + atom => self.handle_clipboard_data(atom), + } + + if let MimeTypes::Temporary { data, to_grab } = &mut 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"); } } } @@ -171,7 +194,7 @@ impl XState { let success = || send_notify(e.property()); if log::log_enabled!(log::Level::Debug) { - let target = self.get_atom_name(e.target()); + let target = get_atom_name(&self.connection, e.target()); debug!("Got selection request for target {target}"); } @@ -181,14 +204,15 @@ impl XState { return true; } + let MimeTypes::Complete(mime_data) = &self.selection_data.mime_types else { + warn!("Got selection request, but mime data is incomplete"); + refuse(); + return true; + }; + match e.target() { x if x == self.atoms.targets => { - let atoms: Box<[x::Atom]> = self - .selection_data - .mime_types - .iter() - .map(|t| t.id.atom) - .collect(); + let atoms: Box<[x::Atom]> = mime_data.iter().map(|t| t.id.atom).collect(); self.connection .send_and_check_request(&x::ChangeProperty { @@ -203,14 +227,9 @@ impl XState { success(); } other => { - let Some(target) = self - .selection_data - .mime_types - .iter() - .find(|t| t.id.atom == other) - else { + let Some(target) = mime_data.iter().find(|t| t.id.atom == other) else { if log::log_enabled!(log::Level::Debug) { - let name = self.get_atom_name(other); + let name = get_atom_name(&self.connection, other); debug!("refusing selection request because given atom could not be found ({})", name); } refuse(); @@ -272,127 +291,123 @@ impl XState { .unwrap(); let targets: &[x::Atom] = reply.value(); - let target_props: Box<[x::Atom]> = targets + if log::log_enabled!(log::Level::Debug) { + let targets_str: Vec = targets + .iter() + .map(|t| get_atom_name(&self.connection, *t)) + .collect(); + debug!("got targets: {targets_str:?}"); + } + + let to_grab = targets .iter() .copied() - .filter(|atom| ![self.atoms.targets, self.atoms.multiple].contains(atom)) + .filter(|atom| { + ![ + self.atoms.targets, + self.atoms.multiple, + self.atoms.save_targets, + ] + .contains(atom) + }) .enumerate() - .flat_map(|(idx, target)| { - let name = [b"dest", idx.to_string().as_bytes()].concat(); + .map(|(idx, target_atom)| { + let dest_name = [b"dest", idx.to_string().as_bytes()].concat(); let reply = self .connection .wait_for_reply(self.connection.send_request(&x::InternAtom { - name: &name, + name: &dest_name, only_if_exists: false, })) .unwrap(); let dest = reply.atom(); - [target, dest] - }) - .collect(); - - // Setup target list - self.connection - .send_and_check_request(&x::ChangeProperty { - mode: x::PropMode::Replace, - window: self.wm_window, - property: self.atoms.selection_reply, - r#type: x::ATOM_ATOM, - data: &target_props, - }) - .unwrap(); - - // Request data for our targets - self.connection - .send_and_check_request(&x::ConvertSelection { - requestor: self.wm_window, - selection: self.atoms.clipboard, - target: self.atoms.multiple, - property: self.atoms.selection_reply, - time: self.selection_data.clear_time.as_ref().copied().unwrap(), - }) - .unwrap(); - - let tmp = target_props - .chunks_exact(2) - .map(|atoms| { - let [target, property] = atoms.try_into().unwrap(); - let name = self - .connection - .wait_for_reply( - self.connection - .send_request(&x::GetAtomName { atom: target }), - ) + self.connection + .send_and_check_request(&x::ConvertSelection { + requestor: self.wm_window, + selection: self.atoms.clipboard, + target: target_atom, + property: dest, + time: self.selection_data.clear_time.as_ref().copied().unwrap(), + }) .unwrap(); - let name = name.name().to_string(); - let target = SelectionTargetId { atom: target, name }; - (target, property) + + let target_name = get_atom_name(&self.connection, target_atom); + ( + SelectionTargetId { + name: target_name, + atom: target_atom, + }, + dest, + ) }) .collect(); - self.selection_data.tmp_mimes = tmp; + self.selection_data.mime_types = MimeTypes::Temporary { + to_grab, + data: Vec::new(), + }; } - fn handle_new_clipboard_data(&mut self, server_state: &mut RealServerState) { - let mut mime_types = Vec::new(); - for (id, dest) in std::mem::take(&mut self.selection_data.tmp_mimes) { - let value = { - if id.atom == self.atoms.timestamp { - TargetValue::U32(vec![self - .selection_data - .clear_time - .as_ref() - .copied() - .unwrap()]) - } else { - 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(); + fn handle_clipboard_data(&mut self, atom: x::Atom) { + let MimeTypes::Temporary { data, to_grab } = &mut self.selection_data.mime_types else { + warn!("Got selection notify, but not awaiting selection 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 atom = id.atom; - let target = self.get_atom_name(atom); - let ty = if reply.r#type() == x::ATOM_NONE { - "None".to_string() - } else { - self.get_atom_name(reply.r#type()) - }; - debug!("unexpected format: {other} (atom: {target}, type: {ty:?}, property: {dest:?})"); - } - continue; + let Some(idx) = to_grab.iter().position(|(id, _)| id.atom == atom) else { + warn!( + "unexpected SelectionNotify type: {}", + get_atom_name(&self.connection, atom) + ); + return; + }; + + let (id, dest) = to_grab.swap_remove(idx); + + let value = match atom { + x if x == self.atoms.timestamp => TargetValue::U32(vec![self + .selection_data + .clear_time + .as_ref() + .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(); + + 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:?})"); } + return; } } - }; + } + }; - trace!("Selection data: {id:?} {value:?}"); - mime_types.push(SelectionTarget { id, value }); - } - - self.selection_data.mime_types = Rc::new(mime_types); - self.connection - .send_and_check_request(&x::DeleteProperty { - window: self.wm_window, - property: self.atoms.selection_reply, - }) - .unwrap(); - - self.set_clipboard_owner(self.selection_data.clear_time.unwrap()); - server_state.set_copy_paste_source(Rc::clone(&self.selection_data.mime_types)); - trace!("Clipboard set from X11"); + trace!("Selection data: {id:?} {value:?}"); + data.push(SelectionTarget { id, value }); } } diff --git a/tests/integration.rs b/tests/integration.rs index d0c92e4..4ddb297 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -575,7 +575,7 @@ fn copy_from_x11() { .unwrap(); assert_eq!(window, owner.owner()); - // wait for request to come through + // wait for requests to come through std::thread::sleep(std::time::Duration::from_millis(100)); let request = match connection.poll_for_event().unwrap() { Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r, @@ -604,61 +604,50 @@ fn copy_from_x11() { }) .unwrap(); - std::thread::sleep(std::time::Duration::from_millis(100)); - 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:?}"), - }; + connection.await_event(); + let mut mime_data = vec![ + ( + connection.atoms.mime1, + x::ATOM_STRING, + b"hello world".as_slice(), + ), + (connection.atoms.mime2, x::ATOM_INTEGER, &[1u8, 2, 3, 4]), + ]; - assert_eq!(request.target(), connection.atoms.multiple); - let pairs = connection - .wait_for_reply(connection.send_request(&x::GetProperty { - delete: true, - window: request.requestor(), - property: request.property(), - r#type: x::ATOM_ATOM, - long_offset: 0, - long_length: 4, - })) - .unwrap(); + while let Some(request) = connection.poll_for_event().unwrap() { + let xcb::Event::X(x::Event::SelectionRequest(request)) = request else { + continue; + }; - let pairs: &[x::Atom] = pairs.value(); - assert_eq!(pairs.len(), 4); - assert!(pairs.contains(&connection.atoms.mime1)); - assert!(pairs.contains(&connection.atoms.mime2)); + let target = request.target(); + let Some(idx) = mime_data.iter().position(|(atom, _, _)| *atom == target) else { + panic!("Expected atom in {mime_data:?}, got {target:?}"); + }; - let mime1data = b"hello world"; - let mime2data = &[1u8, 2, 3, 4]; - for [target, property] in pairs - .chunks_exact(2) - .map(|pair| <[x::Atom; 2]>::try_from(pair).unwrap()) - { - match target { - x if x == connection.atoms.mime1 => { - connection.set_property(request.requestor(), x::ATOM_STRING, property, mime1data); - } - x if x == connection.atoms.mime2 => { - connection.set_property(request.requestor(), x::ATOM_INTEGER, property, mime2data); - } - _ => panic!("unexpected target: {target:?}"), - } + let (_, ty, data) = mime_data.swap_remove(idx); + connection.set_property(request.requestor(), ty, request.property(), data); + + 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(); + std::thread::sleep(std::time::Duration::from_millis(50)); } - 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(); - + assert!( + mime_data.is_empty(), + "Didn't get all mime types: {mime_data:?}" + ); f.wait_and_dispatch(); let owner = connection @@ -684,10 +673,10 @@ fn copy_from_x11() { for testwl::PasteData { mime_type, data } in data { match mime_type { x if x == "text/plain" => { - assert_eq!(&data, mime1data); + assert_eq!(&data, b"hello world"); } x if x == "blah/blah" => { - assert_eq!(&data, mime2data); + assert_eq!(&data, &[1, 2, 3, 4]); } other => panic!("unexpected mime type: {other} ({data:?})"), } @@ -755,7 +744,7 @@ fn copy_from_wayland() { }) .unwrap(); - std::thread::sleep(std::time::Duration::from_millis(50)); + connection.await_event(); let request = match connection.poll_for_event().unwrap() { Some(xcb::Event::X(x::Event::SelectionNotify(r))) => r, other => panic!("Didn't get selection notify event, instead got {other:?}"), @@ -887,8 +876,7 @@ fn bad_clipboard_data() { }) .unwrap(); - // wait for request to come through - std::thread::sleep(std::time::Duration::from_millis(100)); + 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:?}"), @@ -915,26 +903,14 @@ fn bad_clipboard_data() { }) .unwrap(); - std::thread::sleep(std::time::Duration::from_millis(100)); + 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.multiple); - let pairs = connection - .wait_for_reply(connection.send_request(&x::GetProperty { - delete: true, - window: request.requestor(), - property: request.property(), - r#type: x::ATOM_ATOM, - long_offset: 0, - long_length: 4, - })) - .unwrap(); + assert_eq!(request.target(), connection.atoms.mime2); - let pairs: &[x::Atom] = pairs.value(); - assert_eq!(pairs.len(), 2); - assert!(pairs.contains(&connection.atoms.mime2)); + // Don't actually set any data as requested - just report success connection .send_and_check_request(&x::SendEvent { @@ -951,7 +927,7 @@ fn bad_clipboard_data() { }) .unwrap(); - f.wait_and_dispatch(); + std::thread::sleep(std::time::Duration::from_millis(50)); let owner = connection .wait_for_reply(connection.send_request(&x::GetSelectionOwner { selection: connection.atoms.clipboard, @@ -969,8 +945,7 @@ fn bad_clipboard_data() { }) .unwrap(); - std::thread::sleep(std::time::Duration::from_millis(100)); - + connection.await_event(); let mut e = None; while let Some(event) = connection.poll_for_event().unwrap() { if let xcb::Event::X(x::Event::SelectionNotify(event)) = event {