fix: X-owned primary blocks clipboard INCR
Due to an oversight, if `XState` had both a valid primary selection and a valid clipboard selection, `handle_selection_property_change` would early return after `check_for_incr` determines the primary selection existed but was not the recipient of the `PropertyNotifyEvent`, preventing INCR events from ever reaching the clipboard. An addtion to the `incr_copy_from_x11` integration test validates this fix. Also fixed was a missing check in `WaylandSelection`'s `check_for_incr` to confirm the property requesting more data is indeed the property the selection is responsible for. I could not figure out how to write an integration for this mistake, but it is obvious enough in hindsight.
This commit is contained in:
parent
cdf405fac5
commit
d621a0b37a
2 changed files with 51 additions and 20 deletions
|
|
@ -146,7 +146,8 @@ impl Selection {
|
|||
}
|
||||
|
||||
fn check_for_incr(&self, event: &x::PropertyNotifyEvent) -> bool {
|
||||
if event.window() != self.window || event.state() != x::Property::NewValue {
|
||||
debug_assert_eq!(event.state(), x::Property::NewValue);
|
||||
if event.window() != self.window {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -178,8 +179,18 @@ pub struct WaylandSelection<T: SelectionType> {
|
|||
}
|
||||
|
||||
impl<T: SelectionType> WaylandSelection<T> {
|
||||
fn check_for_incr(&mut self, connection: &xcb::Connection) -> Option<bool> {
|
||||
let incr_data = self.incr_data.as_mut()?;
|
||||
fn check_for_incr(
|
||||
&mut self,
|
||||
event: &x::PropertyNotifyEvent,
|
||||
connection: &xcb::Connection,
|
||||
) -> bool {
|
||||
let Some(incr_data) = self.incr_data.as_mut() else {
|
||||
return false;
|
||||
};
|
||||
if incr_data.property != event.atom() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let incr_end = std::cmp::min(
|
||||
incr_data.max_req_bytes + incr_data.start,
|
||||
incr_data.data.len(),
|
||||
|
|
@ -194,7 +205,7 @@ impl<T: SelectionType> WaylandSelection<T> {
|
|||
}) {
|
||||
warn!("failed to write selection data: {e:?}");
|
||||
self.incr_data = None;
|
||||
return Some(true);
|
||||
return true;
|
||||
}
|
||||
|
||||
if incr_data.start == incr_end {
|
||||
|
|
@ -210,7 +221,7 @@ impl<T: SelectionType> WaylandSelection<T> {
|
|||
);
|
||||
incr_data.start = incr_end;
|
||||
}
|
||||
Some(true)
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -817,6 +828,11 @@ impl XState {
|
|||
true
|
||||
}
|
||||
|
||||
/// Check if a PropertyNotifyEvent refers to a supported selection property, then make progress
|
||||
/// on an incremental data transfer if that property is in that process.
|
||||
/// Returns `true` if an attempt at progressing the data transfer was made, whether or not it
|
||||
/// succeeded, and `false` otherwise (e.g. the event does not target a selection property or
|
||||
/// that property is not in the process of an incremental data transfer)
|
||||
pub(super) fn handle_selection_property_change(
|
||||
&mut self,
|
||||
event: &x::PropertyNotifyEvent,
|
||||
|
|
@ -825,24 +841,23 @@ impl XState {
|
|||
connection: &xcb::Connection,
|
||||
event: &x::PropertyNotifyEvent,
|
||||
data: &mut SelectionData<T>,
|
||||
) -> Option<bool> {
|
||||
) -> bool {
|
||||
match event.state() {
|
||||
x::Property::NewValue => {
|
||||
if let Some(selection) = &data.x11_selection() {
|
||||
return Some(selection.check_for_incr(event));
|
||||
return selection.check_for_incr(event);
|
||||
}
|
||||
}
|
||||
x::Property::Delete => {
|
||||
if let Some(selection) = data.wayland_selection_mut() {
|
||||
return selection.check_for_incr(connection);
|
||||
return selection.check_for_incr(event, connection);
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
false
|
||||
}
|
||||
inner(&self.connection, event, &mut self.selection_state.primary)
|
||||
.or_else(|| inner(&self.connection, event, &mut self.selection_state.clipboard))
|
||||
.unwrap_or(false)
|
||||
|| inner(&self.connection, event, &mut self.selection_state.clipboard)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -332,6 +332,7 @@ xcb::atoms_struct! {
|
|||
skip_taskbar => b"_NET_WM_STATE_SKIP_TASKBAR",
|
||||
transient_for => b"WM_TRANSIENT_FOR",
|
||||
clipboard => b"CLIPBOARD",
|
||||
primary => b"PRIMARY",
|
||||
targets => b"TARGETS",
|
||||
multiple => b"MULTIPLE",
|
||||
wm_state => b"WM_STATE",
|
||||
|
|
@ -508,16 +509,14 @@ impl Connection {
|
|||
}
|
||||
|
||||
#[track_caller]
|
||||
fn set_selection_owner(&self, window: x::Window) {
|
||||
fn set_selection_owner(&self, window: x::Window, selection: x::Atom) {
|
||||
self.send_and_check_request(&x::SetSelectionOwner {
|
||||
owner: window,
|
||||
selection: self.atoms.clipboard,
|
||||
selection,
|
||||
time: x::CURRENT_TIME,
|
||||
})
|
||||
.unwrap();
|
||||
let owner = self.get_reply(&x::GetSelectionOwner {
|
||||
selection: self.atoms.clipboard,
|
||||
});
|
||||
let owner = self.get_reply(&x::GetSelectionOwner { selection });
|
||||
|
||||
assert_eq!(window, owner.owner(), "Unexpected selection owner");
|
||||
}
|
||||
|
|
@ -993,7 +992,7 @@ fn copy_from_x11() {
|
|||
|
||||
let window = connection.new_window(connection.root, 0, 0, 20, 20, false);
|
||||
f.map_as_toplevel(&mut connection, window);
|
||||
connection.set_selection_owner(window);
|
||||
connection.set_selection_owner(window, connection.atoms.clipboard);
|
||||
|
||||
let request = connection.await_selection_request();
|
||||
assert_eq!(request.target(), connection.atoms.targets);
|
||||
|
|
@ -1347,7 +1346,7 @@ fn bad_clipboard_data() {
|
|||
let mut connection = Connection::new(&f.display);
|
||||
let window = connection.new_window(connection.root, 0, 0, 20, 20, false);
|
||||
f.map_as_toplevel(&mut connection, window);
|
||||
connection.set_selection_owner(window);
|
||||
connection.set_selection_owner(window, connection.atoms.clipboard);
|
||||
|
||||
let request = connection.await_selection_request();
|
||||
assert_eq!(request.target(), connection.atoms.targets);
|
||||
|
|
@ -1503,9 +1502,10 @@ fn incr_copy_from_x11() {
|
|||
let window = connection.new_window(connection.root, 0, 0, 20, 20, false);
|
||||
f.map_as_toplevel(&mut connection, window);
|
||||
|
||||
connection.set_selection_owner(window);
|
||||
connection.set_selection_owner(window, connection.atoms.clipboard);
|
||||
let request = connection.await_selection_request();
|
||||
assert_eq!(request.target(), connection.atoms.targets);
|
||||
assert_eq!(request.selection(), connection.atoms.clipboard);
|
||||
connection.set_property(
|
||||
request.requestor(),
|
||||
x::ATOM_ATOM,
|
||||
|
|
@ -1515,6 +1515,22 @@ fn incr_copy_from_x11() {
|
|||
connection.send_selection_notify(&request);
|
||||
f.wait_and_dispatch();
|
||||
|
||||
// Also give the window the primary selection.
|
||||
// Due to a bug introduced in primary selection support, `XState::selection_state` having both
|
||||
// a primary and clipboard X selection prevented clipboard INCR checks from occuring.
|
||||
connection.set_selection_owner(window, connection.atoms.primary);
|
||||
let request = connection.await_selection_request();
|
||||
assert_eq!(request.target(), connection.atoms.targets);
|
||||
assert_eq!(request.selection(), connection.atoms.primary);
|
||||
connection.set_property(
|
||||
request.requestor(),
|
||||
x::ATOM_ATOM,
|
||||
request.property(),
|
||||
&[connection.atoms.targets, connection.atoms.mime2],
|
||||
);
|
||||
connection.send_selection_notify(&request);
|
||||
f.wait_and_dispatch();
|
||||
|
||||
let mut destination_property = x::Atom::none();
|
||||
let mut begin_incr = Some(|connection: &mut Connection| {
|
||||
let request = connection.await_selection_request();
|
||||
|
|
@ -1611,7 +1627,7 @@ fn wayland_then_x11_clipboard_owner() {
|
|||
connection.verify_clipboard_owner(connection.wm_window);
|
||||
connection.get_selection_owner_change_events(false, window);
|
||||
|
||||
connection.set_selection_owner(window);
|
||||
connection.set_selection_owner(window, connection.atoms.clipboard);
|
||||
f.testwl.dispatch();
|
||||
connection.verify_clipboard_owner(window);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue