Bug: hard-coded timeout insufficient for large images [X11]

[jasonterando]

The problem | 问题描述

Hi, sorry for this being in English. In x11.rs, line 327, there is a hard-coded timeout of 500 ms for ClipboardContext.read. The problem is that for larger clipboard content, like a large image (like attached), 500 ms does not appear to be enough to read the clipboard data. The "image" example will fail with the attached image, but will succeed if the timeout in ClipboardContext.read is increased to 5000 ms. I believe the answer is to scale the timeout with the clipboard content length, but I am not knowledgeable enough about X11 yet to know if this is even possible. Alternatively, I guess we could add a user-specified timeout or perhaps a "cancel" mechanism.

I'm happy to take on the work and put in a pull request, if you point me in the right direction. Thanks!

Release version | 版本

0.1.11

Operating system | 操作系统

Linux

Logs | 日志

Terminal output with existing timeout of 500 ms:

["image/png", "image/jpeg", "image/avif", "image/bmp", "image/x-bmp", "image/x-MS-bmp", "image/x-icon", "image/x-ico", "image/x-win-bitmap", "image/vnd.microsoft.icon", "application/ico", "image/ico", "image/icon", "text/ico", "image/jxl", "image/tiff", "image/webp", "audio/x-riff"]
err=No image data found

Terminal output with timeout increased to 5000 ms (files are written out successfully)

["image/png", "image/jpeg", "image/avif", "image/bmp", "image/x-bmp", "image/x-MS-bmp", "image/x-icon", "image/x-ico", "image/x-win-bitmap", "image/vnd.microsoft.icon", "application/ico", "image/ico", "image/icon", "text/ico", "image/jxl", "image/tiff", "image/webp", "audio/x-riff"]

[jasonterando]

Took another look sitting at the airport... maybe it makes sense to just None the timeout altogether in read, since you are implementing INCR

[ChurchTao]

@jasonterando Maybe I could open a method with a timeout to solve this problem?

[jasonterando]

I am going to put up a pull request with an idea, let me know what you think, including if it is a bad idea :). The idea is that we would create an alternate constructor (new_with_options) for ClipboardContext that would allow options to be passed in. Currently, the only option is read_timeout, which is an Option<Duration>, and by default, this would remain at 500 ms.

If we decided to have additional configurable options, for X11 or other platforms, it would be an extendable pattern.

The rationale for my approach is as follows:

1. Read timeout is only implemented for X11, it should not be available for other platforms, and ideally, triggers a build error if attempts are made to configure it for Windows, MacOS, etc.
2. We want to avoid either breaking existing code or changing its existing behavior (or... we could choose to default read timeout to None)
3. Configuring the ClipboardContext when we create it is a lot easier than creating overrides or new versions of all of the Read functions

I've updated the image example to demo how an application would use this, but it is basically the following:

#[cfg(unix)]
fn setup_clipboard() -> ClipboardContext {
   ClipboardContext::new_with_options(ClipboardContextX11Options { read_timeout: None }).unwrap()
}

#[cfg(not(unix))]
fn setup_clipboard(ctx: &mut ClipboardContext) -> ClipboardContext{
   ClipboardContext::new().unwrap()
}

fn main() {
   let ctx = setup_clipboard();
   // read stuff..
}

if this approach works for you and you accept it, I can then implement support in tauri-plugin-clipboard

[ChurchTao]

Good job, I had this in mind, let's run rustfmt and clippy, and I'll release it in the next version

[ChurchTao]

v0.2.1