Improved wl-paste mimetype handling in ImageGrab

[rrcgat]

Fixes #7093

Changes proposed in this pull request:

• Add -t <mime/type> arguments to wl-paste when there is an available image MIME type.

[rrcgat]

The CI skip my test, which covered on my local machine. I'm not familiar with GitHub CI, anyone helps?

[nulano]

I guess wl-clipboard is not installed by default in the Ubuntu 22.04 image:
https://github.com/python-pillow/Pillow/actions/runs/4707895239/jobs/8351231955?pr=7094#step:8:4513
https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md

[rrcgat]

@nulano Thanks for your reply, I'll try to modify the GitHub workflow to make it work.

[radarhere]

What is the OSError that might be thrown here?

[rrcgat]

Oh, OSError will not be thrown here anymore. I made a mistake with skipif condition and OSError reported in other systems.

[rrcgat]

Hi everybody, I've tried a lot in test-workflow branch, but I couldn't find a solution to support wl-clipboard in xvfb.

The errors include:

1. Failed to connect to a Wayland server (wl-clipboard)
2. Core dumped without xwayland (in other test files)
3. Missing a seat (wl-clipboard, I've tested seatd on Docker, not work)

Do you have any suggestions?

[radarhere]

If you don't know of a simple way to add coverage for this, then I wouldn't think that is a blocker for this PR. You're not adding code that isn't covered into the middle of an otherwise-covered section - we weren't testing wl-paste or xclip on our CIs to begin with. See https://app.codecov.io/gh/python-pillow/Pillow/blob/main/src/PIL/ImageGrab.py

[rrcgat]

Hi @radarhere , I finally found a way to test wl-paste by using sway. The CI is running, I'll commit to this branch if it works.

[radarhere]

Unfortunately, there is a problem. If I remove the fix, the test doesn't fail.

[rrcgat]

Hi @radarhere , If you remove the fix, the test won't fail because there's only one MIME type (set by subprocess.call(["wl-copy"], stdin=raw_image)), so wl-paste without arguments works fine.

It seems impossible to pick several MIME types with wl-copy, so I don't know how to test this scenario.

[rrcgat]

I didn't set the CODECOV_TOKEN previously, but after setting it, I reran some workflows.

[radarhere]

I've created rrcgat#1 with some suggestions for this PR.

[rrcgat]

Thanks for getting back to me! Apologies for the inconvenience, I wasn't aware of these suggestions until you left a comment.

[radarhere]

I've discovered a problem. I ran Ubuntu, switched to Wayland, opened https://python-pillow.org/images/pillow-logo-light-text-1280x640.png in Firefox, copied the image, and ran wl-paste -l.
The first option in wl-paste -l was TIFF, and when I selected that as the mime type, it saved an empty file!
Only when I chose PNG instead did the image save.

png.mp4

If the first suggestion from wl-paste -l doesn't necessarily work, I guess that means this PR needs to iterate through the mime types until it finds one that works? That seems cumbersome.

I later tried copying a GIF from FIrefox. wl-paste -l gave the same list of mime types.

gif.mov

[rrcgat]

@radarhere Yes, you're right, I didn't test it to much because I simply thought image/png and image/gif is enough for the image type of wl-paste.

In the early days before, I found that image/png can be use for ICO and JPEG in the case of wl-paste. Without much knowledge of image type, I thought that image/png can be use for other image type, so I only check for PNG and GIF format in my first commit.

When I opened https://python-pillow.org/images/pillow-logo-light-text-1280x640.png, Firefox and Chrome offered different MIME types (Chrome only offered text/html and image/png), and I have different order of wl-paste -l compare to yours, which both copied from Firefox (with Gnome and Wayland). For the PNG and JPEG image, even GIF (for example, https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/200px-Rotating_earth_%28large%29.gif) Chrome offered the same MIME type (the output of wl-paste -l).

It seems that there are too many cases of copy, iterate through the mime types works (with different MIME types, wl-paste output with different file size, I don't know if it matters), but as you said, it's cumbersome.

[rrcgat]

Use the hard way here: https://github.com/rrcgat/Pillow/pull/2/files

What do you think?

def grabclipboard():
    ...
    else:
        if shutil.which("wl-paste"):
            args = ["wl-paste"]
            output = subprocess.check_output(["wl-paste", "-l"]).decode()
            clipboard_mimetypes = set(output.splitlines())

            Image.preinit()
            available_mimetypes = set(Image.MIME.values()) & clipboard_mimetypes
            if not available_mimetypes:
                Image.init()
                available_mimetypes = set(Image.MIME.values()) & clipboard_mimetypes
            if available_mimetypes:
                return _grab(
                    *[["wl-paste", "-t", mimetype] for mimetype in available_mimetypes]
                )
        elif shutil.which("xclip"):
            args = ["xclip", "-selection", "clipboard", "-t", "image/png", "-o"]
        else:
            msg = "wl-paste or xclip is required for ImageGrab.grabclipboard() on Linux"
            raise NotImplementedError(msg)
        return _grab(args)


def _grab(*call_args):
    fh, filepath = tempfile.mkstemp()
    for args in call_args:
        subprocess.call(args, stdout=fh)
        if os.path.getsize(fh):
            break
    os.close(fh)
    try:
        im = Image.open(filepath)
        im.load()
    finally:  # Remove it anyway or left it when Image.open fails (for debug usage)?
        os.unlink(filepath)
    return im

[radarhere]

I think iterating through all of the different mime types doesn't sit nicely with #7110, a request that we raise an error on a failed ImageGrab operation - because if we try 12 different wl-paste variations, then we don't know which error to show.

Let me suggest this instead. Given the output of wl-paste -l,

1. If there is only one mime type, use that.
2. If there are multiple mime types, and one of them is "image/png", use that. This worked in my earlier recording for a GIF, when the first mime type was TIFF.
3. Otherwise, use the first mime type listed.

What do you think?

[rrcgat]

@radarhere I agree, this approach is better than iterating through all mime types and it works well in my initial case. I will implement this solution based on your suggestion.

[rrcgat]

Using the first mime type when there are multiple mime types without image/png is great, although image/png mime type is provided in the video:

Screencast.from.2023-05-23.18-50-35.webm

[radarhere]

Thanks, looks good to me. I've created rrcgat#3 with minor formatting suggestions.