`wl_clip_persist` hands satellite a `WritePipe` with the `O_NONBLOCK`
flag, which prevented more than 64 KiB (the Linux pipe capacity since
2.6.11) of selection data to be transferred. Since my research concluded
having `O_NONBLOCK` on Wayland's pipe transfer ends was probably valid,
I implemented basic handling for such a scenario.
I previously considered just using `fcntl` to remove the `O_NONBLOCK`
flag, but decided that was way too hacky, as it would not resolve the
underlying problem (which could easily be triggered by the receiving
program resetting `O_NONBLOCK` during the `write_all`.
The previous method of converting X selections had some flaws, including
one where XWayland would fail to send the necessary PropertyNotify
events if multiple INCR requests were sent to the same selection at
once. By using `write_to` to create a FIFO queue and `next_conversion`
to advance the queue, we can guarantee only one request is being
converted at a time, working around this issue.
Both PRIMARY and CLIPBOARD were using the same target atoms for their
property. When both are simultaneously requested (the behavior of
`wl-clip-persist --clipboard both`), the first would delete the property
the second was prepared to write data to, resulting in getting the
GetProperty reply containing the failure data.
This commit distinguishes `target` as the atom of a mime type contained
within `TARGETS`, and `property` as the atom which contains both data of
`target` and which selection requested it.
The SelectionClear event was previously calling `handle_new_owner`,
which relies on `SelectionClear` being sent only after a
`SetSelectionOwner` event. Even so, the
`XFixes::SelectionEvent::SetSelectionOwner` event already handles this
(and more obviously so). The result was that whenever Wayland owned the
X selection and X took it back, `handle_new_owner` would be called
twice, as would all of the machinery involved in acquiring a selection.
This behavior manifests as a bug if Wayland quickly reclaims ownership
of the selection, which can cause the selection event to be cancelled
before getting the SelectionNotify response is received, resulting in an
empty Wayland selection.
The clipboard and primary selections previously used the same atom to
store TARGETS when queried for them by `handle_target_list`. When a
program requests both selections simultaneously, problems such as one
selection using the target list of the other, then deleting it and
requiring the second target to get the selection targets again could
occur. This was observed when using `wl-clip-persist` and could result
in selection desyncs.
This change gives a unique atom to each primary and clipboard selection
targets so they no longer compete over the same property. It also adds
updates to the `copy_from_x11` intergration to test for this issue.
LuckShiba's commit was not formatted with `rustfmt`, so I took the
opportunity to do some additional refactors.
Some bulky match constructions in xstate/selection were made more
compact as well.
Brought to my attention by #252, several `unwrap`s exist on code whose
failure does not justify the termination of the program, so I took some
time to replace them with proper error handling.
The request for TARGETS in `handle_selection_request` used `unwrap`.
Since the code for a failure of responsing to `SelectionRequest` exists,
it simply uses that.
`handle_target_list` now early returns if receiving the
`GetProperty` request fails.
`handle_new_owner` does not update the timestamp for the last selection
on failure.
`set_owner` now returns a `bool` to indicate whether the
`SetSelectionOwner` and `GetSelectionOwner` requests succeeded and
whether they transferred ownership successfully or not. The call spots
have also been modified to not change the `selection_state` if this call
fails.
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.
Since the range end was always encoded in the data length, calculating
the end of the selection is a bit cleaner and removes a potentially
confusing slice operation on a slice.
Resolve Clippy's `too_many_arguments` lint on
`handle_selection_request` by removing the `success` and `refuse`
parameters and returning a bool indicating which needs to be run.
Use the helper functions defined last commit to simplify
`incr_copy_from_x11`
Reduce the `match` statement checking if the target is the TARGET
atom to an if statement. Although using `match` statements as
`if`/`else if`/`else` chains occur elsewhere, this one is both only 2
cases, and already of particular relevance to the current work.
Refactor `paste_impl` to be much easier to read. I did this code algebra
way back while investigating #142, but since I never figured that out,
the change has just been sitting in my stash.
refactor: simplify paste_data logic, some unnests
Since the connection handshake establishes the most data a request can
send, if the data length exceeds that limit, we can follow ICCCM 2.7.2
sending the INCR property and continuing to send data via PropertyNotify
events.
To test the changes, we create `XState::set_max_req_bytes` to forcefully
trigger the INCR mechanism in integration test runs with a constant,
substantially less amount of data.
Before, every time the clipboard selection changed, satellite would copy
everything on it and force itself to be the clipboard owner, regardless
of X11 or Wayland. Now, satellite is only the owner when the clipboard
owner comes from Wayland, and uses the XFixes extension to watch for
changes in clipboard ownership X11 side. Satellite also now avoids
copying all of the clipboard contents into memory every time, instead
copying directly on request. This is a pretty big change, but should
hopefully help make the clipboard more stable.
Also added some misc test cleanup/using helper functions where possible.
Using the XFixes extension may also end up being necessary for
implementing drag and drop, so it's good the infrastructure is there
now.
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
