Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Set::exclude_from_history on linux by setting x-kde-passwordManagerHint #155

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

MrSmoer
Copy link

@MrSmoer MrSmoer commented Jun 9, 2024

Addresses #129
Implements the Set::exclude_from_history method for linux.

I tested this on Plasma Wayland (they implement data control protocol) and X11 using this short rust program:

use arboard::Clipboard;
#[cfg(target_os = "linux")]
use arboard::SetExtLinux;
use std::{thread, time};

fn main() {
    let ten_millis = time::Duration::from_secs(10);
    let mut clipboard = Clipboard::new().unwrap();

    let d = clipboard.set().exclude_from_history().text("secretext");
    println!("hidden");
    thread::sleep(ten_millis);
    let d = clipboard.set().text("notsecrettext");
    println!("Not hidden");
    thread::sleep(ten_millis);
}

It did not display the secretext when pressing SUPER+V(opens klipper) but it did display the notsecrettext after 10 seconds, so it works as intended.

Gnome doesn't implement the data control protocol, but testing this change with wl-paste -l(probably falling back to the xserver clipboard stuff) seemed to work as well.

In contrast, using a gnome-shell-extension that hooks the the get_mimetypes-method from within the compositor for determinig the clipboard-data-content didn't seem to work properly.
With KeepassXC, which is just using Qt's-functions for handling the clipboard, this function behaved as expected on gnome from what I can tell.

I believe there is some deeper issue with the way gnome handles the clipboard in general, but I was not able to track down the specific difference between how Qt and arboard register their clipboard content to the compositor yet.

Thanks for reviewing this and have a nice Day!

@MrSmoer MrSmoer changed the title Add exclude_from_history on linux by setting x-kde-passwordManagerHint Add Set::exclude_from_history on linux by setting x-kde-passwordManagerHint Jun 11, 2024
Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR! The structure of it looks good. My suggestions are just minor code quality ones.

I would like to test this out for myself (probably tomorrow) but I once this is cleaned up I think its ready.

src/platform/linux/mod.rs Outdated Show resolved Hide resolved
src/platform/linux/mod.rs Outdated Show resolved Hide resolved
src/platform/linux/wayland.rs Outdated Show resolved Hide resolved
src/platform/linux/wayland.rs Outdated Show resolved Hide resolved
src/platform/linux/x11.rs Outdated Show resolved Hide resolved
@MrSmoer
Copy link
Author

MrSmoer commented Jun 20, 2024

I just noticed that it didn't work properly for me without wayland-data-control on plasma-wayland and sway in any version. It works properly with the option enabled.

... and I am not sure what to do about the CI-failure

@complexspaces
Copy link
Collaborator

I just noticed that it didn't work properly for me without wayland-data-control on plasma-wayland and sway in any version. It works properly with the option enabled.

on plasma-wayland and sway in any version. It works properly with the option enabled.

IIRC Plasma defaults to Wayland these days. Do you know if xwayland is running or installed? That would be needed for anything to happen if you aren't using the wayland-data-control feature. Regarding Sway, the same thing applies. I can certainly try to test this on Ubuntu where xwayland comes by default.

... and I am not sure what to do about the CI-failure

This looks like a new lint from the latest stable Rust release. At the top of the x11.rs file, inside the use std block, you'll need to delete usize from the list.

@MrSmoer
Copy link
Author

MrSmoer commented Jun 25, 2024

I tried a little, and I wasn't able to properly solve the new CI-Issue

@complexspaces
Copy link
Collaborator

I spent several hours tonight trying to test this out, with mixed results. This seems to work find in Plasma Wayland like you observed, but GNOME/Ubuntu were a mess. In my testing there, I used pano as a clipboard monitor.

GNOME apparently doesn't support the wayland extension that wl-clipboard-rs uses to put data on the clipboard, so arboard doesn't support wayland on GNOME for the time being, I guess. I believe that a proper-windowed app like KeePassXC uses a different protocol that actually works. I don't know off the top of my head what is is though.

Next testing in a GNOME X11 environment, I still couldn't get it to work with this PR. KeePassXC works fine. From what I can see, it works while the process using arboard is still running but once the arboard-specific X11 Window has been destroyed (when Clipboard is dropped) the clipboard manager saves the contents for some reason. I wasn't able to figure out the reason.

I'm not sure how to proceed here. It doesn't feel right to merge this when it can't be confirmed working-as-intended. I most likely will not have the time to continue digging into the X11 behavior in-depth because there's so many moving parts. Do you have interest in doing so @MrSmoer?

@MrSmoer
Copy link
Author

MrSmoer commented Jul 2, 2024

I already started looking into the way the clipboard data flows, but could really wrap my head around it. The whole stack on gnome seems very convoluted to me, especially in connection with Xwayland and stuff. I dont have thaaat much time to spare, so this is going to be on my list of side-projects. I am not sure what would have to change in the ecosystem but I believe that it's just the way arboard interfaces with the x11-package or within the x11 rust-package. It would take some time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants