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

X11 clipboard sync #270

Merged
merged 10 commits into from
Jul 1, 2017
Merged

X11 clipboard sync #270

merged 10 commits into from
Jul 1, 2017

Conversation

nyorain
Copy link
Contributor

@nyorain nyorain commented Jun 29, 2017

This adds (basic) synchronization between the xwayland and wayland clipboard.
It also fixes some found bugs in the polymorphic data-source implementation and cleans up some aspects of the xwm implementation (mainly removes the global state).

The xselection implementation is nothing beautiful and can be improved. It implements only the absolutely needed functionality and does not care about the icccm spec too much at places (i.e. the given timestamps are probably not always correct and it does not implement INCR data transfer but this is usually not needed).

It only works for text data on the clipboard at the moment but can be easily extended to further types (is there anything else the clipboard is used for?).
Note that it cannot be easily extended to support all types in a generic way though since we have to map the TARGETS atoms from x11 to mime types in wayland (and the other way around) and there is probably no general way to do this. Many applications and toolkits use mime types as x11 targets already though so we might simlpy guess there (not sure if this is a good idea and again, text is probably the only really important clipboard target).

Please tell me if there is anything that has to be done to consider this feature done for now.
This would additionally need some further testing it works for me with the wlc example and sway (mainly tested with chromium and termite on the both backends) but this is so ugly/easy to get wrong that all testing is appreciated.

@ddevault
Copy link
Collaborator

Quick notes just from reading over your description:

(is there anything else the clipboard is used for?).

Images are pretty common. Also perhaps rich text (i.e. libreoffice).

Many applications and toolkits use mime types as x11 targets already though so we might simlpy guess there (not sure if this is a good idea and again, text is probably the only really important clipboard target).

Maybe we could have a hardcoded list of hairy conversions and then fall back on just copying it over? Do you know what other compositors do?

Syncronization works in both directions, right?

@nyorain
Copy link
Contributor Author

nyorain commented Jun 29, 2017

Synchronization works in both directions. Weston implements only text as well and i have no idea what gnome does (probably something similar to what you are describing).
So yes, try some conversions and then fall back sounds like a good idea, should i implement it like that?

@ddevault
Copy link
Collaborator

Yep. See if you can at least figure out image data. Try copying it from a wayland web browser and pasting it into gimp.

notify.property = property;

xcb_send_event(xwm->connection, 0, requestor, XCB_EVENT_MASK_NO_EVENT, (const char*) &notify);
xcb_flush(xwm->connection);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some indentation issues here.

if (xwm->selection.send_fd != -1)
close(xwm->selection.send_fd);

fcntl(fd, F_SETFL, O_WRONLY | O_NONBLOCK);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Around here too. Give it a quick style check overall.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, will do it

NET_WM_WINDOW_TYPE_DND,
NET_WM_WINDOW_TYPE_NORMAL,
ATOM_LAST
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a better place to source this from? Some xcb header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid not, it used to be in xwm.c i believe. I just moved it to a header since selection.c and xwm.c need it

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's too bad.

@nyorain
Copy link
Contributor Author

nyorain commented Jun 30, 2017

Fixed the style issues and implemented the custom conversions with fallback to just offer the atom name as mime type if it might be one (the code checks if the atom name contains a '/' at least to not offer targets like TIMESTAMP or TARGETS which are obviously no mime types and not understand by any wayland application). Still works for me in basic test (chromium, firefox, terminal, etc), was not yet able to test it with images (most applications use filepaths/urls for images anyways).

@ddevault
Copy link
Collaborator

Excellent. A quick glance looks good, I will get this tested and merged later today. Great work!

@ddevault
Copy link
Collaborator

ddevault commented Jul 1, 2017

I tested this with image data and it did not work. Here are some easy steps to reproduce:

  1. Open a web browser with wayland (i.e. env QT_QPA_PLATFORM=wayland-egl qutebrowser) and copy an image to the clipboard (the image, not its url)
  2. Open gimp and file -> create -> from clipboard

That being said, this is better than what we had before. I'm going to merge this now and expect further issues to be resolved in further pull requests.

@ddevault ddevault merged commit 314d09a into Cloudef:master Jul 1, 2017
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