Re: clist/notebook bug? + patch

[Tim Janik <timj gtk org>]

On Sun, 17 Jan 1999, Damon Chaplin wrote:

> Damon Chaplin wrote:
> > 
> > Elliot Turner wrote:
> > >
> > > Hello,
> > >
> > > I noticed some strange behaivor with gtk+-1.1.12 in relation to CLists and
> > > NoteBooks.
> > >
> > > I have a notebook widget with several pages, one of which contains a
> > > GtkCList as it's child.
> > >
> > > I use gtk_signal_connect() to connect a signal handler function to the clist
> > > to handle the signal "select_row".  inside this handler function, I check to
> > > see if the event is a GDK_2BUTTON_PRESS.. if it is, i call
> > > gtk_notebook_set_page() to switch the currently active page of the parent
> > > notebook.
> > >
> > > This results in most of the application becoming unresponsive to any mouse
> > > clicks.  The notebook is still responsive, (it seems like all parent widgets
> > > of the clist which got the GDK_2BUTTON_CLICK are still responsive, but
> > > nothing else) but children widgets inside other notebook pages become
> > > completely unresponsive.
> > 
> > This seems to be the same problem I reported about the file selection dialog.
> > (If I hid the file dialog when a file is selected with a double-click, it
> > froze and I couldn't select any widgets.)
> 
> 
> I've figured this out (eventually!)

thanx for biting the bullet damon.

> The problem is due to the double-click, which results in these signals:
> 
>   button_press_event    (type = GDK_BUTTON_PRESS)
> 
>   button_release_event
> 
>   button_press_event    (type = GDK_2BUTTON_PRESS)
>   button_press_event    (type = GDK_BUTTON_PRESS)
> 
>   button_release_event
> 
> In our code we both used handlers which hide the clist on the
> GDK_2BUTTON_PRESS (either by hiding a dialog or switching notebook pages).
> 
> But the clist still gets the next button_press_event (even though it is now
> unmapped). Now it attempts to grab the pointer and a GTK grab again.
> X won't let it grab the pointer if it is not mapped, but GTK will let it
> grab all GTK events.
> 
> This confuses GtkCList, which thinks it will always have both grabs. It uses
> this test a lot:
>   if (gdk_pointer_is_grabbed () && GTK_WIDGET_HAS_GRAB (clist))
> 
> So it doesn't think it has the grabs, and so never releases the GTK grab.
> 
> 
> Solution
> ========
> 
> A quick solution is for gtk_clist_button_press() to just return if the widget
> is no longer mapped:
> 
> --- gtkclist.c.orig     Sat Jan  9 22:32:57 1999
> +++ gtkclist.c  Sun Jan 17 17:21:37 1999
> @@ -4845,6 +4845,11 @@
>    g_return_val_if_fail (GTK_IS_CLIST (widget), FALSE);
>    g_return_val_if_fail (event != NULL, FALSE);
>  
> +  /* It is possible that something handled a double-click event and the clist
> +     has now been hidden, in which case we don't want to do anything. */
> +  if (!GTK_WIDGET_MAPPED (widget))
> +    return FALSE;
> +
>    clist = GTK_CLIST (widget);
>  
>    button_actions = clist->button_actions[event->button - 1];
> 
> 
> A better approach may be to make sure that only mapped widgets can have GTK
> grabs, and for widgets to check that they are mapped before trying to get a
> grab.

while your patch certainly fixes the reported symptoms, the actuall problem
remains unsolved. gtk grabs actually work completely independant of a widgets
state and that is intentional. if a widget acquires a gtk grab, it has to
take care about handling and releasing the grab accordingly, that's why the
gtkwindow.c code removes the gtk grab for modal windows in its ::hide handler.
gtk will only asure that grabs are not held by destructed widgets.
gdk grabs, or X grabs therefore, can only be held and will only remain for
viewable windows, so they can't be acquired for hidden windows and will be
released automatically upon window hiding, completely unrelated to any gtk
grabs of course.
the clist code doesn't take into account that a call to gdk_pointer_grab() may
fail, while a call to gtk_grab_add() will allways succeed, so code like

  gdk_pointer_grab (clist->clist_window, FALSE, mask, NULL, NULL, event->time);
  gtk_grab_add (widget);
[...]
  if (gdk_pointer_is_grabbed () && GTK_WIDGET_HAS_GRAB (clist))
    {
      gtk_grab_remove (GTK_WIDGET (clist));
      gdk_pointer_ungrab (GDK_CURRENT_TIME);
    }

will simply mess up the grab states if the gdk grab failed, as you mentioned.
the fix to this needs to go into the clist code directly. possible solutions
are:
- enforce the gdk grab (the handlebox code does this), e.g.

  while (gdk_pointer_grab (clist->clist_window,
                           FALSE, mask, NULL, NULL,
                           event->time) != 0)
    /* wait for success */;

- don't add the gtk grab if the pointer grab failed:

if (gdk_pointer_grab (clist->clist_window,
                      FALSE, mask, NULL, NULL,
                      event->time) == 0)
  gtk_grab_add (widget);

- handle both grabs individually:

  gdk_pointer_grab (clist->clist_window, FALSE, mask, NULL, NULL, event->time);
  gtk_grab_add (widget);
[...]
  if (gdk_pointer_is_grabbed ())
    gdk_pointer_ungrab (GDK_CURRENT_TIME);
  if (GTK_WIDGET_HAS_GRAB (clist))
    gtk_grab_remove (GTK_WIDGET (clist));

the correct way depends actually on the specific situations the clist code
(or any grab related code) wants to handle.    

> 
> Damon
> 
> P.S. GtkCTree and GtkList have similar code to GtkCList, so they may need
> fixing
> as well.

yeah, a quik look reveals that all of them implement the above problem in
one way or the other. unfortunately most of the grab checking code has to
be adapted.

---
ciaoTJ