Implement accelerators in the gtk port.

Implement accelerators in the gtk port.

- Adds accelerators to all the menu items that had them on windows.
- Adds accelerators to back (Alt+left), forward (Alt+right), reload
  (Ctr+R) and focus the URL box (Ctr+L).

[Evan Martin, 2009-02-25 02:30:38]

http://codereview.chromium.org/28105/diff/1011/1013
File chrome/browser/gtk/browser_toolbar_view_gtk.h (right):

http://codereview.chromium.org/28105/diff/1011/1013#newcode74
Line 74: // equivalent to a reload click).
Accelerators should emit commands (which buttons also do).  E.g., ctrl+R should
call ExecuteCommand(IDC_RELOAD).  I think the accelerators belong on the window,
not the toolbar buttons.

(E.g. imagine the toolbar is hidden in full-screen mode -- we still want ctl-r
to reload the page.)

http://codereview.chromium.org/28105/diff/1011/1018
File chrome/browser/gtk/standard_menus.h (right):

http://codereview.chromium.org/28105/diff/1011/1018#newcode37
Line 37: // A gdk keysyms that is used to activate this item from outside the
menu.
typo: keysym

http://codereview.chromium.org/28105/diff/1011/1018#newcode41
Line 41: unsigned int  accel_modifiers;
extra space

http://codereview.chromium.org/28105/diff/1011/1019
File chrome/browser/tab_contents/web_contents_view_gtk.cc (right):

http://codereview.chromium.org/28105/diff/1011/1019#newcode137
Line 137: gtk_accel_groups_activate(G_OBJECT(vbox_->window),
this is super-weird.  vbox_ shouldn't even have a window, i think?  and it's not
a gobject even if it did i believe.

http://codereview.chromium.org/28105/diff/1011/1020
File webkit/glue/webinputevent.h (right):

http://codereview.chromium.org/28105/diff/1011/1020#newcode153
Line 153: guint gdk_keyval;
comments on these?

avi is refactoring all of this so expect to conflict when he lands :\

[Elliot Glaysher, 2009-02-25 17:49:55]

http://codereview.chromium.org/28105/diff/1011/1013
File chrome/browser/gtk/browser_toolbar_view_gtk.h (right):

http://codereview.chromium.org/28105/diff/1011/1013#newcode74
Line 74: // equivalent to a reload click).
On 2009/02/25 02:30:39, Evan Martin wrote:
> Accelerators should emit commands (which buttons also do).  E.g., ctrl+R
should
> call ExecuteCommand(IDC_RELOAD).  I think the accelerators belong on the
window,
> not the toolbar buttons.
> 
> (E.g. imagine the toolbar is hidden in full-screen mode -- we still want ctl-r
> to reload the page.)

Do you mean just the button accelerators, or the menu ones as well?

[Elliot Glaysher, 2009-02-25 22:08:41]

http://codereview.chromium.org/28105/diff/1011/1018
File chrome/browser/gtk/standard_menus.h (right):

http://codereview.chromium.org/28105/diff/1011/1018#newcode37
Line 37: // A gdk keysyms that is used to activate this item from outside the
menu.
On 2009/02/25 02:30:39, Evan Martin wrote:
> typo: keysym

Done.

http://codereview.chromium.org/28105/diff/1011/1018#newcode41
Line 41: unsigned int  accel_modifiers;
On 2009/02/25 02:30:39, Evan Martin wrote:
> extra space

Done.

http://codereview.chromium.org/28105/diff/1011/1019
File chrome/browser/tab_contents/web_contents_view_gtk.cc (right):

http://codereview.chromium.org/28105/diff/1011/1019#newcode137
Line 137: gtk_accel_groups_activate(G_OBJECT(vbox_->window),
Nope!

vbox_ is a GtkWidget and all GtkWidgets keep a pointer to the GdkWindow that
they are a part of.

http://library.gnome.org/devel/gtk/unstable/GtkWidget.html#GtkWidget--window

GdkWindows are Gobjects.

http://library.gnome.org/devel/gdk/unstable/gdk-Windows.html

And finally, we need to cast it to a GObject because that's what
gtk_accel_groups wants:

http://library.gnome.org/devel/gtk/2.11/gtk-Keyboard-Accelerators.html#gtk-ac...

http://codereview.chromium.org/28105/diff/1011/1020
File webkit/glue/webinputevent.h (right):

http://codereview.chromium.org/28105/diff/1011/1020#newcode153
Line 153: guint gdk_keyval;
On 2009/02/25 02:30:39, Evan Martin wrote:
> comments on these?
> 
> avi is refactoring all of this so expect to conflict when he lands :\

Done.

[Evan Martin, 2009-02-25 22:11:09]

LG

[Avi (use Gerrit), 2009-02-25 22:38:40]

I strongly disagree with having landed this. It introduces the same security
issues that we're trying to get rid of for Windows, and adds more
platform-dependence to a class that we're trying to make platform-independent
(and for which I have a patch ready!).

http://codereview.chromium.org/28105/diff/1028/22
File chrome/browser/tab_contents/web_contents_view_gtk.cc (right):

http://codereview.chromium.org/28105/diff/1028/22#newcode140
Line 140: }
No. Handling random crap that the renderer sends back is a security hole. It's
wrong on Windows (and we're going to fix it) and it's wrong here. We need to
keep a copy of the original event on the browser side somewhere. We cannot rely
on the renderer. Let's fix it everywhere at once.

http://codereview.chromium.org/28105/diff/1028/23
File webkit/glue/webinputevent.h (right):

http://codereview.chromium.org/28105/diff/1028/23#newcode157
Line 157: guint gdk_modifier;
How is this different from the existing modifier info? Can you please reuse
fields?

[Avi (use Gerrit), 2009-02-25 22:46:19]

Can you back this out? It seems like you only add these fields so that you can
send events back through, and we need a generalized solution.

[darin (slow to review), 2009-02-26 04:16:55]

http://codereview.chromium.org/28105/diff/1028/23
File webkit/glue/webinputevent.h (right):

http://codereview.chromium.org/28105/diff/1028/23#newcode157
Line 157: guint gdk_modifier;
yeah, i don't understand this either.  PlatformKeyboardEvent has no equivalent
fields, so this seems plainly wrong.