Makes ctrl-shift-o show the bookmark manager on linux and windows.

Makes ctrl-shift-o show the bookmark manager on linux and windows.

BUG=48749
TEST=open chrome and make sure ctrl-shift-o shows the bookmark manager.

R=sky@chromium.org,pkasting@chromium.org,evan@chromium.org

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

[tfarina, 2011-07-12 18:33:34]

This is the shortcut that Firefox uses. Any objections?

[sky, 2011-07-12 19:32:48]

Did you run this by Glen and other ui folk?

  -Scott

On Tue, Jul 12, 2011 at 11:33 AM,  <tfarina@chromium.org> wrote:
> Reviewers: sky, Evan Martin,
>
> Message:
> This is the shortcut that Firefox uses. Any objections?
>
> Description:
> gtk: Makes ctrl-shift-o show the bookmark manager on linux.
>
> BUG=48749
> TEST=open chrome, and makes sure that shortcut opens the bookmark manager.
>
> R=sky@chromium.org,evan@chromium.org
>
>
> Please review this at http://codereview.chromium.org/7344014/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/ui/gtk/accelerators_gtk.cc
>
>
> Index: chrome/browser/ui/gtk/accelerators_gtk.cc
> diff --git a/chrome/browser/ui/gtk/accelerators_gtk.cc
> b/chrome/browser/ui/gtk/accelerators_gtk.cc
> index
>
f80b994e7c9ccb49cc1efef6b1bd153e9faa2505..aa08183ff01f589e4eefd7ff0773f73f291aff6b
> 100644
> --- a/chrome/browser/ui/gtk/accelerators_gtk.cc
> +++ b/chrome/browser/ui/gtk/accelerators_gtk.cc
> @@ -173,6 +173,8 @@ const struct AcceleratorMapping {
>   { GDK_s, IDC_SAVE_PAGE, GDK_CONTROL_MASK },
>   { GDK_e, IDC_SHOW_APP_MENU, GDK_MOD1_MASK },
>   { GDK_f, IDC_SHOW_APP_MENU, GDK_MOD1_MASK },
> +  { GDK_o, IDC_SHOW_BOOKMARK_MANAGER,
> +    GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) },
>  };
>
>  }  // namespace
>
>
>

[Peter Kasting, 2011-07-12 19:47:47]

This shortcut seems OK to me in principle.  What about Windows and Mac?  What
does Firefox do on those platforms?

[tfarina, 2011-07-12 23:44:22]

On 2011/07/12 19:32:48, sky wrote:
> Did you run this by Glen and other ui folk?
> 
No, not yet.

Would you ask Glen offline? I can copy him on this patch though.

[tfarina, 2011-07-12 23:46:12]

On 2011/07/12 19:47:47, Peter Kasting wrote:
> This shortcut seems OK to me in principle.  What about Windows and Mac?
Currently, Mac uses command-alt-b. Windows, I was going to use ctrl-shift-o for
Windows too.

>  What does Firefox do on those platforms?
On Linux, ctrl-shift-o is what Firefox uses.

[Peter Kasting, 2011-07-13 00:07:20]

On 2011/07/12 23:44:22, tfarina wrote:
> Would you ask Glen offline? I can copy him on this patch though.

I'm Glen's appointed representative.

On 2011/07/12 23:46:12, tfarina wrote:
> On 2011/07/12 19:47:47, Peter Kasting wrote:
> >  What does Firefox do on those platforms?
> On Linux, ctrl-shift-o is what Firefox uses.

Yes, you said that already.  I was wondering what Firefox does on Mac and
Windows.  If we're being consistent on one platform, perhaps we should be
consistent on the others.

[tfarina, 2011-07-13 00:23:05]

On 2011/07/13 00:07:20, Peter Kasting wrote:
> Yes, you said that already.  I was wondering what Firefox does on Mac and
> Windows. 
OK, asking on irc:

FF/Mac uses cmd-shift-b
FF/Win uses ctrl+shift+b

Chrome/Mac uses ctrl-opt-b which matches Safari.
cmd-shift-b toggles bookmarks bar in Chrome/Mac.

> If we're being consistent on one platform, perhaps we should be
> consistent on the others.

[Peter Kasting, 2011-07-13 00:35:38]

On 2011/07/13 00:23:05, tfarina wrote:
> On 2011/07/13 00:07:20, Peter Kasting wrote:
> > Yes, you said that already.  I was wondering what Firefox does on Mac and
> > Windows. 
> OK, asking on irc:
> 
> FF/Mac uses cmd-shift-b
> FF/Win uses ctrl+shift+b

Fie.  I hate everything.

Let's go ahead and add ctrl-shift-o on Windows.

[tfarina, 2011-07-13 01:14:27]

On 2011/07/13 00:35:38, Peter Kasting wrote:
> On 2011/07/13 00:23:05, tfarina wrote:
> > On 2011/07/13 00:07:20, Peter Kasting wrote:
> > > Yes, you said that already.  I was wondering what Firefox does on Mac and
> > > Windows. 
> > OK, asking on irc:
> > 
> > FF/Mac uses cmd-shift-b
> > FF/Win uses ctrl+shift+b
> 
> Fie.  I hate everything.
> 
> Let's go ahead and add ctrl-shift-o on Windows.

Done. Please, could you take another look?

[Peter Kasting, 2011-07-13 01:17:57]

LGTM