Re: Redundant code in gtk_propagate_event()

["Padraig O'Briain" <Padraig Obriain Sun COM>]

> To: "Padraig O'Briain" <Padraig Obriain Sun COM>
> Cc: gtk-devel-list gnome org
> Subject: Re: Redundant code in gtk_propagate_event()
> User-Agent: Gnus/5.0807 (Gnus v5.8.7) Emacs/20.7
> MIME-Version: 1.0
> 
> 
> "Padraig O'Briain" <Padraig Obriain Sun COM> writes:
> 
> > While looking at the the code of gtk_propagate_event() I noticed what looks 
like 
> > redundant code.
> > 
> > Ok to commit?
> > 
> > --- gtkmain.c   2001/09/21 19:58:35     1.176
> > +++ gtkmain.c   2001/10/04 08:24:42
> > @@ -1724,12 +1724,8 @@ gtk_propagate_event (GtkWidget *widget,
> >           
> >           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);
> > -               }
> > +               if (GTK_WIDGET_IS_SENSITIVE (window))
> > +                 gtk_widget_event (window, event);
> >             }
> >                   
> >           handled_event = TRUE; /* don't send to widget */
> 
> Why do you think this is redundant code? Well, you could
> simplify to:
> 
>    window = gtk_widget_get_toplevel (widget);
>    if (GTK_IS_WINDOW (window))
>      {
>      }

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().


------------- 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