Re: Redundant code in gtk_propagate_event()



"Padraig O'Briain" <Padraig Obriain Sun COM> writes:

> The code is gtk_prtopagate_event() currently is as below. What I was proposing 
> was to remove the second call to gtk_widget_get_toplevel().

Oh, misread the patch. The second gtk_widget_get_toplevel()
is not at all redundant. It's there because 

 a) we don't keep a reference on the 'window' widget so
    it might have been destroyed out from under us
    in the call to gtk_widget_event()

 b) 'window' might no longer be the toplevel widget for the widget 
    even if it still exists.

Regards,
                                        Owen

> ------------- Begin Included Message -------------
> 
>       GtkWidget *window;
> 
>       window = gtk_widget_get_toplevel (widget);
>       if (window && GTK_IS_WINDOW (window))
>         {
>           /* If there is a grab within the window, give the grab widget
>            * a first crack at the key event
>            */
>           if (widget != window && GTK_WIDGET_HAS_GRAB (widget))
>             handled_event = gtk_widget_event (widget, event);
> 
>           if (!handled_event)
>             {
>               window = gtk_widget_get_toplevel (widget);
>               if (window && GTK_IS_WINDOW (window))
>                 {
>                   if (GTK_WIDGET_IS_SENSITIVE (window))
>                     gtk_widget_event (window, event);
>                 }
>             }
> 
>           handled_event = TRUE; /* don't send to widget */
>         }
> 
> ------------- End Included Message -------------
> 
> > 
> > Since get_toplevel() never returns NULL for valid arguments;
> > but the GTK_IS_WINDOW() check is needed; in various 
> > pathological cases you can have widgets that receive events
> > where the toplevel widget is not GtkWindow. Those would
> > mostly be:
> > 
> >  - Someone called gtk_propagate_event() directly.
> >  - A widget was force-realized via gtk_widget_realize()
> >    while anchored.
> > 
> > Both of those are programmer errors .... but still, I think
> > it's worth having this check to get a bit of extra safety.
> > 
> > Regards,
> >                                         Owen




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]