From 7f8ac28ea34a2329ca63b735f3f2d68d24269142 Mon Sep 17 00:00:00 2001 From: Billy Messenger <60663878+BillyDM@users.noreply.github.com> Date: Mon, 1 Apr 2024 15:40:48 -0500 Subject: [PATCH 1/8] add drag n drop support to x11 --- Cargo.toml | 2 + src/x11/drag_n_drop.rs | 176 ++++++++++++++++++++ src/x11/mod.rs | 1 + src/x11/window.rs | 218 ++++++++++++++++++++++++- src/x11/xcb_connection.rs | 25 ++- src/x11/xcb_connection/get_property.rs | 169 +++++++++++++++++++ 6 files changed, 582 insertions(+), 9 deletions(-) create mode 100644 src/x11/drag_n_drop.rs create mode 100644 src/x11/xcb_connection/get_property.rs diff --git a/Cargo.toml b/Cargo.toml index 2d65e46..275f45d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,8 @@ raw-window-handle = "0.5" x11rb = { version = "0.13.0", features = ["cursor", "resource_manager", "allow-unsafe-code"] } x11 = { version = "2.21", features = ["xlib", "xlib_xcb"] } nix = "0.22.0" +percent-encoding = "2.3.1" +bytemuck = "1.15.0" [target.'cfg(target_os="windows")'.dependencies] winapi = { version = "0.3.8", features = ["libloaderapi", "winuser", "windef", "minwindef", "guiddef", "combaseapi", "wingdi", "errhandlingapi", "ole2", "oleidl", "shellapi", "winerror"] } diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs new file mode 100644 index 0000000..aab94d5 --- /dev/null +++ b/src/x11/drag_n_drop.rs @@ -0,0 +1,176 @@ +// Code copied and modifed from https://github.com/rust-windowing/winit/blob/master/src/platform_impl/linux/x11/dnd.rs + +use std::{ + io, + os::raw::*, + path::{Path, PathBuf}, + str::Utf8Error, +}; + +use percent_encoding::percent_decode; +use x11rb::{ + errors::ConnectionError, + protocol::xproto::{self, ConnectionExt}, + x11_utils::Serialize, +}; + +use crate::{DropData, Point}; + +use super::xcb_connection::{GetPropertyError, XcbConnection}; + +pub(crate) struct DragNDrop { + // Populated by XdndEnter event handler + pub version: Option, + + pub type_list: Option>, + + // Populated by XdndPosition event handler + pub source_window: Option, + + // Populated by SelectionNotify event handler (triggered by XdndPosition event handler) + pub data: DropData, + + pub logical_pos: Point, +} + +impl DragNDrop { + pub fn new() -> Self { + Self { + version: None, + type_list: None, + source_window: None, + data: DropData::None, + logical_pos: Point::new(0.0, 0.0), + } + } + + pub fn reset(&mut self) { + self.version = None; + self.type_list = None; + self.source_window = None; + self.data = DropData::None; + self.logical_pos = Point::new(0.0, 0.0); + } + + pub fn send_status( + &self, this_window: xproto::Window, target_window: xproto::Window, accepted: bool, + conn: &XcbConnection, + ) -> Result<(), ConnectionError> { + let (accepted, action) = + if accepted { (1, conn.atoms.XdndActionPrivate) } else { (0, conn.atoms.None) }; + + let event = xproto::ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, + window: target_window, + format: 32, + data: [this_window, accepted, 0, 0, action as _].into(), + sequence: 0, + type_: conn.atoms.XdndStatus as _, + }; + + conn.conn + .send_event(false, target_window, xproto::EventMask::NO_EVENT, event.serialize()) + .map(|_| ()) + } + + pub fn send_finished( + &self, this_window: xproto::Window, target_window: xproto::Window, accepted: bool, + conn: &XcbConnection, + ) -> Result<(), ConnectionError> { + let (accepted, action) = + if accepted { (1, conn.atoms.XdndFinished) } else { (0, conn.atoms.None) }; + + let event = xproto::ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, + window: target_window, + format: 32, + data: [this_window, accepted, action as _, 0, 0].into(), + sequence: 0, + type_: conn.atoms.XdndStatus as _, + }; + + conn.conn + .send_event(false, target_window, xproto::EventMask::NO_EVENT, event.serialize()) + .map(|_| ()) + } + + pub fn get_type_list( + &self, source_window: xproto::Window, conn: &XcbConnection, + ) -> Result, GetPropertyError> { + conn.get_property( + source_window, + conn.atoms.XdndTypeList, + xproto::Atom::from(xproto::AtomEnum::ATOM), + ) + } + + pub fn convert_selection( + &self, window: xproto::Window, time: xproto::Timestamp, conn: &XcbConnection, + ) -> Result<(), ConnectionError> { + conn.conn + .convert_selection( + window, + conn.atoms.XdndSelection, + conn.atoms.TextUriList, + conn.atoms.XdndSelection, + time, + ) + .map(|_| ()) + } + + pub fn read_data( + &self, window: xproto::Window, conn: &XcbConnection, + ) -> Result, GetPropertyError> { + conn.get_property(window, conn.atoms.XdndSelection, conn.atoms.TextUriList) + } + + pub fn parse_data(&self, data: &mut [c_uchar]) -> Result, DndDataParseError> { + if !data.is_empty() { + let mut path_list = Vec::new(); + let decoded = percent_decode(data).decode_utf8()?.into_owned(); + for uri in decoded.split("\r\n").filter(|u| !u.is_empty()) { + // The format is specified as protocol://host/path + // However, it's typically simply protocol:///path + let path_str = if uri.starts_with("file://") { + let path_str = uri.replace("file://", ""); + if !path_str.starts_with('/') { + // A hostname is specified + // Supporting this case is beyond the scope of my mental health + return Err(DndDataParseError::HostnameSpecified(path_str)); + } + path_str + } else { + // Only the file protocol is supported + return Err(DndDataParseError::UnexpectedProtocol(uri.to_owned())); + }; + + let path = Path::new(&path_str).canonicalize()?; + path_list.push(path); + } + Ok(path_list) + } else { + Err(DndDataParseError::EmptyData) + } + } +} + +#[derive(Debug)] +pub enum DndDataParseError { + EmptyData, + InvalidUtf8(#[allow(dead_code)] Utf8Error), + HostnameSpecified(#[allow(dead_code)] String), + UnexpectedProtocol(#[allow(dead_code)] String), + UnresolvablePath(#[allow(dead_code)] io::Error), +} + +impl From for DndDataParseError { + fn from(e: Utf8Error) -> Self { + DndDataParseError::InvalidUtf8(e) + } +} + +impl From for DndDataParseError { + fn from(e: io::Error) -> Self { + DndDataParseError::UnresolvablePath(e) + } +} diff --git a/src/x11/mod.rs b/src/x11/mod.rs index 71faf04..116104f 100644 --- a/src/x11/mod.rs +++ b/src/x11/mod.rs @@ -5,5 +5,6 @@ mod window; pub use window::*; mod cursor; +mod drag_n_drop; mod keyboard; mod visual_info; diff --git a/src/x11/window.rs b/src/x11/window.rs index 86af03c..0e71ab8 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use std::thread; use std::time::*; +use keyboard_types::Modifiers; use raw_window_handle::{ HasRawDisplayHandle, HasRawWindowHandle, RawDisplayHandle, RawWindowHandle, XlibDisplayHandle, XlibWindowHandle, @@ -14,16 +15,17 @@ use raw_window_handle::{ use x11rb::connection::Connection; use x11rb::protocol::xproto::{ - AtomEnum, ChangeWindowAttributesAux, ConfigureWindowAux, ConnectionExt as _, CreateGCAux, - CreateWindowAux, EventMask, PropMode, Visualid, Window as XWindow, WindowClass, + Atom, AtomEnum, ChangeWindowAttributesAux, ConfigureWindowAux, ConnectionExt, CreateGCAux, + CreateWindowAux, EventMask, PropMode, Timestamp, Visualid, Window as XWindow, WindowClass, }; use x11rb::protocol::Event as XEvent; use x11rb::wrapper::ConnectionExt as _; +use super::drag_n_drop::DragNDrop; use super::XcbConnection; use crate::{ - Event, MouseButton, MouseCursor, MouseEvent, PhyPoint, PhySize, ScrollDelta, Size, WindowEvent, - WindowHandler, WindowInfo, WindowOpenOptions, WindowScalePolicy, + DropData, Event, MouseButton, MouseCursor, MouseEvent, PhyPoint, PhySize, Point, ScrollDelta, + Size, WindowEvent, WindowHandler, WindowInfo, WindowOpenOptions, WindowScalePolicy, }; use super::keyboard::{convert_key_press_event, convert_key_release_event, key_mods}; @@ -102,6 +104,7 @@ struct WindowInner { window_info: WindowInfo, visual_id: Visualid, mouse_cursor: MouseCursor, + drag_n_drop: DragNDrop, frame_interval: Duration, event_loop_running: bool, @@ -261,6 +264,15 @@ impl<'a> Window<'a> { &[xcb_connection.atoms.WM_DELETE_WINDOW], )?; + // Enable drag and drop (TODO: Make this toggleable?) + xcb_connection.conn.change_property32( + PropMode::REPLACE, + window_id, + xcb_connection.atoms.XdndAware, + AtomEnum::ATOM, + &[5u32], // Latest version; hasn't changed since 2002 + )?; + xcb_connection.conn.flush()?; // TODO: These APIs could use a couple tweaks now that everything is internal and there is @@ -285,6 +297,7 @@ impl<'a> Window<'a> { window_info, visual_id: visual_info.visual_id, mouse_cursor: MouseCursor::default(), + drag_n_drop: DragNDrop::new(), frame_interval: Duration::from_millis(15), event_loop_running: false, @@ -503,13 +516,202 @@ impl WindowInner { // window //// XEvent::ClientMessage(event) => { - if event.format == 32 - && event.data.as_data32()[0] == self.xcb_connection.atoms.WM_DELETE_WINDOW - { + if event.format != 32 { + return; + } + + if event.data.as_data32()[0] == self.xcb_connection.atoms.WM_DELETE_WINDOW { self.handle_close_requested(handler); + return; + } + + //// + // drag n drop + //// + if event.type_ == self.xcb_connection.atoms.XdndEnter { + let data = event.data.as_data32(); + + let source_window = data[0] as XWindow; + let flags = data[1]; + let version = flags >> 24; + + self.drag_n_drop.version = Some(version); + + let has_more_types = flags - (flags & (u32::max_value() - 1)) == 1; + if !has_more_types { + let type_list = vec![data[2] as Atom, data[3] as Atom, data[4] as Atom]; + self.drag_n_drop.type_list = Some(type_list); + } else if let Ok(more_types) = + self.drag_n_drop.get_type_list(source_window, &self.xcb_connection) + { + self.drag_n_drop.type_list = Some(more_types); + } + + handler.on_event( + &mut crate::Window::new(Window { inner: self }), + Event::Mouse(MouseEvent::DragEntered { + // We don't get the position until we get an `XdndPosition` event. + position: Point::new(0.0, 0.0), + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + // We don't get data until we get an `XdndPosition` event. + data: DropData::None, + }), + ); + } else if event.type_ == self.xcb_connection.atoms.XdndPosition { + let data = event.data.as_data32(); + + let source_window = data[0] as XWindow; + + // By our own state flow, `version` should never be `None` at this point. + let version = self.drag_n_drop.version.unwrap_or(5); + + let accepted = if let Some(ref type_list) = self.drag_n_drop.type_list { + type_list.contains(&self.xcb_connection.atoms.TextUriList) + } else { + false + }; + + if !accepted { + if let Err(_e) = self.drag_n_drop.send_status( + self.window_id, + source_window, + false, + &self.xcb_connection, + ) { + // TODO: log warning + } + + self.drag_n_drop.reset(); + return; + } + + self.drag_n_drop.source_window = Some(source_window); + + let packed_coordinates = data[2]; + let x = packed_coordinates >> 16; + let y = packed_coordinates & !(x << 16); + let mut physical_pos = PhyPoint::new(x as i32, y as i32); + + // The coordinates are relative to the root window, not our window >:( + if let Ok(r) = + self.xcb_connection.conn.get_geometry(self.window_id).unwrap().reply() + { + if r.root != self.window_id { + if let Ok(r) = self + .xcb_connection + .conn + .translate_coordinates( + r.root, + self.window_id, + physical_pos.x as i16, + physical_pos.y as i16, + ) + .unwrap() + .reply() + { + physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); + } + } + } + + self.drag_n_drop.logical_pos = physical_pos.to_logical(&self.window_info); + + let ev = Event::Mouse(MouseEvent::DragMoved { + position: self.drag_n_drop.logical_pos, + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + data: self.drag_n_drop.data.clone(), + }); + handler.on_event(&mut crate::Window::new(Window { inner: self }), ev); + + if let DropData::None = &self.drag_n_drop.data { + let time = if version >= 1 { + data[3] as Timestamp + } else { + // In version 0, time isn't specified + x11rb::CURRENT_TIME + }; + + // This results in the `SelectionNotify` event below + if let Err(_e) = self.drag_n_drop.convert_selection( + self.window_id, + time, + &self.xcb_connection, + ) { + // TODO: log warning + } + } + + if let Err(_e) = self.drag_n_drop.send_status( + self.window_id, + source_window, + true, + &self.xcb_connection, + ) { + // TODO: log warning + } + } else if event.type_ == self.xcb_connection.atoms.XdndDrop { + let (source_window, accepted) = + if let Some(source_window) = self.drag_n_drop.source_window { + let ev = Event::Mouse(MouseEvent::DragDropped { + position: self.drag_n_drop.logical_pos, + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + data: self.drag_n_drop.data.clone(), + }); + handler.on_event(&mut crate::Window::new(Window { inner: self }), ev); + + (source_window, true) + } else { + // `source_window` won't be part of our DND state if we already rejected the drop in our + // `XdndPosition` handler. + let source_window = event.data.as_data32()[0] as XWindow; + (source_window, false) + }; + + if let Err(_e) = self.drag_n_drop.send_finished( + self.window_id, + source_window, + accepted, + &self.xcb_connection, + ) { + // TODO: log warning + } + + self.drag_n_drop.reset(); + } else if event.type_ == self.xcb_connection.atoms.XdndLeave { + self.drag_n_drop.reset(); + + handler.on_event( + &mut crate::Window::new(Window { inner: self }), + Event::Mouse(MouseEvent::DragLeft), + ); } } + XEvent::SelectionNotify(event) => { + if event.property == self.xcb_connection.atoms.XdndSelection { + if let Ok(mut data) = + self.drag_n_drop.read_data(self.window_id, &self.xcb_connection) + { + match self.drag_n_drop.parse_data(&mut data) { + Ok(path_list) => { + self.drag_n_drop.data = DropData::Files(path_list); + } + Err(_e) => { + self.drag_n_drop.data = DropData::None; + + // TODO: Log warning + } + } + } + } + } + + //// + // window resize + //// XEvent::ConfigureNotify(event) => { let new_physical_size = PhySize::new(event.width as u32, event.height as u32); @@ -545,6 +747,7 @@ impl WindowInner { // we generate a CursorMoved as well, so the mouse position from here isn't lost let physical_pos = PhyPoint::new(event.event_x as i32, event.event_y as i32); let logical_pos = physical_pos.to_logical(&self.window_info); + handler.on_event( &mut crate::Window::new(Window { inner: self }), Event::Mouse(MouseEvent::CursorMoved { @@ -588,7 +791,6 @@ impl WindowInner { ); } }, - XEvent::ButtonRelease(event) => { if !(4..=7).contains(&event.detail) { let button_id = mouse_id(event.detail); diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index cd47670..fd45ef2 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -5,7 +5,7 @@ use x11::{xlib, xlib::Display, xlib_xcb}; use x11rb::connection::Connection; use x11rb::cursor::Handle as CursorHandle; -use x11rb::protocol::xproto::{Cursor, Screen}; +use x11rb::protocol::xproto::{self, Cursor, Screen}; use x11rb::resource_manager; use x11rb::xcb_ffi::XCBConnection; @@ -13,10 +13,27 @@ use crate::MouseCursor; use super::cursor; +mod get_property; +pub use get_property::GetPropertyError; + x11rb::atom_manager! { pub Atoms: AtomsCookie { WM_PROTOCOLS, WM_DELETE_WINDOW, + + // Drag-N-Drop Atoms + XdndAware, + XdndEnter, + XdndLeave, + XdndDrop, + XdndPosition, + XdndStatus, + XdndActionPrivate, + XdndSelection, + XdndFinished, + XdndTypeList, + TextUriList: b"text/uri-list", + None: b"None", } } @@ -116,6 +133,12 @@ impl XcbConnection { pub fn screen(&self) -> &Screen { &self.conn.setup().roots[self.screen] } + + pub fn get_property( + &self, window: xproto::Window, property: xproto::Atom, property_type: xproto::Atom, + ) -> Result, GetPropertyError> { + self::get_property::get_property(window, property, property_type, &self.conn) + } } impl Drop for XcbConnection { diff --git a/src/x11/xcb_connection/get_property.rs b/src/x11/xcb_connection/get_property.rs new file mode 100644 index 0000000..5ce4253 --- /dev/null +++ b/src/x11/xcb_connection/get_property.rs @@ -0,0 +1,169 @@ +// Code copied and modifed from https://github.com/rust-windowing/winit/blob/master/src/platform_impl/linux/x11/util/window_property.rs + +use std::error::Error; +use std::ffi::c_int; +use std::fmt; +use std::mem; +use std::sync::Arc; + +use bytemuck::Pod; + +use x11rb::errors::ReplyError; +use x11rb::protocol::xproto::{self, ConnectionExt}; +use x11rb::xcb_ffi::XCBConnection; + +#[derive(Debug, Clone)] +pub enum GetPropertyError { + X11rbError(Arc), + TypeMismatch(xproto::Atom), + FormatMismatch(c_int), +} + +/* +impl GetPropertyError { + pub fn is_actual_property_type(&self, t: xproto::Atom) -> bool { + if let GetPropertyError::TypeMismatch(actual_type) = *self { + actual_type == t + } else { + false + } + } +} +*/ + +impl> From for GetPropertyError { + fn from(e: T) -> Self { + Self::X11rbError(Arc::new(e.into())) + } +} + +impl fmt::Display for GetPropertyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + GetPropertyError::X11rbError(err) => err.fmt(f), + GetPropertyError::TypeMismatch(err) => write!(f, "type mismatch: {err}"), + GetPropertyError::FormatMismatch(err) => write!(f, "format mismatch: {err}"), + } + } +} + +impl Error for GetPropertyError {} + +// Number of 32-bit chunks to retrieve per iteration of get_property's inner loop. +// To test if `get_property` works correctly, set this to 1. +const PROPERTY_BUFFER_SIZE: u32 = 1024; // 4k of RAM ought to be enough for anyone! + +pub(super) fn get_property( + window: xproto::Window, property: xproto::Atom, property_type: xproto::Atom, + conn: &XCBConnection, +) -> Result, GetPropertyError> { + let mut iter = PropIterator::new(conn, window, property, property_type); + let mut data = vec![]; + + loop { + if !iter.next_window(&mut data)? { + break; + } + } + + Ok(data) +} + +/// An iterator over the "windows" of the property that we are fetching. +struct PropIterator<'a, T> { + /// Handle to the connection. + conn: &'a XCBConnection, + + /// The window that we're fetching the property from. + window: xproto::Window, + + /// The property that we're fetching. + property: xproto::Atom, + + /// The type of the property that we're fetching. + property_type: xproto::Atom, + + /// The offset of the next window, in 32-bit chunks. + offset: u32, + + /// The format of the type. + format: u8, + + /// Keep a reference to `T`. + _phantom: std::marker::PhantomData, +} + +impl<'a, T: Pod> PropIterator<'a, T> { + /// Create a new property iterator. + fn new( + conn: &'a XCBConnection, window: xproto::Window, property: xproto::Atom, + property_type: xproto::Atom, + ) -> Self { + let format = match mem::size_of::() { + 1 => 8, + 2 => 16, + 4 => 32, + _ => unreachable!(), + }; + + Self { + conn, + window, + property, + property_type, + offset: 0, + format, + _phantom: Default::default(), + } + } + + /// Get the next window and append it to `data`. + /// + /// Returns whether there are more windows to fetch. + fn next_window(&mut self, data: &mut Vec) -> Result { + // Send the request and wait for the reply. + let reply = self + .conn + .get_property( + false, + self.window, + self.property, + self.property_type, + self.offset, + PROPERTY_BUFFER_SIZE, + )? + .reply()?; + + // Make sure that the reply is of the correct type. + if reply.type_ != self.property_type { + return Err(GetPropertyError::TypeMismatch(reply.type_)); + } + + // Make sure that the reply is of the correct format. + if reply.format != self.format { + return Err(GetPropertyError::FormatMismatch(reply.format.into())); + } + + // Append the data to the output. + if mem::size_of::() == 1 && mem::align_of::() == 1 { + // We can just do a bytewise append. + data.extend_from_slice(bytemuck::cast_slice(&reply.value)); + } else { + // Rust's borrowing and types system makes this a bit tricky. + // + // We need to make sure that the data is properly aligned. Unfortunately the best + // safe way to do this is to copy the data to another buffer and then append. + // + // TODO(notgull): It may be worth it to use `unsafe` to copy directly from + // `reply.value` to `data`; check if this is faster. Use benchmarks! + let old_len = data.len(); + let added_len = reply.value.len() / mem::size_of::(); + data.resize(old_len + added_len, T::zeroed()); + bytemuck::cast_slice_mut::(&mut data[old_len..]).copy_from_slice(&reply.value); + } + + // Check `bytes_after` to see if there are more windows to fetch. + self.offset += PROPERTY_BUFFER_SIZE; + Ok(reply.bytes_after != 0) + } +} From dab368b5249f59a0ef7b8d99062754ad943470f9 Mon Sep 17 00:00:00 2001 From: Billy Messenger <60663878+BillyDM@users.noreply.github.com> Date: Mon, 1 Apr 2024 15:55:05 -0500 Subject: [PATCH 2/8] only request root window id once --- src/x11/window.rs | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/x11/window.rs b/src/x11/window.rs index 0e71ab8..65bc566 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -105,6 +105,7 @@ struct WindowInner { visual_id: Visualid, mouse_cursor: MouseCursor, drag_n_drop: DragNDrop, + root_window_id: Option, frame_interval: Duration, event_loop_running: bool, @@ -291,6 +292,17 @@ impl<'a> Window<'a> { GlContext::new(context) }); + let root_window_id = + if let Ok(r) = xcb_connection.conn.get_geometry(window_id).unwrap().reply() { + if r.root != window_id { + Some(r.root) + } else { + None + } + } else { + None + }; + let mut inner = WindowInner { xcb_connection, window_id, @@ -298,6 +310,7 @@ impl<'a> Window<'a> { visual_id: visual_info.visual_id, mouse_cursor: MouseCursor::default(), drag_n_drop: DragNDrop::new(), + root_window_id, frame_interval: Duration::from_millis(15), event_loop_running: false, @@ -594,24 +607,20 @@ impl WindowInner { let mut physical_pos = PhyPoint::new(x as i32, y as i32); // The coordinates are relative to the root window, not our window >:( - if let Ok(r) = - self.xcb_connection.conn.get_geometry(self.window_id).unwrap().reply() - { - if r.root != self.window_id { - if let Ok(r) = self - .xcb_connection - .conn - .translate_coordinates( - r.root, - self.window_id, - physical_pos.x as i16, - physical_pos.y as i16, - ) - .unwrap() - .reply() - { - physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); - } + if let Some(root_id) = self.root_window_id { + if let Ok(r) = self + .xcb_connection + .conn + .translate_coordinates( + root_id, + self.window_id, + physical_pos.x as i16, + physical_pos.y as i16, + ) + .unwrap() + .reply() + { + physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); } } From 702e7ddd6759abecd4ecdc35d2a5088660520fea Mon Sep 17 00:00:00 2001 From: Billy Messenger <60663878+BillyDM@users.noreply.github.com> Date: Tue, 2 Apr 2024 13:31:13 -0500 Subject: [PATCH 3/8] remove commented code, improve attribution --- src/x11/drag_n_drop.rs | 4 +++- src/x11/xcb_connection/get_property.rs | 16 +++------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs index aab94d5..0d856a2 100644 --- a/src/x11/drag_n_drop.rs +++ b/src/x11/drag_n_drop.rs @@ -1,4 +1,6 @@ -// Code copied and modifed from https://github.com/rust-windowing/winit/blob/master/src/platform_impl/linux/x11/dnd.rs +// The following code was copied and modified from: +// https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/dnd.rs +// winit license (Apache License 2.0): https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE use std::{ io, diff --git a/src/x11/xcb_connection/get_property.rs b/src/x11/xcb_connection/get_property.rs index 5ce4253..67f263e 100644 --- a/src/x11/xcb_connection/get_property.rs +++ b/src/x11/xcb_connection/get_property.rs @@ -1,4 +1,6 @@ -// Code copied and modifed from https://github.com/rust-windowing/winit/blob/master/src/platform_impl/linux/x11/util/window_property.rs +// The following code was copied and modified from: +// https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/util/window_property.rs +// winit license (Apache License 2.0): https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE use std::error::Error; use std::ffi::c_int; @@ -19,18 +21,6 @@ pub enum GetPropertyError { FormatMismatch(c_int), } -/* -impl GetPropertyError { - pub fn is_actual_property_type(&self, t: xproto::Atom) -> bool { - if let GetPropertyError::TypeMismatch(actual_type) = *self { - actual_type == t - } else { - false - } - } -} -*/ - impl> From for GetPropertyError { fn from(e: T) -> Self { Self::X11rbError(Arc::new(e.into())) From 0065a4db6d5d825295552c142192815d45df5496 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz Date: Fri, 19 Apr 2024 14:42:57 +0200 Subject: [PATCH 4/8] Fix connection flushing, and add a few consistency checks --- examples/render_femtovg/src/main.rs | 7 ++++++- src/x11/drag_n_drop.rs | 16 +++++++++++++--- src/x11/event_loop.rs | 16 +++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/examples/render_femtovg/src/main.rs b/examples/render_femtovg/src/main.rs index 0f5504b..397145c 100644 --- a/examples/render_femtovg/src/main.rs +++ b/examples/render_femtovg/src/main.rs @@ -83,7 +83,12 @@ impl WindowHandler for FemtovgExample { self.canvas.set_size(phy_size.width, phy_size.height, size.scale() as f32); self.damaged = true; } - Event::Mouse(MouseEvent::CursorMoved { position, .. }) => { + Event::Mouse( + MouseEvent::CursorMoved { position, .. } + | MouseEvent::DragEntered { position, .. } + | MouseEvent::DragMoved { position, .. } + | MouseEvent::DragDropped { position, .. }, + ) => { self.current_mouse_position = position.to_physical(&self.current_size); self.damaged = true; } diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs index 0d856a2..3fddd30 100644 --- a/src/x11/drag_n_drop.rs +++ b/src/x11/drag_n_drop.rs @@ -10,6 +10,8 @@ use std::{ }; use percent_encoding::percent_decode; +use x11rb::connection::Connection; +use x11rb::protocol::xproto::Timestamp; use x11rb::{ errors::ConnectionError, protocol::xproto::{self, ConnectionExt}, @@ -31,6 +33,7 @@ pub(crate) struct DragNDrop { // Populated by SelectionNotify event handler (triggered by XdndPosition event handler) pub data: DropData, + pub data_requested_at: Option, pub logical_pos: Point, } @@ -42,6 +45,7 @@ impl DragNDrop { type_list: None, source_window: None, data: DropData::None, + data_requested_at: None, logical_pos: Point::new(0.0, 0.0), } } @@ -51,6 +55,7 @@ impl DragNDrop { self.type_list = None; self.source_window = None; self.data = DropData::None; + self.data_requested_at = None; self.logical_pos = Point::new(0.0, 0.0); } @@ -70,9 +75,14 @@ impl DragNDrop { type_: conn.atoms.XdndStatus as _, }; - conn.conn - .send_event(false, target_window, xproto::EventMask::NO_EVENT, event.serialize()) - .map(|_| ()) + conn.conn.send_event( + false, + target_window, + xproto::EventMask::NO_EVENT, + event.serialize(), + )?; + + conn.conn.flush() } pub fn send_finished( diff --git a/src/x11/event_loop.rs b/src/x11/event_loop.rs index 6c915d2..194d1a1 100644 --- a/src/x11/event_loop.rs +++ b/src/x11/event_loop.rs @@ -174,6 +174,7 @@ impl EventLoop { // drag n drop //// if event.type_ == self.window.xcb_connection.atoms.XdndEnter { + self.drag_n_drop.reset(); let data = event.data.as_data32(); let source_window = data[0] as XWindow; @@ -278,7 +279,7 @@ impl EventLoop { self.handler .on_event(&mut crate::Window::new(Window { inner: &self.window }), ev); - if let DropData::None = &self.drag_n_drop.data { + if self.drag_n_drop.data_requested_at.is_none() { let time = if version >= 1 { data[3] as Timestamp } else { @@ -293,6 +294,9 @@ impl EventLoop { &self.window.xcb_connection, ) { // TODO: log warning + } else { + self.drag_n_drop.data_requested_at = Some(time); + self.window.xcb_connection.conn.flush().unwrap(); } } @@ -347,6 +351,16 @@ impl EventLoop { XEvent::SelectionNotify(event) => { if event.property == self.window.xcb_connection.atoms.XdndSelection { + let Some(requested_time) = self.drag_n_drop.data_requested_at else { + // eprintln!("Received DnD selection data, but we didn't request any. Weird. Skipping."); + return; + }; + + if event.time != requested_time { + // eprintln!("Received stale DnD selection data ({}), expected data is {requested_time}. Skipping.", event.time); + return; + } + if let Ok(mut data) = self .drag_n_drop .read_data(self.window.window_id, &self.window.xcb_connection) From 5757c06431a06e33ceebf658026f864730b7f08c Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz Date: Fri, 19 Apr 2024 15:01:25 +0200 Subject: [PATCH 5/8] Remove unneeded root window ID fetching, use the id provided by the connection instead. --- src/x11/event_loop.rs | 40 ++++++++++++++++------------------------ src/x11/window.rs | 13 ------------- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/src/x11/event_loop.rs b/src/x11/event_loop.rs index 194d1a1..3a3f0bb 100644 --- a/src/x11/event_loop.rs +++ b/src/x11/event_loop.rs @@ -240,30 +240,22 @@ impl EventLoop { let mut physical_pos = PhyPoint::new(x as i32, y as i32); // The coordinates are relative to the root window, not our window >:( - if let Ok(r) = self - .window - .xcb_connection - .conn - .get_geometry(self.window.window_id) - .unwrap() - .reply() - { - if r.root != self.window.window_id { - if let Ok(r) = self - .window - .xcb_connection - .conn - .translate_coordinates( - r.root, - self.window.window_id, - physical_pos.x as i16, - physical_pos.y as i16, - ) - .unwrap() - .reply() - { - physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); - } + let root_id = self.window.xcb_connection.screen().root; + if root_id != self.window.window_id { + if let Ok(r) = self + .window + .xcb_connection + .conn + .translate_coordinates( + root_id, + self.window.window_id, + physical_pos.x as i16, + physical_pos.y as i16, + ) + .unwrap() + .reply() + { + physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); } } diff --git a/src/x11/window.rs b/src/x11/window.rs index dfd890d..56eb4a5 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -99,7 +99,6 @@ pub(crate) struct WindowInner { pub(crate) window_info: WindowInfo, visual_id: Visualid, mouse_cursor: Cell, - root_window_id: Option, pub(crate) close_requested: Cell, @@ -281,24 +280,12 @@ impl<'a> Window<'a> { GlContext::new(context) }); - let root_window_id = - if let Ok(r) = xcb_connection.conn.get_geometry(window_id).unwrap().reply() { - if r.root != window_id { - Some(r.root) - } else { - None - } - } else { - None - }; - let mut inner = WindowInner { xcb_connection, window_id, window_info, visual_id: visual_info.visual_id, mouse_cursor: Cell::new(MouseCursor::default()), - root_window_id, close_requested: Cell::new(false), From 49e16bb7f05bcb6e616294cdb115b1aaf7fcbfbe Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz Date: Fri, 19 Apr 2024 23:40:35 +0200 Subject: [PATCH 6/8] Make the license disclaimers for the winit-originating code more robust. --- src/x11/drag_n_drop.rs | 41 +++++++++++++++++++-- src/x11/xcb_connection/get_property.rs | 49 ++++++++++++++++++++------ 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs index 3fddd30..91c71c8 100644 --- a/src/x11/drag_n_drop.rs +++ b/src/x11/drag_n_drop.rs @@ -1,6 +1,41 @@ -// The following code was copied and modified from: -// https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/dnd.rs -// winit license (Apache License 2.0): https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE +/* +The code in this file was derived from the Winit project (https://github.com/rust-windowing/winit). +The original, unmodified code file this work is derived from can be found here: + +https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/dnd.rs + +The original code this is based on is licensed under the following terms: +*/ + +/* +Copyright 2024 "The Winit contributors". + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +The full licensing terms of the original source code, at the time of writing, can also be found at: +https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE . + +The Derived Work present in this file contains modifications made to the original source code, is +Copyright (c) 2024 "The Baseview contributors", +and is licensed under either the Apache License, Version 2.0; or The MIT license, at your option. + +Copies of those licenses can be respectively found at: +* https://github.com/RustAudio/baseview/blob/master/LICENSE-APACHE or http://www.apache.org/licenses/LICENSE-2.0 ; +* https://github.com/RustAudio/baseview/blob/master/LICENSE-MIT. + +*/ use std::{ io, diff --git a/src/x11/xcb_connection/get_property.rs b/src/x11/xcb_connection/get_property.rs index 67f263e..317b33c 100644 --- a/src/x11/xcb_connection/get_property.rs +++ b/src/x11/xcb_connection/get_property.rs @@ -1,6 +1,41 @@ -// The following code was copied and modified from: -// https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/util/window_property.rs -// winit license (Apache License 2.0): https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE +/* +The code in this file was derived from the Winit project (https://github.com/rust-windowing/winit). +The original, unmodified code file this work is derived from can be found here: + +https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/util/window_property.rs + +The original code this is based on is licensed under the following terms: +*/ + +/* +Copyright 2024 "The Winit contributors". + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +The full licensing terms of the original source code, at the time of writing, can also be found at: +https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE . + +The Derived Work present in this file contains modifications made to the original source code, is +Copyright (c) 2024 "The Baseview contributors", +and is licensed under either the Apache License, Version 2.0; or The MIT license, at your option. + +Copies of those licenses can be respectively found at: +* https://github.com/RustAudio/baseview/blob/master/LICENSE-APACHE or http://www.apache.org/licenses/LICENSE-2.0 ; +* https://github.com/RustAudio/baseview/blob/master/LICENSE-MIT. + +*/ use std::error::Error; use std::ffi::c_int; @@ -139,15 +174,9 @@ impl<'a, T: Pod> PropIterator<'a, T> { // We can just do a bytewise append. data.extend_from_slice(bytemuck::cast_slice(&reply.value)); } else { - // Rust's borrowing and types system makes this a bit tricky. - // - // We need to make sure that the data is properly aligned. Unfortunately the best - // safe way to do this is to copy the data to another buffer and then append. - // - // TODO(notgull): It may be worth it to use `unsafe` to copy directly from - // `reply.value` to `data`; check if this is faster. Use benchmarks! let old_len = data.len(); let added_len = reply.value.len() / mem::size_of::(); + data.resize(old_len + added_len, T::zeroed()); bytemuck::cast_slice_mut::(&mut data[old_len..]).copy_from_slice(&reply.value); } From 2add749cc086360ba3b21e11e462c55b685a10eb Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz Date: Sat, 20 Apr 2024 06:00:15 +0200 Subject: [PATCH 7/8] Refactor the drag_n_drop implementation to use a more robust state enum --- src/x11/drag_n_drop.rs | 713 +++++++++++++++++++++++++++++++---------- src/x11/event_loop.rs | 209 ++---------- 2 files changed, 567 insertions(+), 355 deletions(-) diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs index 91c71c8..8e8a8fa 100644 --- a/src/x11/drag_n_drop.rs +++ b/src/x11/drag_n_drop.rs @@ -1,223 +1,596 @@ -/* -The code in this file was derived from the Winit project (https://github.com/rust-windowing/winit). -The original, unmodified code file this work is derived from can be found here: - -https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/src/platform_impl/linux/x11/dnd.rs - -The original code this is based on is licensed under the following terms: -*/ - -/* -Copyright 2024 "The Winit contributors". - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -/* -The full licensing terms of the original source code, at the time of writing, can also be found at: -https://github.com/rust-windowing/winit/blob/44aabdddcc9f720aec860c1f83c1041082c28560/LICENSE . - -The Derived Work present in this file contains modifications made to the original source code, is -Copyright (c) 2024 "The Baseview contributors", -and is licensed under either the Apache License, Version 2.0; or The MIT license, at your option. - -Copies of those licenses can be respectively found at: -* https://github.com/RustAudio/baseview/blob/master/LICENSE-APACHE or http://www.apache.org/licenses/LICENSE-2.0 ; -* https://github.com/RustAudio/baseview/blob/master/LICENSE-MIT. - -*/ - +use keyboard_types::Modifiers; +use std::error::Error; +use std::fmt::{Display, Formatter}; use std::{ - io, - os::raw::*, + io, mem, path::{Path, PathBuf}, str::Utf8Error, }; use percent_encoding::percent_decode; use x11rb::connection::Connection; -use x11rb::protocol::xproto::Timestamp; +use x11rb::errors::ReplyError; +use x11rb::protocol::xproto::{ClientMessageEvent, SelectionNotifyEvent, Timestamp}; use x11rb::{ errors::ConnectionError, protocol::xproto::{self, ConnectionExt}, x11_utils::Serialize, }; -use crate::{DropData, Point}; +use super::xcb_connection::GetPropertyError; +use crate::x11::{Window, WindowInner}; +use crate::{DropData, Event, MouseEvent, PhyPoint, WindowHandler}; +use DragNDropState::*; + +/// The Drag-N-Drop session state of a `baseview` X11 window, for which it is the target. +/// +/// For more information about what the heck is going on here, see the +/// [XDND (X Drag-n-Drop) specification](https://www.freedesktop.org/wiki/Specifications/XDND/). +pub(crate) enum DragNDropState { + /// There is no active XDND session for this window. + NoCurrentSession, + /// At some point in this session's lifetime, we have decided we couldn't possibly handle the + /// source's Drop data. Every request from this source window from now on will be rejected, + /// until either a Leave or Drop event is received. + PermanentlyRejected { + /// The source window the rejected drag session originated from. + source_window: xproto::Window, + }, + /// We have registered a new session (after receiving an Enter event), and are now waiting + /// for a position event. + WaitingForPosition { + /// The protocol version used in this session. + protocol_version: u8, + /// The source window the current drag session originates from. + source_window: xproto::Window, + }, + /// We have performed a request for data (via `XConvertSelection`), and are now waiting for a + /// reply. + /// + /// More position events can still be received to further update the position data. + WaitingForData { + /// The source window the current drag session originates from. + source_window: xproto::Window, + /// The current position of the pointer, from the last received position event. + position: PhyPoint, + /// The timestamp of the event we made the selection request from. + /// + /// This is either from the first position event, or from the drop event if it arrived first. + /// + /// In very old versions of the protocol (v0), this timestamp isn't provided. In that case, + /// this will be `None`. + requested_at: Option, + /// This will be true if we received a drop event *before* we managed to fetch the data. + /// + /// If this is true, this means we must complete the drop upon receiving the data, instead + /// of just going to [`Ready`]. + dropped: bool, + }, + /// We have completed our quest for the drop data. All fields are populated, and the + /// [`WindowHandler`] has been notified about the drop session. + /// + /// We are now waiting for the user to either drop the file, or leave the window. + /// + /// More position events can still be received to further update the position data. + Ready { + /// The source window the current drag session originates from. + source_window: xproto::Window, + position: PhyPoint, + data: DropData, + }, +} + +impl DragNDropState { + pub fn handle_enter_event( + &mut self, window: &WindowInner, handler: &mut dyn WindowHandler, + event: &ClientMessageEvent, + ) -> Result<(), GetPropertyError> { + let data = event.data.as_data32(); + + let source_window = data[0] as xproto::Window; + let [protocol_version, _, _, flags] = data[1].to_le_bytes(); -use super::xcb_connection::{GetPropertyError, XcbConnection}; + // Fetch the list of supported data types. It can be either stored inline in the event, or + // in a separate property on the source window. + const FLAG_HAS_MORE_TYPES: u8 = 1 << 0; + let has_more_types = (FLAG_HAS_MORE_TYPES & flags) == FLAG_HAS_MORE_TYPES; -pub(crate) struct DragNDrop { - // Populated by XdndEnter event handler - pub version: Option, + let extra_types; + let supported_types = if !has_more_types { + &data[2..5] + } else { + extra_types = window.xcb_connection.get_property( + source_window, + window.xcb_connection.atoms.XdndTypeList, + xproto::Atom::from(xproto::AtomEnum::ATOM), + )?; - pub type_list: Option>, + &extra_types + }; - // Populated by XdndPosition event handler - pub source_window: Option, + // We only support the TextUriList type + let data_type_supported = + supported_types.contains(&window.xcb_connection.atoms.TextUriList); - // Populated by SelectionNotify event handler (triggered by XdndPosition event handler) - pub data: DropData, - pub data_requested_at: Option, + // If there was an active drag session that we informed the handler about, we need to + // generate the matching DragLeft before cancelling the previous session. + let interrupted_active_drag = matches!(self, Ready { .. }); - pub logical_pos: Point, -} + // Clear any previous state, and mark the new session as started if we can handle the drop. + *self = if data_type_supported { + WaitingForPosition { source_window, protocol_version } + } else { + // Permanently reject the drop if the data isn't supported. + PermanentlyRejected { source_window } + }; -impl DragNDrop { - pub fn new() -> Self { - Self { - version: None, - type_list: None, - source_window: None, - data: DropData::None, - data_requested_at: None, - logical_pos: Point::new(0.0, 0.0), + // Do this at the end, in case the handler panics, so that it doesn't poison our internal state. + if interrupted_active_drag { + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragLeft), + ); } + + Ok(()) } - pub fn reset(&mut self) { - self.version = None; - self.type_list = None; - self.source_window = None; - self.data = DropData::None; - self.data_requested_at = None; - self.logical_pos = Point::new(0.0, 0.0); + pub fn handle_position_event( + &mut self, window: &WindowInner, handler: &mut dyn WindowHandler, + event: &ClientMessageEvent, + ) -> Result<(), ReplyError> { + let data = event.data.as_data32(); + + let event_source_window = data[0] as xproto::Window; + let (event_x, event_y) = decode_xy(data[2]); + + match self { + // Someone sent us a position event without first sending an enter event. + // Weird, but we'll still politely tell them we reject the drop. + NoCurrentSession => Ok(send_status_event(event_source_window, window, false)?), + + // The current session's source window does not match the given event. + // This means it can either be from a stale session, or a misbehaving app. + // In any case, we ignore the event but still tell the source we reject the drop. + WaitingForPosition { source_window, .. } + | PermanentlyRejected { source_window, .. } + | WaitingForData { source_window, .. } + | Ready { source_window, .. } + if *source_window != event_source_window => + { + Ok(send_status_event(event_source_window, window, false)?) + } + + // We decided to permanently reject this drop. + // This means the WindowHandler can't do anything with the data, so we reject the drop. + PermanentlyRejected { .. } => { + Ok(send_status_event(event_source_window, window, false)?) + } + + // This is the position event we were waiting for. Now we can request the selection data. + // The code above already checks that source_window == event_source_window. + WaitingForPosition { protocol_version, source_window: _ } => { + // In version 0, time isn't specified + let timestamp = (*protocol_version >= 1).then_some(data[3] as Timestamp); + + request_convert_selection(window, timestamp)?; + + let position = translate_root_coordinates(window, event_x, event_y)?; + + *self = WaitingForData { + requested_at: timestamp, + source_window: event_source_window, + position, + dropped: false, + }; + + Ok(()) + } + + // We are still waiting for the data. So we'll just update the position in the meantime. + WaitingForData { position, .. } => { + *position = translate_root_coordinates(window, event_x, event_y)?; + + Ok(()) + } + + // We have already received the data. We can + Ready { position, data, .. } => { + *position = translate_root_coordinates(window, event_x, event_y)?; + + // Inform the source that we are still accepting the drop. + send_status_event(event_source_window, window, true)?; + + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragMoved { + position: position.to_logical(&window.window_info), + data: data.clone(), + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + }), + ); + + Ok(()) + } + } } - pub fn send_status( - &self, this_window: xproto::Window, target_window: xproto::Window, accepted: bool, - conn: &XcbConnection, - ) -> Result<(), ConnectionError> { - let (accepted, action) = - if accepted { (1, conn.atoms.XdndActionPrivate) } else { (0, conn.atoms.None) }; - - let event = xproto::ClientMessageEvent { - response_type: xproto::CLIENT_MESSAGE_EVENT, - window: target_window, - format: 32, - data: [this_window, accepted, 0, 0, action as _].into(), - sequence: 0, - type_: conn.atoms.XdndStatus as _, + pub fn handle_leave_event( + &mut self, window: &WindowInner, handler: &mut dyn WindowHandler, + event: &ClientMessageEvent, + ) { + let data = event.data.as_data32(); + let event_source_window = data[0] as xproto::Window; + + let current_source_window = match self { + NoCurrentSession => return, + WaitingForPosition { source_window, .. } + | PermanentlyRejected { source_window, .. } + | WaitingForData { source_window, .. } + | Ready { source_window, .. } => *source_window, }; - conn.conn.send_event( - false, - target_window, - xproto::EventMask::NO_EVENT, - event.serialize(), - )?; + // Only accept the leave event if it comes from the source window of the current drag session. + if event_source_window != current_source_window { + return; + } + + // If there was an active drag session that we informed the handler about, we need to + // generate the matching DragLeft before cancelling the previous session. + let left_active_drag = matches!(self, Ready { .. }); - conn.conn.flush() + // Clear everything. + *self = NoCurrentSession; + + // Do this at the end, in case the handler panics, so that it doesn't poison our internal state. + if left_active_drag { + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragLeft), + ); + } } - pub fn send_finished( - &self, this_window: xproto::Window, target_window: xproto::Window, accepted: bool, - conn: &XcbConnection, + pub fn handle_drop_event( + &mut self, window: &WindowInner, handler: &mut dyn WindowHandler, + event: &ClientMessageEvent, ) -> Result<(), ConnectionError> { - let (accepted, action) = - if accepted { (1, conn.atoms.XdndFinished) } else { (0, conn.atoms.None) }; - - let event = xproto::ClientMessageEvent { - response_type: xproto::CLIENT_MESSAGE_EVENT, - window: target_window, - format: 32, - data: [this_window, accepted, action as _, 0, 0].into(), - sequence: 0, - type_: conn.atoms.XdndStatus as _, - }; + let data = event.data.as_data32(); + + let event_source_window = data[0] as xproto::Window; + + match self { + // Someone sent us a position event without first sending an enter event. + // Weird, but we'll still politely tell them we reject the drop. + NoCurrentSession => Ok(send_finished_event(event_source_window, window, false)?), + + // The current session's source window does not match the given event. + // This means it can either be from a stale session, or a misbehaving app. + // In any case, we ignore the event but still tell the source we reject the drop. + WaitingForPosition { source_window, .. } + | PermanentlyRejected { source_window, .. } + | WaitingForData { source_window, .. } + | Ready { source_window, .. } + if *source_window != event_source_window => + { + Ok(send_finished_event(event_source_window, window, false)?) + } - conn.conn - .send_event(false, target_window, xproto::EventMask::NO_EVENT, event.serialize()) - .map(|_| ()) - } + // We decided to permanently reject this drop. + // This means the WindowHandler can't do anything with the data, so we reject the drop. + PermanentlyRejected { .. } => { + send_finished_event(event_source_window, window, false)?; + + *self = NoCurrentSession; + + Ok(()) + } + + // We received a drop event without any position event. That's very weird, but not + // irrecoverable: the drop event provides enough data as it is. + // The code above already checks that source_window == event_source_window. + WaitingForPosition { protocol_version, source_window: _ } => { + // In version 0, time isn't specified + let timestamp = (*protocol_version >= 1).then_some(data[2] as Timestamp); + + // We have the timestamp, we can use it to request to convert the selection, + // even in this state. + + request_convert_selection(window, timestamp)?; + + *self = WaitingForData { + requested_at: timestamp, + source_window: event_source_window, + // We don't have usable position data. Maybe we'll receive a position later, + // but otherwise this will have to do. + position: PhyPoint::new(0, 0), + dropped: true, + }; - pub fn get_type_list( - &self, source_window: xproto::Window, conn: &XcbConnection, - ) -> Result, GetPropertyError> { - conn.get_property( - source_window, - conn.atoms.XdndTypeList, - xproto::Atom::from(xproto::AtomEnum::ATOM), - ) + Ok(()) + } + + // We are still waiting to receive the data. + // In that case, we'll wait to receive all of it before finalizing the drop. + WaitingForData { dropped, requested_at, .. } => { + // If we have a timestamp, that means this is version >= 1. + if let Some(requested_at) = *requested_at { + let event_timestamp = data[2] as Timestamp; + + // Just in case, check if this drop event isn't stale + if requested_at > event_timestamp { + return Ok(()); + } + } + + // Indicate to the selection_notified handler that the user has performed the drop. + // Now it should complete the drop instead of just waiting for more events. + *dropped = true; + + Ok(()) + } + + // The normal case. + Ready { .. } => { + send_finished_event(event_source_window, window, true)?; + + let Ready { data, position, .. } = mem::replace(self, NoCurrentSession) else { + unreachable!() + }; + + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragDropped { + position: position.to_logical(&window.window_info), + data, + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + }), + ); + + Ok(()) + } + } } - pub fn convert_selection( - &self, window: xproto::Window, time: xproto::Timestamp, conn: &XcbConnection, + pub fn handle_selection_notify_event( + &mut self, window: &WindowInner, handler: &mut dyn WindowHandler, + event: &SelectionNotifyEvent, ) -> Result<(), ConnectionError> { - conn.conn - .convert_selection( - window, - conn.atoms.XdndSelection, - conn.atoms.TextUriList, - conn.atoms.XdndSelection, - time, - ) - .map(|_| ()) + // Ignore the event if we weren't actually waiting for a selection notify event + let WaitingForData { source_window, requested_at, position, dropped } = *self else { + return Ok(()); + }; + + // Ignore if this was meant for another window (?) + if event.requestor != window.window_id { + return Ok(()); + } + + // Ignore if this is stale selection data. + if let Some(requested_at) = requested_at { + if requested_at != event.time { + return Ok(()); + } + } + + // The sender should have set the data on our window, let's fetch it. + match fetch_dnd_data(window) { + Err(_e) => { + *self = PermanentlyRejected { source_window }; + + if dropped { + send_finished_event(source_window, window, false) + } else { + send_status_event(source_window, window, false) + } + + // TODO: Log warning + } + Ok(data) => { + let logical_position = position.to_logical(&window.window_info); + + // Inform the source that we are (still) accepting the drop. + + // Handle the case where the user already dropped, but we received the data only later. + if dropped { + *self = NoCurrentSession; + + send_finished_event(source_window, window, true)?; + + // Now that we have actual drop data, we can inform the handler about the drag AND drop events. + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragEntered { + position: logical_position, + data: data.clone(), + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + }), + ); + + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragDropped { + position: logical_position, + data: data.clone(), + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + }), + ); + } else { + // Save the data, now that we finally have it! + *self = Ready { data: data.clone(), source_window, position }; + + send_status_event(source_window, window, true)?; + + // Now that we have actual drop data, we can inform the handler about the drag event. + handler.on_event( + &mut crate::Window::new(Window { inner: window }), + Event::Mouse(MouseEvent::DragEntered { + position: logical_position, + data, + // We don't get modifiers for drag n drop events. + modifiers: Modifiers::empty(), + }), + ); + } + + Ok(()) + } + } } +} + +fn send_status_event( + source_window: xproto::Window, window: &WindowInner, accepted: bool, +) -> Result<(), ConnectionError> { + let conn = &window.xcb_connection; + let (accepted, action) = + if accepted { (1, conn.atoms.XdndActionPrivate) } else { (0, conn.atoms.None) }; + + let event = ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, + window: source_window, + format: 32, + data: [window.window_id, accepted, 0, 0, action as _].into(), + sequence: 0, + type_: conn.atoms.XdndStatus, + }; + + conn.conn.send_event(false, source_window, xproto::EventMask::NO_EVENT, event.serialize())?; + + conn.conn.flush() +} + +pub fn send_finished_event( + source_window: xproto::Window, window: &WindowInner, accepted: bool, +) -> Result<(), ConnectionError> { + let conn = &window.xcb_connection; + let (accepted, action) = + if accepted { (1, conn.atoms.XdndFinished) } else { (0, conn.atoms.None) }; + + let event = ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, + window: source_window, + format: 32, + data: [window.window_id, accepted, action as _, 0, 0].into(), + sequence: 0, + type_: conn.atoms.XdndStatus as _, + }; + + conn.conn.send_event(false, source_window, xproto::EventMask::NO_EVENT, event.serialize())?; + + conn.conn.flush() +} - pub fn read_data( - &self, window: xproto::Window, conn: &XcbConnection, - ) -> Result, GetPropertyError> { - conn.get_property(window, conn.atoms.XdndSelection, conn.atoms.TextUriList) +fn request_convert_selection( + window: &WindowInner, timestamp: Option, +) -> Result<(), ConnectionError> { + let conn = &window.xcb_connection; + + conn.conn.convert_selection( + window.window_id, + conn.atoms.XdndSelection, + conn.atoms.TextUriList, + conn.atoms.XdndSelection, + timestamp.unwrap_or(x11rb::CURRENT_TIME), + )?; + + conn.conn.flush() +} + +fn decode_xy(data: u32) -> (u16, u16) { + ((data >> 16) as u16, data as u16) +} + +fn translate_root_coordinates( + window: &WindowInner, x: u16, y: u16, +) -> Result { + let root_id = window.xcb_connection.screen().root; + if root_id == window.window_id { + return Ok(PhyPoint::new(x as i32, y as i32)); } - pub fn parse_data(&self, data: &mut [c_uchar]) -> Result, DndDataParseError> { - if !data.is_empty() { - let mut path_list = Vec::new(); - let decoded = percent_decode(data).decode_utf8()?.into_owned(); - for uri in decoded.split("\r\n").filter(|u| !u.is_empty()) { - // The format is specified as protocol://host/path - // However, it's typically simply protocol:///path - let path_str = if uri.starts_with("file://") { - let path_str = uri.replace("file://", ""); - if !path_str.starts_with('/') { - // A hostname is specified - // Supporting this case is beyond the scope of my mental health - return Err(DndDataParseError::HostnameSpecified(path_str)); - } - path_str - } else { - // Only the file protocol is supported - return Err(DndDataParseError::UnexpectedProtocol(uri.to_owned())); - }; + let reply = window + .xcb_connection + .conn + .translate_coordinates(root_id, window.window_id, x as i16, y as i16)? + .reply()?; + + Ok(PhyPoint::new(reply.dst_x as i32, reply.dst_y as i32)) +} + +fn fetch_dnd_data(window: &WindowInner) -> Result> { + let conn = &window.xcb_connection; + + let data: Vec = + conn.get_property(window.window_id, conn.atoms.XdndSelection, conn.atoms.TextUriList)?; + + let path_list = parse_data(&data)?; - let path = Path::new(&path_str).canonicalize()?; - path_list.push(path); + Ok(DropData::Files(path_list)) +} + +// See: https://edeproject.org/spec/file-uri-spec.txt +// TL;DR: format is "file:///", hostname is optional and can be "localhost" +fn parse_data(data: &[u8]) -> Result, ParseError> { + if data.is_empty() { + return Err(ParseError::EmptyData); + } + + let decoded = percent_decode(data).decode_utf8().map_err(ParseError::InvalidUtf8)?; + + let mut path_list = Vec::new(); + for uri in decoded.split("\r\n").filter(|u| !u.is_empty()) { + // We only support the file:// protocol + let Some(mut uri) = uri.strip_prefix("file://") else { + return Err(ParseError::UnsupportedProtocol(uri.into())); + }; + + if !uri.starts_with('/') { + // Try (and hope) to see if it's just localhost + if let Some(stripped) = uri.strip_prefix("localhost") { + if !stripped.starts_with('/') { + // There is something else after "localhost" but before '/' + return Err(ParseError::UnsupportedHostname(uri.into())); + } + + uri = stripped; + } else { + // We don't support hostnames. + return Err(ParseError::UnsupportedHostname(uri.into())); } - Ok(path_list) - } else { - Err(DndDataParseError::EmptyData) } + + let path = Path::new(uri).canonicalize().map_err(ParseError::CanonicalizeError)?; + path_list.push(path); } + Ok(path_list) } #[derive(Debug)] -pub enum DndDataParseError { +enum ParseError { EmptyData, - InvalidUtf8(#[allow(dead_code)] Utf8Error), - HostnameSpecified(#[allow(dead_code)] String), - UnexpectedProtocol(#[allow(dead_code)] String), - UnresolvablePath(#[allow(dead_code)] io::Error), + InvalidUtf8(Utf8Error), + UnsupportedHostname(String), + UnsupportedProtocol(String), + CanonicalizeError(io::Error), } -impl From for DndDataParseError { - fn from(e: Utf8Error) -> Self { - DndDataParseError::InvalidUtf8(e) - } -} +impl Display for ParseError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to parse Drag-n-Drop data: ")?; -impl From for DndDataParseError { - fn from(e: io::Error) -> Self { - DndDataParseError::UnresolvablePath(e) + match self { + ParseError::EmptyData => f.write_str("data is empty"), + ParseError::InvalidUtf8(e) => e.fmt(f), + ParseError::UnsupportedHostname(uri) => write!(f, "unsupported hostname in URI: {uri}"), + ParseError::UnsupportedProtocol(uri) => write!(f, "unsupported protocol in URI: {uri}"), + ParseError::CanonicalizeError(e) => write!(f, "unable to resolve path: {e}"), + } } } + +impl Error for ParseError {} diff --git a/src/x11/event_loop.rs b/src/x11/event_loop.rs index 3a3f0bb..efdd84c 100644 --- a/src/x11/event_loop.rs +++ b/src/x11/event_loop.rs @@ -1,16 +1,14 @@ -use crate::x11::drag_n_drop::DragNDrop; +use crate::x11::drag_n_drop::DragNDropState; use crate::x11::keyboard::{convert_key_press_event, convert_key_release_event, key_mods}; use crate::x11::{ParentHandle, Window, WindowInner}; use crate::{ - DropData, Event, MouseButton, MouseEvent, PhyPoint, PhySize, Point, ScrollDelta, WindowEvent, - WindowHandler, WindowInfo, + Event, MouseButton, MouseEvent, PhyPoint, PhySize, ScrollDelta, WindowEvent, WindowHandler, + WindowInfo, }; -use keyboard_types::Modifiers; use std::error::Error; use std::os::fd::AsRawFd; use std::time::{Duration, Instant}; use x11rb::connection::Connection; -use x11rb::protocol::xproto::{Atom, ConnectionExt, Timestamp, Window as XWindow}; use x11rb::protocol::Event as XEvent; pub(super) struct EventLoop { @@ -22,7 +20,7 @@ pub(super) struct EventLoop { frame_interval: Duration, event_loop_running: bool, - drag_n_drop: DragNDrop, + drag_n_drop: DragNDropState, } impl EventLoop { @@ -37,7 +35,7 @@ impl EventLoop { frame_interval: Duration::from_millis(15), event_loop_running: false, new_physical_size: None, - drag_n_drop: DragNDrop::new(), + drag_n_drop: DragNDropState::NoCurrentSession, } } @@ -174,199 +172,40 @@ impl EventLoop { // drag n drop //// if event.type_ == self.window.xcb_connection.atoms.XdndEnter { - self.drag_n_drop.reset(); - let data = event.data.as_data32(); - - let source_window = data[0] as XWindow; - let flags = data[1]; - let version = flags >> 24; - - self.drag_n_drop.version = Some(version); - - let has_more_types = flags - (flags & (u32::max_value() - 1)) == 1; - if !has_more_types { - let type_list = vec![data[2] as Atom, data[3] as Atom, data[4] as Atom]; - self.drag_n_drop.type_list = Some(type_list); - } else if let Ok(more_types) = - self.drag_n_drop.get_type_list(source_window, &self.window.xcb_connection) - { - self.drag_n_drop.type_list = Some(more_types); + if let Err(_e) = self.drag_n_drop.handle_enter_event( + &self.window, + &mut *self.handler, + &event, + ) { + // TODO: log warning } - - self.handler.on_event( - &mut crate::Window::new(Window { inner: &self.window }), - Event::Mouse(MouseEvent::DragEntered { - // We don't get the position until we get an `XdndPosition` event. - position: Point::new(0.0, 0.0), - // We don't get modifiers for drag n drop events. - modifiers: Modifiers::empty(), - // We don't get data until we get an `XdndPosition` event. - data: DropData::None, - }), - ); } else if event.type_ == self.window.xcb_connection.atoms.XdndPosition { - let data = event.data.as_data32(); - - let source_window = data[0] as XWindow; - - // By our own state flow, `version` should never be `None` at this point. - let version = self.drag_n_drop.version.unwrap_or(5); - - let accepted = if let Some(ref type_list) = self.drag_n_drop.type_list { - type_list.contains(&self.window.xcb_connection.atoms.TextUriList) - } else { - false - }; - - if !accepted { - if let Err(_e) = self.drag_n_drop.send_status( - self.window.window_id, - source_window, - false, - &self.window.xcb_connection, - ) { - // TODO: log warning - } - - self.drag_n_drop.reset(); - return; - } - - self.drag_n_drop.source_window = Some(source_window); - - let packed_coordinates = data[2]; - let x = packed_coordinates >> 16; - let y = packed_coordinates & !(x << 16); - let mut physical_pos = PhyPoint::new(x as i32, y as i32); - - // The coordinates are relative to the root window, not our window >:( - let root_id = self.window.xcb_connection.screen().root; - if root_id != self.window.window_id { - if let Ok(r) = self - .window - .xcb_connection - .conn - .translate_coordinates( - root_id, - self.window.window_id, - physical_pos.x as i16, - physical_pos.y as i16, - ) - .unwrap() - .reply() - { - physical_pos = PhyPoint::new(r.dst_x as i32, r.dst_y as i32); - } - } - - self.drag_n_drop.logical_pos = - physical_pos.to_logical(&self.window.window_info); - - let ev = Event::Mouse(MouseEvent::DragMoved { - position: self.drag_n_drop.logical_pos, - // We don't get modifiers for drag n drop events. - modifiers: Modifiers::empty(), - data: self.drag_n_drop.data.clone(), - }); - self.handler - .on_event(&mut crate::Window::new(Window { inner: &self.window }), ev); - - if self.drag_n_drop.data_requested_at.is_none() { - let time = if version >= 1 { - data[3] as Timestamp - } else { - // In version 0, time isn't specified - x11rb::CURRENT_TIME - }; - - // This results in the `SelectionNotify` event below - if let Err(_e) = self.drag_n_drop.convert_selection( - self.window.window_id, - time, - &self.window.xcb_connection, - ) { - // TODO: log warning - } else { - self.drag_n_drop.data_requested_at = Some(time); - self.window.xcb_connection.conn.flush().unwrap(); - } - } - - if let Err(_e) = self.drag_n_drop.send_status( - self.window.window_id, - source_window, - true, - &self.window.xcb_connection, + if let Err(_e) = self.drag_n_drop.handle_position_event( + &self.window, + &mut *self.handler, + &event, ) { // TODO: log warning } } else if event.type_ == self.window.xcb_connection.atoms.XdndDrop { - let (source_window, accepted) = if let Some(source_window) = - self.drag_n_drop.source_window + if let Err(_e) = + self.drag_n_drop.handle_drop_event(&self.window, &mut *self.handler, &event) { - let ev = Event::Mouse(MouseEvent::DragDropped { - position: self.drag_n_drop.logical_pos, - // We don't get modifiers for drag n drop events. - modifiers: Modifiers::empty(), - data: self.drag_n_drop.data.clone(), - }); - self.handler - .on_event(&mut crate::Window::new(Window { inner: &self.window }), ev); - - (source_window, true) - } else { - // `source_window` won't be part of our DND state if we already rejected the drop in our - // `XdndPosition` handler. - let source_window = event.data.as_data32()[0] as XWindow; - (source_window, false) - }; - - if let Err(_e) = self.drag_n_drop.send_finished( - self.window.window_id, - source_window, - accepted, - &self.window.xcb_connection, - ) { // TODO: log warning } - - self.drag_n_drop.reset(); } else if event.type_ == self.window.xcb_connection.atoms.XdndLeave { - self.drag_n_drop.reset(); - - self.handler.on_event( - &mut crate::Window::new(Window { inner: &self.window }), - Event::Mouse(MouseEvent::DragLeft), - ); + self.drag_n_drop.handle_leave_event(&self.window, &mut *self.handler, &event); } } XEvent::SelectionNotify(event) => { if event.property == self.window.xcb_connection.atoms.XdndSelection { - let Some(requested_time) = self.drag_n_drop.data_requested_at else { - // eprintln!("Received DnD selection data, but we didn't request any. Weird. Skipping."); - return; - }; - - if event.time != requested_time { - // eprintln!("Received stale DnD selection data ({}), expected data is {requested_time}. Skipping.", event.time); - return; - } - - if let Ok(mut data) = self - .drag_n_drop - .read_data(self.window.window_id, &self.window.xcb_connection) - { - match self.drag_n_drop.parse_data(&mut data) { - Ok(path_list) => { - self.drag_n_drop.data = DropData::Files(path_list); - } - Err(_e) => { - self.drag_n_drop.data = DropData::None; - - // TODO: Log warning - } - } + if let Err(_e) = self.drag_n_drop.handle_selection_notify_event( + &self.window, + &mut *self.handler, + &event, + ) { + // TODO: Log warning } } } From 9b8cde2469026885e3c3cce8dd94ae7f0728c955 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz Date: Mon, 22 Apr 2024 16:15:55 +0200 Subject: [PATCH 8/8] Hardening around X11 connection error handling in DND implementation --- src/x11/drag_n_drop.rs | 61 +++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/src/x11/drag_n_drop.rs b/src/x11/drag_n_drop.rs index 8e8a8fa..540b9bd 100644 --- a/src/x11/drag_n_drop.rs +++ b/src/x11/drag_n_drop.rs @@ -175,15 +175,17 @@ impl DragNDropState { request_convert_selection(window, timestamp)?; - let position = translate_root_coordinates(window, event_x, event_y)?; - + // We set our state before translating position data, in case that fails. *self = WaitingForData { requested_at: timestamp, source_window: event_source_window, - position, + position: PhyPoint::new(0, 0), dropped: false, }; + let WaitingForData { position, .. } = self else { unreachable!() }; + *position = translate_root_coordinates(window, event_x, event_y)?; + Ok(()) } @@ -194,12 +196,15 @@ impl DragNDropState { Ok(()) } - // We have already received the data. We can + // We have already received the data. We can update the position and notify the handler Ready { position, data, .. } => { - *position = translate_root_coordinates(window, event_x, event_y)?; - // Inform the source that we are still accepting the drop. - send_status_event(event_source_window, window, true)?; + // Do this first, in case translate_root_coordinates fails, or the handler panics. + // Do not return right away on failure though, we can still inform the handler about + // the new position. + let status_result = send_status_event(event_source_window, window, true); + + *position = translate_root_coordinates(window, event_x, event_y)?; handler.on_event( &mut crate::Window::new(Window { inner: window }), @@ -211,6 +216,7 @@ impl DragNDropState { }), ); + status_result?; Ok(()) } } @@ -263,7 +269,7 @@ impl DragNDropState { match self { // Someone sent us a position event without first sending an enter event. // Weird, but we'll still politely tell them we reject the drop. - NoCurrentSession => Ok(send_finished_event(event_source_window, window, false)?), + NoCurrentSession => send_finished_event(event_source_window, window, false), // The current session's source window does not match the given event. // This means it can either be from a stale session, or a misbehaving app. @@ -274,17 +280,15 @@ impl DragNDropState { | Ready { source_window, .. } if *source_window != event_source_window => { - Ok(send_finished_event(event_source_window, window, false)?) + send_finished_event(event_source_window, window, false) } // We decided to permanently reject this drop. // This means the WindowHandler can't do anything with the data, so we reject the drop. PermanentlyRejected { .. } => { - send_finished_event(event_source_window, window, false)?; - *self = NoCurrentSession; - Ok(()) + send_finished_event(event_source_window, window, false) } // We received a drop event without any position event. That's very weird, but not @@ -297,7 +301,18 @@ impl DragNDropState { // We have the timestamp, we can use it to request to convert the selection, // even in this state. - request_convert_selection(window, timestamp)?; + // If we fail to send the request when the drop has completed, we can't do anything. + // Just cancel the drop. + if let Err(e) = request_convert_selection(window, timestamp) { + *self = NoCurrentSession; + + // Try to inform the source that we ended up rejecting the drop. + // If the initial request failed, this is likely to fail too, so we'll ignore + // it if it errors, so we can focus on the original error. + let _ = send_finished_event(event_source_window, window, false); + + return Err(e); + }; *self = WaitingForData { requested_at: timestamp, @@ -333,12 +348,14 @@ impl DragNDropState { // The normal case. Ready { .. } => { - send_finished_event(event_source_window, window, true)?; - let Ready { data, position, .. } = mem::replace(self, NoCurrentSession) else { unreachable!() }; + // Don't return immediately if sending the reply fails, we can still notify the window + // handler about the drop. + let reply_result = send_finished_event(event_source_window, window, true); + handler.on_event( &mut crate::Window::new(Window { inner: window }), Event::Mouse(MouseEvent::DragDropped { @@ -349,7 +366,7 @@ impl DragNDropState { }), ); - Ok(()) + reply_result } } } @@ -393,11 +410,11 @@ impl DragNDropState { // Inform the source that we are (still) accepting the drop. - // Handle the case where the user already dropped, but we received the data only later. + // Handle the case where the user already dropped, but we only received the data later. if dropped { *self = NoCurrentSession; - send_finished_event(source_window, window, true)?; + let reply_result = send_finished_event(source_window, window, true); // Now that we have actual drop data, we can inform the handler about the drag AND drop events. handler.on_event( @@ -419,11 +436,13 @@ impl DragNDropState { modifiers: Modifiers::empty(), }), ); + + reply_result } else { // Save the data, now that we finally have it! *self = Ready { data: data.clone(), source_window, position }; - send_status_event(source_window, window, true)?; + let reply_result = send_status_event(source_window, window, true); // Now that we have actual drop data, we can inform the handler about the drag event. handler.on_event( @@ -435,9 +454,9 @@ impl DragNDropState { modifiers: Modifiers::empty(), }), ); - } - Ok(()) + reply_result + } } } }