Re: [patch] color picker crash

[Owen Taylor <otaylor redhat com>]

Erwann Chenede <Erwann Chenede Sun COM> writes:

> Hi,
> 
> I just found a bug in color picker (grab release problem).
> 
> Here is a patch to fix it.
> 
> Index: gdk/x11/gdkimage-x11.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk/x11/gdkimage-x11.c,v
> retrieving revision 1.43
> diff -u -r1.43 gdkimage-x11.c
> --- gdk/x11/gdkimage-x11.c	25 Apr 2002 22:29:12 -0000	1.43
> +++ gdk/x11/gdkimage-x11.c	29 Apr 2002 16:41:07 -0000
> @@ -652,7 +652,7 @@
>    if (have_grab)
>      {				
>        gdk_x11_display_ungrab (gdk_drawable_get_display (drawable));
> -      XFlush (GDK_DRAWABLE_XDISPLAY (drawable));
> +      XFlush (gdk_x11_drawable_get_xdisplay (drawable));
>        have_grab = FALSE;
>      }
> 
> The problem is that GDK_DRAWABLE_XDISPLAY expect a GdkDrawable as oppose
> to gdk_x11_drawable_get_xdisplay which can deal with GdkDrawable and
> GdkDrawableImplX11.
> So I guess an other patch would be the change GDK_DRAWABLE_XDISPLAY
> itself to be gdk_x11_drawable_get_xdisplay.
> 
> What is best ?

It's not clear to me why gdk_x11_drawable_get_xdisplay() can
handle an "impl" object .... I think it's best to say impls
are impls, and wrapper drawable are wrapper drawables and
they aren't interchangable.

gdk_x11_drawable_get_xdisplay() is essentially a public function
and the impl objects are never revealed publically.

There are all sorts of different ways the display is referneced
in that function.

    XFlush (GDK_DRAWABLE_DISPLAY (drawable)); [ seriously messed up... ]

    Display *xdisplay = GDK_SCREEN_XDISPLAY (impl->screen); 
      XTranslateCoordinates (GDK_SCREEN_XDISPLAY (impl->screen),

      XFlush (GDK_DRAWABLE_XDISPLAY (drawable));

What I think is right is to add, after:

   impl = GDK_DRAWABLE_IMPL_X11 (drawable);

   display = gdk_screen_get_display (impl->screen);
   xdisplay = GDK_DISPLAY_XDISPLAY (display)

And use those variables throughout the function.

Regards,
                                        Owen