Race using wlr_data_control to create a clipboard manager

[davidedmundson]

Our clipboard manager (klipper) works as follows:

• On a new clipboard, we copy any text
• If the clipboard becomes empty (typically because a client died), we paste our previously copied text
• A user can also explicitly select an item from the history

We had this on X, and I've retrofitted wlr_data_control into it.

I've implemented wlr_data_control and all of wl-copy, wl-paste work perfectly, to some extent klipper works, but there's a race I can't fix neatly, which I think would happen for all other clipboard managers trying to do the same thing.

My race is:

first copy:

• firefox creates a wl_data_offer
• kwin sees it forwards it klipper which copies the text...
  (all good so far)

second copy:

• firefox deletes its old wl_data_offer before creating a new one (unlike other toolkits, but still a valid thing to do)

• kwin forwards this update to klipper

• klipper knows the clipboard is empty and starts its operation to prevent an empty clipboard

• firefox creates a new wl_data_offer and calls set_selection

• klipper creates a new wlr_data_offer (with the old clipboard text) and calls set_selection

The compositor gets these in any order, and we end up replacing our new clipboard content with out-of-date previous clipboard contents.

---

Ultimately to prevent a race I think we need an additional fence like:

    <event name="selection">
        <arg name="id" type="object" interface="zwlr_data_control_offer_v1" allow_null=true>
         An integer value that increases, like the xdg_shell configure events
        <arg name="serial" type="int">
    </event>
    
    <request name="set_selection">
          <arg name="source" type="object" interface="zwlr_data_control_source_v1"
        allow-null="true"/>
        Optional serial which matches the clipboard event we're responding to. If this is out-of-date this selection will be immediately cancelled and ignored. If negative, we always apply the new selection.
        <arg name="serial" type="int"> 
    </request>

[davidedmundson]

As an alternative, less technically correct, but good enough solution to my problem.

    <request name="set_selection">
          <arg name="source" type="object" interface="zwlr_data_control_source_v1"
        allow-null="true"/>
        <arg name="replaces_current_clipboard" type="boolean"> Some sort of flag to say we only apply the contents if no client has a selection.  Otherwise the selection is immediately cancelled.
    </request>

[emersion]

I agree that race is worth fixing. Serials are uint in the Wayland protocol and any value is a valid serial, so I guess we'll need something else (like two separate requests).

But if an application really wants to unset the selection, should a clipboard manager always reset it to the previous value? For instance if an application copies a password to the clipboard and wants to empty the clipboard after 30 seconds, we don't want the clipboard manager to reset the clipboard with the password text afterwards.

[davidedmundson]

But if an application really wants to unset the selection, should a clipboard manager always reset it to the previous value?

Yes, but that's also more complex. We don't want to just not restore it, we also want to not show it in our UI or save it to disk. That needs to happen at the time the selection is made, not the time it's cleared.

On X11 we "solve" this with a magic mimetype. We have a mimetype of x-kde-passwordManagerHint and then our clipboard manager just skips it.

[emersion]

Well, in a perfect world, all clients support the text-input protocol, and the passwords never go through the clipboard, they are fed directly to the receiving client via text-input.

I wonder if there are other use-cases for emptying the clipboard?

[davidedmundson]

I had a quick look through our bug reports for klipper, which go back 14 years, and password managers seem to be the only case where a behaviour change was requested.

[davidedmundson]

I agree that race is worth fixing. Serials are uint in the Wayland protocol and any value is a valid serial, so I guess we'll need something else (like two separate requests).

Should I submit a MR to do that in a v2.
If I do it as 2 requests, and I send the serial of the selection event separately then it could be backwards compatible.

[emersion]

Yeah, I think that'd be a good idea.

[claell]

@davidedmundson Do you have an update on the state of this? If I am not mistaken this issue starts to hit now after the release of Plasma 5.20, so this will likely produce user frustration soon.

[davidedmundson]

I merged a hacky workaround for 5.20
Effectively like above but via a mimetype offer with a special key meaning it's from klipper and only replaces an empty clipboard.

Obviously I want to merge something cleaner and universal but the time pressure is off.

[claell]

Has that been recently and still needs time to be integrated into 5.20.1, maybe?
Asking, because I am experiencing this bug on 5.20 right now. For reference: https://bugs.kde.org/show_bug.cgi?id=424754.

[emersion]

wlr-protocols has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/issues/92