gtk: Fix alt-f/e to bring up the wrench menu.

gtk: Fix alt-f/e to bring up the wrench menu.

The invisible GtkMenuBar eats up alt-f, alt-e and other accelerators for the
top-level items. So make sure these items don't have any accelerators.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91790

[Elliot Glaysher, 2011-07-07 17:15:17]

LGTM as long as this doesn't remove the accelerators from the actual
menu items (I assume that they are dumped in their own accelerator
context?)

On Wed, Jul 6, 2011 at 9:00 PM,  <sadrul@chromium.org> wrote:
> Reviewers: Elliot Glaysher,
>
> Description:
> gtk: Fix alt-f/e to bring up the wrench menu.
>
> The invisible GtkMenuBar eats up alt-f, alt-e and other accelerators for the
> top-level items. So make sure these items don't have any accelerators.
>
> BUG=none
> TEST=none
>
>
> Please review this at http://codereview.chromium.org/7316017/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/ui/gtk/global_menu_bar.cc
>
>
> Index: chrome/browser/ui/gtk/global_menu_bar.cc
> diff --git a/chrome/browser/ui/gtk/global_menu_bar.cc
> b/chrome/browser/ui/gtk/global_menu_bar.cc
> index
>
ccef09d7d7af2dd2d9c665a06045e0e406da3972..c6980ad2ad1011442530013aa31162b6ce3df81f
> 100644
> --- a/chrome/browser/ui/gtk/global_menu_bar.cc
> +++ b/chrome/browser/ui/gtk/global_menu_bar.cc
> @@ -245,7 +245,7 @@ void GlobalMenuBar::BuildGtkMenuFrom(
>   gtk_widget_show(menu);
>
>   GtkWidget* menu_item = gtk_menu_item_new_with_mnemonic(
> -      gfx::ConvertAcceleratorsFromWindowsStyle(
> +      gfx::RemoveWindowsStyleAccelerators(
>           l10n_util::GetStringUTF8(menu_str_id)).c_str());
>
>   // Give the owner a chance to sink the reference before we add it to the
> menu
>
>
>

[sadrul, 2011-07-07 17:22:34]

On 2011/07/07 17:15:17, Elliot Glaysher wrote:
> LGTM as long as this doesn't remove the accelerators from the actual
> menu items (I assume that they are dumped in their own accelerator
> context?)

Yes, the menu entries in the wrench menu, or the invisible GtkMenuBar, continue
to have the accelerators. Only the accelerators for the toplevel items in the
menubar (File, Edit, View, History, Tools, Help) do not get set, but since it
isn't visible anyway, it doesn't cause a problem. I do not, however, know how it
may, if at all, affect Unity.

[sadrul, 2011-07-07 17:44:37]

On 2011/07/07 17:22:34, sadrul wrote:
> On 2011/07/07 17:15:17, Elliot Glaysher wrote:
> > LGTM as long as this doesn't remove the accelerators from the actual
> > menu items (I assume that they are dumped in their own accelerator
> > context?)
> 
> Yes, the menu entries in the wrench menu, or the invisible GtkMenuBar,
continue
> to have the accelerators. Only the accelerators for the toplevel items in the
> menubar (File, Edit, View, History, Tools, Help) do not get set, but since it
> isn't visible anyway, it doesn't cause a problem. I do not, however, know how
it
> may, if at all, affect Unity.

I don't have Unity available. Do you want to try it out on Unity first, or
should I land it?

[Elliot Glaysher, 2011-07-07 17:46:17]

Land it.

On Thu, Jul 7, 2011 at 10:44 AM,  <sadrul@chromium.org> wrote:
> On 2011/07/07 17:22:34, sadrul wrote:
>>
>> On 2011/07/07 17:15:17, Elliot Glaysher wrote:
>> > LGTM as long as this doesn't remove the accelerators from the actual
>> > menu items (I assume that they are dumped in their own accelerator
>> > context?)
>
>> Yes, the menu entries in the wrench menu, or the invisible GtkMenuBar,
>
> continue
>>
>> to have the accelerators. Only the accelerators for the toplevel items in
>> the
>> menubar (File, Edit, View, History, Tools, Help) do not get set, but since
>> it
>> isn't visible anyway, it doesn't cause a problem. I do not, however, know
>> how
>
> it
>>
>> may, if at all, affect Unity.
>
> I don't have Unity available. Do you want to try it out on Unity first, or
> should I land it?
>
> http://codereview.chromium.org/7316017/
>