Re: popup menu keybinding

[Havoc Pennington <hp redhat com>]

Tim Janik <timj gtk org> writes: 
> 
> i just now see that you added *popup_menu fields to textview and gtkentry.
> user-provided popupmenus on entries may not be so common but for textviews
> surely are, but users can't provide or extend those builtin popup
> menus.

"Enable extending of popup menus" is a bug already open in bugzilla,
which I'm working on now. But that's a separate issue introduced by
adding these menus in the first place - which is not what my patch
does. ;-) This patch simply adds keybindings to pop the menu up, the
menus are already in GTK for a couple months or more.

The way I'm planning to do extension of the menus is with a signal,
which I haven't thought of a good name for yet:
 void (* about_to_popup_menu) (GtkWidget *widget, /* or entry, textview */
                               GtkMenu   *menu)

So the signal arg is simply the menu about to be popped up, such that
the app can add custom entries. i.e. we emit this signal just before
gtk_menu_popup().

> and what should apps do that mean to have icons for menu items in
> every menu?

We should have icons for these menu items, using the stock system, and
also key shortcuts. But that is an orthogonal issue to this patch. ;-)

Apps that really, really want to do their own thing have two options:
 - just override the popup_menu binding and button_press_event,
   and pop up their own menu
 - do assorted hacks in about_to_popup_menu handler to modify
   the menu, e.g. you could replace all the items if you wanted

Now, long-term we can do this a bit more cleanly, with a full-blown
menu API, using e.g. the menu merge code as with Bonobo controls. But
in the short term we have several requirements:

 - we must have a way to cut/copy/paste the CLIPBOARD selection
   on entry/textview
 - we need a way to switch input methods
 - both of the above must be keyboard navigable

So that means we need the popup menus in GTK 2, and the simple
about_to_popup_menu signal hook so you can add menu items without 
breaking the built-in items.

This simple hook won't be a long-term liability, it isn't hurting
anything by existing even once we have a full menu API to use.

> creating a new menu for every entry that button3 is clicked in is an
> extreme memory waste btw, users that actually do use those popupmenus,
> and do so in a bunch of entries supplied by an application will create
> a big stack of unused widgets. at most you should have one such menu per
> widget class (e.g. GtkEntry and GtkTextView).

Again, this is an issue that already existed, not one introduced by my
patch, I agree it would be nice to fix.

> the popupmenu behaviour of not letting one select the first item
> with drag selections is also quite irritating.

I think you are ranting on popup menus in general - this is a simple
patch to pop them up with a keybinding. ;-)

> you need to disable accelerator settings for those menus, currently the
> users can set accelerators on them that aren't negotiated or saved which is
> highly confusing.

Sure. (But again, a preexisting issue, not an issue with this patch. ;-)

> to finally come to your <Shft>+F10 key binding,

Finally! ;-)

>  beast for instance is an
> application that makes use of F* and <Shft>+F*, with most window managers
> taking <Ctrl>+F* and <Alt>+F* away, there's not much left for an application
> if gtk now starts to clutter <Shft>+F* as well.
> why i could have understood (not apprechiated though) an F10 binding that
> activates the menu bar in a window which is quite common in *some* GUIs,
> <Shft>+F10 certainly isn't.

So what binding do you suggest instead? 

Rationale for this one: Calum's keybinding document indicates this is
standard on Windows and Java, I'm sure it's also used by KDE, so this
is what users will expect.

It's customizable via gtkrc, and Shift+F10 seems like a reasonable
solution to me for that reason. If it annoys someone to death they can
get rid of it.

Anyway, in the case where a keybinding is already used by the other
platforms in that table, I think we should use the same key. We can
ship "keybinding theme" gtkrc files that let people get different
behavior if they prefer. This will encourage said people to fix all
remaining non-use of GtkBindingSet in GTK... ;-)

> also i think it's a pretty bad idea to introduce any keybinding on GtkWidget
> class, since this will affect *all* widgets out there, including third party
> ones.

Not silently - they have to explicitly decide to implement the
popup_menu signal. If they don't implement it, the keybinding does
nothing.

The only problem I see is that Shift+F10 gets silently "consumed" if
you were using the default key_press_event handler, this is very easy
to work around by overriding key_press_event.

> dictating <Shft>+F10 for them for menu popups and further dictating the
> signature those widgets may use to popup menus is too much policy for
> gtk+ and is prone to lead to extreme confusion.

The confusion is experienced by users if other widgets randomly have
different bindings for "popup the popup." If you have a widget that
has a popup menu and use a nonstandard binding for that popup, you are
doing something silly, and while GTK makes it possible (you can do
whatever you want in your key_press_event handler), we shouldn't
particularly encourage it.

> if at all, you should add those bindings only to those widgets that actually
> have popupmenus, that curently being gtktextview and gtkentry.
> 

I'm willing to be convinced of that, the reason I put it in GtkWidget
is that if I want the popup shortcut to be say Ctrl+Shift+p instead of
Shift+F10, I would like to customize that with one line in my gtkrc,
and have it affect all widgets including third party widgets.

Is there a way to achieve that other than putting the signal in
GtkWidget?

I think it makes sense to add a popup to any widget, so I think it
makes sense to have this signal in GtkWidget.

Anyhow, I do somewhat see the concern about having bindings in
GtkWidget, but I don't see how to meet the user need to configure
the popup menu keybinding globally for all widgets at once without
doing that. Ideas? 

Havoc