Repository navigation
Skip the X11 clipboard manager handover when no manager is running - #246
RaphaelFakhri wants to merge 2 commits into
Conversation
| // Per the `ClipboardManager` specification, if nothing owns the `CLIPBOARD_MANAGER` | ||
| // selection then no clipboard manager is running and clipboard persistence isn't | ||
| // available. The X server would reject the conversion right away, so don't wait for | ||
| // a handover that can never happen. |
There was a problem hiding this comment.
Please link an exact section of the specification or quote it directly inline.
Additionally, let's go ahead and link to the GTK clipboard implementation which performs this operation as well as a citation of correctness: https://gitlab.gnome.org/GNOME/gtk/-/blob/1ef7621a27abf0fb0f3aa719b895868b55766156/gdk/x11/gdkclipboard-x11.c#L645
There was a problem hiding this comment.
Done in fa56ca8. The comment now quotes both sections of the specification with links to them, and links the GTK check at gdkclipboard-x11.c#L645.
| // available. The X server would reject the conversion right away, so don't wait for | ||
| // a handover that can never happen. |
There was a problem hiding this comment.
| // available. The X server would reject the conversion right away, so don't wait for | |
| // a handover that can never happen. | |
| // available. |
| .unwrap(); | ||
|
|
||
| let start = Instant::now(); | ||
| inner.ask_clipboard_manager_to_request_our_data().unwrap(); |
There was a problem hiding this comment.
I'm not sure this test is valid because Clipboard::new uses the global clipboard and therefore may race with other clipboard tests running in parallel and being Droped (or writing to it). Its also something that is going to have variable results depending on if its running in CI or on someone's desktop.
I think its probably best to just remove it since we have no way to mock the X server itself.
There was a problem hiding this comment.
Removed in fa56ca8, along with the MAX_HANDOVER_DURATION constant that only the test used. cargo fmt --check and cargo clippy --all-features -- -D warnings pass.
Summary
When a
Clipboardthat owns theCLIPBOARDselection is dropped on X11, arboard asks the clipboard manager to save the contents by converting theCLIPBOARD_MANAGERselection toSAVE_TARGETS. If no clipboard manager is running, nothing ownsCLIPBOARD_MANAGER, so the X server immediately answers with a refusedSelectionNotify. arboard then waits for a content request that never arrives, which:Could not hand the clipboard contents over to the clipboard manager. The request timed out.even though there was nothing to hand over to.This happens on every drop under window managers without a clipboard manager, as reported in #226.
The ClipboardManager specification states that clipboard persistence is unavailable when
CLIPBOARD_MANAGERhas no owner. This change checks the owner first and skips the handover when there is none. The behavior with a running clipboard manager is unchanged.Related to #226. The
all_testspersistence check in that report still requires a clipboard manager, because without one nothing can keep the contents after the owner exits.Testing
The fix has no automated test, because the X server can't be mocked and
Clipboard::newuses the global clipboard, which races with the other clipboard tests.cargo fmt --all -- --checkandcargo clippy --all-features -- -D warnings -D clippy::dbg_macroare clean.