From 0fca8e8cb57f0b4b03bc12b1b17bec5855e7ec7e Mon Sep 17 00:00:00 2001 From: Francesca Plebani Date: Fri, 2 Nov 2018 17:41:51 -0400 Subject: [PATCH] X11: Fix DND freezing the WM (#688) Fixes #687 `XdndFinished` isn't supposed to be sent when rejecting a `XdndPosition`; it should only be sent in response to `XdndDrop`. https://freedesktop.org/wiki/Specifications/XDND/ --- CHANGELOG.md | 1 + src/platform/linux/x11/mod.rs | 40 +++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2581ca8d..0016098a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - On Linux, if neither X11 nor Wayland manage to initialize, the corresponding panic now consists of a single line only. - Add optional `serde` feature with implementations of `Serialize`/`Deserialize` for DPI types and various event types. - Add `PartialEq`, `Eq`, and `Hash` implementations on public types that could have them but were missing them. +- On X11, drag-and-drop receiving an unsupported drop type can no longer cause the WM to freeze. # Version 0.17.2 (2018-08-19) diff --git a/src/platform/linux/x11/mod.rs b/src/platform/linux/x11/mod.rs index f8cd69fc..461acde1 100644 --- a/src/platform/linux/x11/mod.rs +++ b/src/platform/linux/x11/mod.rs @@ -278,25 +278,25 @@ impl EventsLoop { } } else if client_msg.message_type == self.dnd.atoms.position { // This event occurs every time the mouse moves while a file's being dragged - // over our window. We emit HoveredFile in response; while the Mac OS X backend - // does that upon a drag entering, XDnD doesn't have access to the actual drop + // over our window. We emit HoveredFile in response; while the macOS backend + // does that upon a drag entering, XDND doesn't have access to the actual drop // data until this event. For parity with other platforms, we only emit - // HoveredFile the first time, though if winit's API is later extended to - // supply position updates with HoveredFile or another event, implementing + // `HoveredFile` the first time, though if winit's API is later extended to + // supply position updates with `HoveredFile` or another event, implementing // that here would be trivial. let source_window = client_msg.data.get_long(0) as c_ulong; - // Equivalent to (x << shift) | y - // where shift = mem::size_of::() * 8 + // Equivalent to `(x << shift) | y` + // where `shift = mem::size_of::() * 8` // Note that coordinates are in "desktop space", not "window space" - // (in x11 parlance, they're root window coordinates) + // (in X11 parlance, they're root window coordinates) //let packed_coordinates = client_msg.data.get_long(2); //let shift = mem::size_of::() * 8; //let x = packed_coordinates >> shift; //let y = packed_coordinates & !(x << shift); - // By our own state flow, version should never be None at this point. + // By our own state flow, `version` should never be `None` at this point. let version = self.dnd.version.unwrap_or(5); // Action is specified in versions 2 and up, though we don't need it anyway. @@ -318,23 +318,21 @@ impl EventsLoop { // In version 0, time isn't specified ffi::CurrentTime }; - // This results in the SelectionNotify event below + // This results in the `SelectionNotify` event below self.dnd.convert_selection(window, time); } self.dnd.send_status(window, source_window, DndState::Accepted) - .expect("Failed to send XDnD status message."); + .expect("Failed to send `XdndStatus` message."); } } else { unsafe { self.dnd.send_status(window, source_window, DndState::Rejected) - .expect("Failed to send XDnD status message."); - self.dnd.send_finished(window, source_window, DndState::Rejected) - .expect("Failed to send XDnD finished message."); + .expect("Failed to send `XdndStatus` message."); } self.dnd.reset(); } } else if client_msg.message_type == self.dnd.atoms.drop { - if let Some(source_window) = self.dnd.source_window { + let (source_window, state) = if let Some(source_window) = self.dnd.source_window { if let Some(Ok(ref path_list)) = self.dnd.result { for path in path_list { callback(Event::WindowEvent { @@ -343,10 +341,16 @@ impl EventsLoop { }); } } - unsafe { - self.dnd.send_finished(window, source_window, DndState::Accepted) - .expect("Failed to send XDnD finished message."); - } + (source_window, DndState::Accepted) + } else { + // `source_window` won't be part of our DND state if we already rejected the drop in our + // `XdndPosition` handler. + let source_window = client_msg.data.get_long(0) as c_ulong; + (source_window, DndState::Rejected) + }; + unsafe { + self.dnd.send_finished(window, source_window, state) + .expect("Failed to send `XdndFinished` message."); } self.dnd.reset(); } else if client_msg.message_type == self.dnd.atoms.leave {