After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 672358 - Introduce a GdkClipboard abstraction to replace the various GtkClipboard implementations
Introduce a GdkClipboard abstraction to replace the various GtkClipboard impl...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
unspecified
Other Linux
: Normal normal
: 3.6
Assigned To: gtk-bugs
gtk-bugs
wayland
Depends on:
Blocks:
 
 
Reported: 2012-03-18 19:27 UTC by Darxus
Modified: 2013-08-28 17:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Don't use Wayland GtkClipboard implementation if also building for X (930 bytes, patch)
2012-03-22 12:45 UTC, Rob Bradford
committed Details | Review
remaining selection fixes (3.50 KB, patch)
2013-08-24 06:23 UTC, Kristian Høgsberg
committed Details | Review

Description Darxus 2012-03-18 19:27:33 UTC
I'm mostly going to repeat what I said here:
https://bugs.launchpad.net/ubuntu/+source/gtk+3.0/+bug/954352/comments/15

Someone created gtk+ packages for Ubuntu Precise with both the wayland and x11 backends enabled.  It doesn't work.  X doesn't start without putting 

export CLUTTER_BACKEND=x11
export GDK_BACKEND=x11

in ~/.xsessionrc

Once that's done, X starts, but isn't usable.  Best example, I think, is if you try to run gnome-terminal with those previous to environment variables set, you get:

Gdk-CRITICAL **: gdk_wayland_device_get_selection_type_atoms_libgtk_only: assertion `GDK_IS_DEVICE_CORE (gdk_device)' failed
Comment 1 Rob Bradford 2012-03-19 16:37:12 UTC
This is because of the way the clipboard support is implemented (ala quartz) with a separate file patched in that implements the GtkClipboard API but doesn't use the GDK abstraction.

The correct solution is to add some kind of GdkClipboard / GdkDnD abstraction to push this through - that would then allow the multiple backends to coexist.
Comment 2 Matthias Clasen 2012-03-20 01:44:23 UTC
Wouldn't it be enough to make gtk_clipboard_get_for_display smart enough to return the right clipboard implementation for the display at hand ?
Comment 3 Emmanuele Bassi (:ebassi) 2012-03-20 07:27:45 UTC
(In reply to comment #2)
> Wouldn't it be enough to make gtk_clipboard_get_for_display smart enough to
> return the right clipboard implementation for the display at hand ?

we'd still need to implement a separate GtkClipboardImplementation class, with a vtable composed of all the methods of the class, and then make an implementation for x11, wayland, win32, and quartz and select at run time depending on the display manager type.
Comment 4 Matthias Clasen 2012-03-20 10:13:07 UTC
right, of course
Comment 5 Matthias Clasen 2012-03-20 10:13:25 UTC
sounds like a good 3.6 goal
Comment 6 Rob Bradford 2012-03-20 16:49:33 UTC
For 3.4 do we want to:

1. Pull out the copy and paste implementation
2. Make configure bail out if you try to build both Wayland and X backend
3. Only build the Wayland GtkClipboard implementation iff X backend is disabled
4. Do nothing
Comment 7 Emmanuele Bassi (:ebassi) 2012-03-20 16:53:22 UTC
apparently, the Quartz backend follows 4: if you build both the x11 and the quartz backend on Mac OS you'll only get the quartz-based clipboard variant.

so, at this point, doing nothing and waiting for a proper abstracted implementation in 3.6 (with a Wayland implementation tracking 1.0) may just be the better option.
Comment 8 Rob Bradford 2012-03-22 12:45:04 UTC
Created attachment 210329 [details] [review]
build: Don't use Wayland GtkClipboard implementation if also building for X 

This patch is for "#3" above. I've tested it with both X11 and Wayland builds.

I know it's close to freeze time so I won't be offended if it doesn't land.
Comment 9 Matthias Clasen 2012-03-22 13:47:15 UTC
Lets hold this for 3.4.1 at this point, I want to stick to crash fixes between now and next week
Comment 10 Matthias Clasen 2012-03-29 00:28:16 UTC
Review of attachment 210329 [details] [review]:

Looks fine for 3.4.1 as a quick fix to improve the situation.
For 3.6, we should look at doing the clipboard abstraction
Comment 11 Rob Bradford 2012-03-29 10:53:45 UTC
Comment on attachment 210329 [details] [review]
build: Don't use Wayland GtkClipboard implementation if also building for X 

Patch committed but keeping bug open for underlying issue.
Comment 12 Matthias Clasen 2013-03-12 21:46:51 UTC
retitling for clarity
Comment 13 Matthias Clasen 2013-08-20 15:34:55 UTC
first part of this work has been pushed
Comment 14 Kristian Høgsberg 2013-08-24 06:23:09 UTC
Created attachment 252982 [details] [review]
remaining selection fixes

I pushed a number of straightforward fixes for wayland clipboard handling, and this patch is the last few fixes required to get basic copy and paste working.

The patch is essentially a couple of heuristics to either filter out the legacy X atom content types or rewrite UTF8_STRING to text/plain;charset=utf-8.  The patch works fine, but I wasn't sure if we wanted to do this at a higher level instead.  We could make gtk_clipboard_request_text() just ask for text/plain;charset=utf-8, and then let the X backend expand that to the UTF8_STRING, COMPOUND_TEXT and STRING targets as well.
Comment 15 Rob Bradford 2013-08-27 13:49:33 UTC
Kristian, I think we should land this as it currently stands and look to do the change in the higher level in the next cycle.
Comment 16 Benjamin Otte (Company) 2013-08-27 14:07:36 UTC
Yeah, the fix by Kristian makes sense.

We need a clipboard abstraction that gets rid of GtkTargetEntry or mime types because mime types don't work for at least text. But I suspect they're equally broken for images, HTML, and all the other kinds of data you want to exchange between display server and apps.

I originally wanted to avoid having per-backend and per-data-type serialization functionality, but I'm more and more convinced that is the right approach, at least internally.
Comment 17 Benjamin Otte (Company) 2013-08-27 14:13:56 UTC
Also, the whole GtkClipboard API is a mess because it is severly underdefined and doesn't tell you what it does.

For example, I'm not sure if Wayland or X11 get this one right: "When the results of the result are later received the supplied callback will be called." And that's only from the docs of the most important function.
Comment 18 Rob Bradford 2013-08-28 13:31:12 UTC
Comment on attachment 252982 [details] [review]
remaining selection fixes

Kristian has pushed these changes as 459e6a3
Comment 19 Rob Bradford 2013-08-28 13:32:21 UTC
Are we good to close this bug now?
Comment 20 Matthias Clasen 2013-08-28 16:09:33 UTC
There is more work to do, but it is no longer wayland-specific, so we could move the bug from wayland to gdk.
Comment 21 Kristian Høgsberg 2013-08-28 17:19:50 UTC
(In reply to comment #18)
> (From update of attachment 252982 [details] [review])
> Kristian has pushed these changes as 459e6a3

Only just pushed it now:

commit 921e81ebc94ac1d706bb54436fd870eb0d47f091
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Wed Aug 28 10:18:36 2013 -0700

    wayland: Don't try to fetch ICCCM text content types
    
    Go straight for text/plain;charset=utf-8 and ignore fallback attempts for
    increasingly obscure text types.

commit 73eea248d4cd01f486a5dacfac35b4067f6c1650
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Wed Aug 28 10:17:10 2013 -0700

    wayland: Filter out ICCCM content types when setting clipboard contents

commit 630c52c308e84cb0a5aa0856c667aac4f543f4c2
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Wed Aug 28 10:14:42 2013 -0700

    wayland: Fix missing return value in gtk_clipboard_wayland_set_contents()

Closing this one now.
Comment 22 Kristian Høgsberg 2013-08-28 17:21:24 UTC
Ha, Benjamin pushed it, all good then.  I did like the "increasingly obscure" commit message though.