Re: [evolution-patches] Re: FWD: Seeking review for bug 45631, "activate popup menu in mail composer's to/cc"

[Not Zed <notzed ximian com>]

I know there's been other patches like this elsewhere ... but i was
wondering, is this actually the right way to do it?

Shouldn't it just be hooking onto the popup_menu event, which is already
bound to F10/can be bound to S-F10?

I'm not sure, i'm just wondering if it should be a discussion point ...


On Wed, 2003-08-06 at 11:39, Mike Kestner wrote:
> On Wed, 2003-08-06 at 03:10, Gilbert Fang wrote:
> 
> >   This patch will enhance the keyboard navigation of evolution which is
> > one part of our group's tasks.
> 
> Comments in-line.  Thanks for the patch.  Please resubmit after rework.
> 
>         case GDK_KEY_PRESS: /* Fall Through */
>         case GDK_KEY_RELEASE:
> +
> +               /* Handle S-F10 key binding here. */
> +
> +               if (event->type == GDK_KEY_PRESS 
> 
> You should add this code to the KEY_PRESS case instead and fall through
> to the KEY_RELEASE case if the popup doesn't need to be displayed.
> 
> +                   && event->key.keyval == GDK_F10 
> +                   && (event->key.state&GDK_SHIFT_MASK)){
> +
> +                       /* Simulate a GdkEventButton here, so that we
> can call e_text_do_popup directly */
> +
> +                       GdkEventButton *button = g_new0 (GdkEventButton,
> 1);
> +                       button->type = GDK_BUTTON_PRESS;
> +                       button->time = GDK_CURRENT_TIME;
> +                       button->button = 0;
> 
> You should probably use gdk_event_new here instead of just g_new0. 
> Also, you need to call gdk_event_free on the event when you are done
> with it.  The patch currently leaks the button struct.