Re: #55310: somewhat more correct patch

[Kristian Rietveld <kristian planet nl>]

On 21 Sep 2001, Owen Taylor wrote:
>
> > @@ -1124,6 +1126,8 @@
> >    gtk_style_set_background (widget->style, menu->view_window, GTK_STATE_NORMAL);
> >    gtk_style_set_background (widget->style, widget->window, GTK_STATE_NORMAL);
> >
> > +  gtk_menu_scroll_item_visible (GTK_MENU_SHELL (widget), NULL);
>
> Shouldn't this be
>
>  gtk_menu_scroll_item_visible (menu_shell, menu_shell->active_menu_item)
>
> ?

Yeah, think so.

>
> > +static void
> > +gtk_menu_select_item (GtkMenuShell  *menu_shell,
> > +		      GtkWidget     *menu_item)
> > +{
> > +  GtkMenu *menu;
> > +
> > +  menu = GTK_MENU (menu_shell);
> > +
> > +  g_return_if_fail (GTK_IS_MENU (menu_shell));
>
> For a public function, this needs to be:
>
>   GtkMenu *menu;
>
>   g_return_if_fail (GTK_IS_MENU (menu_shell));
>
>   menu = GTK_MENU (menu_shell);

Oh, of course ... made a mistake with pasting I guess :)

>
> Since, you shouldn't try to use menu as a menu_shell before
> the check. However, we generally avoid putting such checks
> in virtual-function-implementations now, and just write:
>
>   GtkMenu *menu = GTK_MENU (menu_shell);

Ok.

>
> Other than that, looks fine to commit.

Okay.

>
> Regards,
>                                         Owen
>
>