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 580511 - gdk_x11_atom_to_xatom_for_display translates GDK_NONE as "NONE" not None
gdk_x11_atom_to_xatom_for_display translates GDK_NONE as "NONE" not None
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2009-04-27 21:31 UTC by Simon Tatham
Modified: 2009-08-31 18:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-Bug-580511-gdk_x11_atom_to_xatom_for_display-tra.patch (2.17 KB, patch)
2009-05-22 20:01 UTC, Martin Nordholts
none Details | Review
dnd-none-test.c (1.46 KB, text/plain)
2009-05-22 20:04 UTC, Martin Nordholts
  Details
0001-Bug-580511-gdk_x11_atom_to_xatom_for_display-tra.patch (2.22 KB, patch)
2009-05-23 10:10 UTC, Martin Nordholts
none Details | Review

Description Simon Tatham 2009-04-27 21:31:50 UTC
The gdk_x11_atom_to_xatom_for_display() function, in gdk/x11/gdkproperty-x11.c, responds to the special input value GDK_NONE by generating an XInternAtom call for the string "NONE", which yields an X11 atom identifier not equal to the special value None.

The effects of this can be seen when requesting an unsupported selection target type from a GTK-using application. To demonstrate this most easily I need to refer to my own small utility 'xcopy', which has a verbose logging mode that shows up the problem.

If you download xcopy.tar.gz from http://www.chiark.greenend.org.uk/~sgtatham/utils/, you can demonstrate the bug by the following procedure:

 - Start up gnome-terminal.
 - Drag to select some text in it.
 - Run 'xcopy -v -r -a RECTANGLE'. (-v means verbose; -r means read the contents of the selection; -a RECTANGLE means to request a data type identified by the word RECTANGLE, which is not a data type supported by gnome-terminal.)

The log data output by xcopy will include a line of the form
got SelectionNotify: requestor=03400001 selection=PRIMARY target=RECTANGLE property=NONE
The field "property=NONE" is the key one. If you try exactly the same test with xterm, it will instead say
got SelectionNotify: requestor=03200001 selection=PRIMARY target=RECTANGLE property=None
"None" in title case is output by xcopy in response to the atom value in the SelectionNotify event being the special value None. In the case where it's talking to gdk, however, it instead outputs NONE in all-caps, and that's because it received a non-null atom value that expanded to the _string_ NONE. The upshot is that a program receiving such a notification may wrongly think that it has received some zero-length selection data, rather than that it has not received any data and should try a different format.

In particular, this caused me to have an actual practical problem with Evolution 2.12.3 talking to the Cygwin X server on Windows. Right-clicking a URL in Evolution and requesting "Copy Link Location" copies to the clipboard in only plain STRING format; if you then try to paste into a Windows window, the Cygwin X server's clipboard synchronisation module first requests the selection in COMPOUND_TEXT format - and gets one of these pseudo-NONE atoms in response, which inhibits it from falling back to a different format, so cut and paste fails.

From looking at the code, the problem seems to begin when gtkselection.c calls gdk_selection_send_notify_for_display with property set to GDK_NONE. gdk_selection_send_notify_for_display() fills in xevent.property by passing that value straight on to gdk_x11_atom_to_xatom_for_display. It should either check its input for GDK_NONE and avoid calling that function, or (I think more likely) gdk_x11_atom_to_xatom_for_display ought to check _its_ input for GDK_NONE so as to fix the same problem in any other situation where it might crop up.

I've just downloaded gtk+ from git and verified (by code inspection) that the problem still seems to be present in the most up-to-date revision as far as I can see. I attach a tiny patch by way of demonstration of the sort of fix that I think should work, though unfortunately I haven't been able to test it:

diff --git a/gdk/x11/gdkproperty-x11.c b/gdk/x11/gdkproperty-x11.c
index 36b8e9f..bf18674 100644
--- a/gdk/x11/gdkproperty-x11.c
+++ b/gdk/x11/gdkproperty-x11.c
@@ -190,6 +190,9 @@ gdk_x11_atom_to_xatom_for_display (GdkDisplay *display,

   g_return_val_if_fail (GDK_IS_DISPLAY (display), None);

+  if (atom == GDK_NONE)
+    return None;
+
   if (display->closed)
     return None;
Comment 1 Martin Nordholts 2009-05-22 20:00:10 UTC
The commit d183f44748a8d introduces problems, namely critical warnings when a drop target within the same application calls

  gdk_drag_status (context, 0, time);

in a drag-motion callback. A sample stacktrace when breaking on warnings looks like this

  • #0 __kernel_vsyscall
  • #1 raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #2 abort
    at abort.c line 88
  • #3 IA__g_logv
    at gmessages.c line 506
  • #4 IA__g_log
    at gmessages.c line 526
  • #5 IA__g_return_if_fail_warning
    at gmessages.c line 541
  • #6 IA__gdk_x11_xatom_to_atom_for_display
    at gdkproperty-x11.c line 296
  • #7 xdnd_action_from_atom
    at gdkdnd-x11.c line 1922
  • #8 xdnd_status_filter
    at gdkdnd-x11.c line 1995
  • #9 xdnd_send_xevent
    at gdkdnd-x11.c line 2214
  • #10 IA__gdk_drag_status
    at gdkdnd-x11.c line 3620
  • #11 gimp_container_tree_view_drop_status
    at gimpcontainertreeview-dnd.c line 164
  • #12 gimp_container_tree_view_drag_motion
    at gimpcontainertreeview-dnd.c line 272
  • #13 _gtk_marshal_BOOLEAN__OBJECT_INT_INT_UINT
    at gtkmarshalers.c line 411

Comment 2 Martin Nordholts 2009-05-22 20:01:19 UTC
Created attachment 135201 [details] [review]
0001-Bug-580511-gdk_x11_atom_to_xatom_for_display-tra.patch

Proposed fix

commit 26e61d5ad4d1a3484fdce73e99f5f5f5d48787d0
Author: Martin Nordholts <martinn@src.gnome.org>
Date:   Fri May 22 21:57:14 2009 +0200

    Bug 580511 – gdk_x11_atom_to_xatom_for_display translates GDK_NONE as

    Allow None <=> GDK_NONE conversions and remove special casing. If we
    don't allow these conversions we need to add special casing to more
    places so it's better to just allow the conversions.
Comment 3 Martin Nordholts 2009-05-22 20:04:11 UTC
Created attachment 135202 [details]
dnd-none-test.c

Simple application that can be used to provoke the critical warnings, just drag from the Drag source to the Drag dest. Compile command for your copy-paste pleasure:

gcc -g `pkg-config --libs --cflags gtk+-2.0` dnd-none-test.c -o dnd-none-test
Comment 4 Matthias Clasen 2009-05-23 02:53:29 UTC
I prefer to fix the places where we confuse None with an atom.
Comment 5 Martin Nordholts 2009-05-23 10:08:56 UTC
Even though None is not strictly an Atom it is still a valid value in an variable of type Atom. What would be the benefit of special-casing this value all over the place?
Comment 6 Martin Nordholts 2009-05-23 10:10:46 UTC
Created attachment 135228 [details] [review]
0001-Bug-580511-gdk_x11_atom_to_xatom_for_display-tra.patch

Previous patch had a copy-paste problem.
Comment 7 Michael Natterer 2009-05-29 13:20:55 UTC
I just did the same patch as Martin and wanted to file it.

And btw, I agree with Martin: while None is not strictly an atom,
it is still a special value of the same type.

I vote for special casing None and GDK_NONE in the conversion functions.
Comment 8 Björn Lindqvist 2009-05-30 23:35:49 UTC
Also agree. Another place where GDK_NONE (NULL actually) is purposefully sent as an argument is gdk_wmspec_change_state. 
Comment 9 Matthias Clasen 2009-06-06 02:39:32 UTC
Ok ok
Comment 10 Eugen Dedu 2009-06-13 22:05:41 UTC
I see that this was fixed on master only.  Couldn't this be fixed in branch too?
Comment 11 Cody Russell 2009-08-03 11:40:56 UTC
I just merged into gtk-2-16 branch.
Comment 12 Hussam Al-Tayeb 2009-08-20 03:31:22 UTC
I still get:
error: ** Gdk **: gdk_x11_xatom_to_atom_for_display: assertion `xatom != None' failed
in firefox 3.5.2 using gtk+ 2.17.8 
Is it related to this bug?
Best I could find is this redhat report https://bugzilla.redhat.com/show_bug.cgi?id=507769
Comment 13 Hussam Al-Tayeb 2009-08-20 03:32:15 UTC
I still get:
error: ** Gdk **: gdk_x11_xatom_to_atom_for_display: assertion `xatom != None' failed
in firefox 3.5.2 using gtk+ 2.17.8 
Is it related to this bug?
Best I could find is this redhat report https://bugzilla.redhat.com/show_bug.cgi?id=507769
Comment 14 Hussam Al-Tayeb 2009-08-20 03:33:33 UTC
Oops. Sorry for posting twice.
Comment 15 Xavier 2009-08-31 17:58:38 UTC
In which gtk2 version is this problem fixed?

I am using 2.16.5 and I have no problems with the dnd-none-test program

But firefox 3.5.2 shows the same error and crashes all the time.
Comment 16 Matthias Clasen 2009-08-31 18:24:39 UTC
The presence or absence of the error message has no relation to crashes.
The fix is in 2.16.6
Comment 17 Xavier 2009-08-31 18:35:03 UTC
I guess it's just firefox crash reports which are very confusing then.
The reports have a section like "errors happening before the crash", and the only errors there are :
error: ** Gdk **: gdk_x11_xatom_to_atom_for_display: assertion `xatom != None'

sorry for the noise.