Skip to content

Commit

Permalink
ash: Allow web pages to use the Ctrl-M minimize shortcut
Browse files Browse the repository at this point in the history
The active window is minimized only if the active web page or app does not
handle the key first.

BUG=284522
TEST=manual, ash_unittests AcceleratorCommandsTest.*

Review URL: https://chromiumcodereview.appspot.com/23611005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221294 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jamescook@chromium.org committed Sep 4, 2013
1 parent 9e2cf27 commit fa94127
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 19 deletions.
36 changes: 36 additions & 0 deletions ash/accelerators/accelerator_commands.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/accelerators/accelerator_commands.h"

#include "ash/shell.h"
#include "ash/shell_delegate.h"
#include "ash/wm/window_cycle_controller.h"
#include "ash/wm/window_util.h"

namespace ash {
namespace accelerators {

bool ToggleMinimized() {
aura::Window* window = wm::GetActiveWindow();
// Attempt to restore the window that would be cycled through next from
// the launcher when there is no active window.
if (!window) {
ash::Shell::GetInstance()->window_cycle_controller()->
HandleCycleWindow(WindowCycleController::FORWARD, false);
return true;
}
// Disable the shortcut for minimizing full screen window due to
// crbug.com/131709, which is a crashing issue related to minimizing
// full screen pepper window.
if (wm::IsWindowFullscreen(window) || !wm::CanMinimizeWindow(window))
return false;
ash::Shell::GetInstance()->delegate()->RecordUserMetricsAction(
ash::UMA_MINIMIZE_PER_KEY);
wm::MinimizeWindow(window);
return true;
}

} // namespace accelerators
} // namespace ash
23 changes: 23 additions & 0 deletions ash/accelerators/accelerator_commands.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef ASH_ACCELERATORS_ACCELERATOR_COMMANDS_H_
#define ASH_ACCELERATORS_ACCELERATOR_COMMANDS_H_

#include "ash/ash_export.h"

// This file contains implementations of commands that are bound to keyboard
// shortcuts in Ash or in the embedding application (e.g. Chrome).
namespace ash {
namespace accelerators {

// Minimizes the active window, if present. If no windows are active, restores
// the first unminimized window. Returns true if a window was minimized or
// restored.
ASH_EXPORT bool ToggleMinimized();

} // namespace accelerators
} // namespace ash

#endif // ASH_ACCELERATORS_ACCELERATOR_COMMANDS_H_
31 changes: 31 additions & 0 deletions ash/accelerators/accelerator_commands_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ash/accelerators/accelerator_commands.h"

#include "ash/test/ash_test_base.h"
#include "ash/wm/window_util.h"
#include "ui/aura/window.h"

namespace ash {
namespace accelerators {

typedef test::AshTestBase AcceleratorCommandsTest;

TEST_F(AcceleratorCommandsTest, ToggleMinimized) {
scoped_ptr<aura::Window> window(
CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
wm::ActivateWindow(window.get());

ToggleMinimized();
EXPECT_TRUE(wm::IsWindowMinimized(window.get()));
EXPECT_FALSE(wm::IsWindowNormal(window.get()));

ToggleMinimized();
EXPECT_FALSE(wm::IsWindowMinimized(window.get()));
EXPECT_TRUE(wm::IsWindowNormal(window.get()));
}

} // namespace accelerators
} // namespace ash
20 changes: 3 additions & 17 deletions ash/accelerators/accelerator_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <iostream>
#include <string>

#include "ash/accelerators/accelerator_commands.h"
#include "ash/accelerators/accelerator_table.h"
#include "ash/ash_switches.h"
#include "ash/caps_lock_delegate.h"
Expand Down Expand Up @@ -843,23 +844,8 @@ bool AcceleratorController::PerformAction(int action,
internal::SnapSizer::RIGHT_EDGE);
return true;
}
case WINDOW_MINIMIZE: {
aura::Window* window = wm::GetActiveWindow();
// Attempt to restore the window that would be cycled through next from
// the launcher when there is no active window.
if (!window)
return HandleCycleWindowMRU(WindowCycleController::FORWARD, false);
// Disable the shortcut for minimizing full screen window due to
// crbug.com/131709, which is a crashing issue related to minimizing
// full screen pepper window.
if (!wm::IsWindowFullscreen(window) && wm::CanMinimizeWindow(window)) {
ash::Shell::GetInstance()->delegate()->RecordUserMetricsAction(
ash::UMA_MINIMIZE_PER_KEY);
wm::MinimizeWindow(window);
return true;
}
break;
}
case WINDOW_MINIMIZE:
return accelerators::ToggleMinimized();
case TOGGLE_FULLSCREEN: {
if (key_code == ui::VKEY_MEDIA_LAUNCH_APP2) {
shell->delegate()->RecordUserMetricsAction(
Expand Down
2 changes: 0 additions & 2 deletions ash/accelerators/accelerator_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ const AcceleratorData kAcceleratorData[] = {
{ true, ui::VKEY_OEM_4, ui::EF_ALT_DOWN, WINDOW_SNAP_LEFT },
{ true, ui::VKEY_OEM_6, ui::EF_ALT_DOWN, WINDOW_SNAP_RIGHT },
{ true, ui::VKEY_OEM_MINUS, ui::EF_ALT_DOWN, WINDOW_MINIMIZE },
// Convenience for users switching from Mac OS.
{ true, ui::VKEY_M, ui::EF_CONTROL_DOWN, WINDOW_MINIMIZE },
{ true, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, TOGGLE_MAXIMIZED },
{ true, ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
WINDOW_POSITION_CENTER },
Expand Down
26 changes: 26 additions & 0 deletions ash/accelerators/accelerator_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,32 @@

namespace ash {

// There are four classes of accelerators in Ash:
//
// Ash (OS) reserved:
// * Neither packaged apps nor web pages can cancel.
// * For example, Alt-Tab window cycling.
// * See kReservedActions below.
//
// Ash (OS) non-reserved:
// * Packaged apps can cancel but web pages cannot.
// * For example, volume up and down.
// * See kActionsAllowedInAppMode below.
//
// Browser reserved:
// * Packaged apps can cancel but web pages cannot.
// * For example, browser back and forward from first-row function keys.
// * See IsReservedCommandOrKey() in
// chrome/browser/ui/browser_command_controller.cc.
//
// Browser non-reserved:
// * Both packaged apps and web pages can cancel.
// * For example, selecting tabs by number with Ctrl-1 to Ctrl-9.
// * See kAcceleratorMap in chrome/browser/ui/views/accelerator_table.cc.
//
// In particular, there is not an accelerator processing pass for Ash after
// the browser gets the accelerator. See crbug.com/285308 for details.
//
// Please put if/def sections at the end of the bare section and keep the list
// within each section in alphabetical order.
enum AcceleratorAction {
Expand Down
5 changes: 5 additions & 0 deletions ash/ash.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
],
'sources': [
# All .cc, .h under ash, except unittests
'accelerators/accelerator_commands.cc',
'accelerators/accelerator_commands.h',
'accelerators/accelerator_controller.cc',
'accelerators/accelerator_controller.h',
'accelerators/accelerator_dispatcher.cc',
Expand Down Expand Up @@ -535,6 +537,8 @@
'conditions': [
['OS=="mac"', {
'sources/': [
['exclude', 'accelerators/accelerator_commands.cc'],
['exclude', 'accelerators/accelerator_commands.h'],
['exclude', 'accelerators/accelerator_controller.cc'],
['exclude', 'accelerators/accelerator_controller.h'],
['exclude', 'accelerators/accelerator_dispatcher.cc'],
Expand Down Expand Up @@ -679,6 +683,7 @@
'../ui/compositor/test/layer_animator_test_controller.h',
'../ui/views/test/test_views_delegate.cc',
'../ui/views/test/test_views_delegate.h',
'accelerators/accelerator_commands_unittest.cc',
'accelerators/accelerator_controller_unittest.cc',
'accelerators/accelerator_filter_unittest.cc',
'accelerators/accelerator_table_unittest.cc',
Expand Down
9 changes: 9 additions & 0 deletions chrome/browser/ui/browser_command_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#endif

#if defined(USE_ASH)
#include "ash/accelerators/accelerator_commands.h"
#include "chrome/browser/ui/ash/ash_util.h"
#endif

Expand Down Expand Up @@ -444,6 +445,11 @@ void BrowserCommandController::ExecuteCommandWithDisposition(
case IDC_TOGGLE_ASH_DESKTOP:
chrome::ToggleAshDesktop();
break;
case IDC_MINIMIZE_WINDOW:
ash::accelerators::ToggleMinimized();
break;
// If Ash needs many more commands here we should implement a general
// mechanism to pass accelerators back into Ash. http://crbug.com/285308
#endif

#if defined(OS_WIN)
Expand Down Expand Up @@ -836,6 +842,9 @@ void BrowserCommandController::InitCommandState() {
chrome::HOST_DESKTOP_TYPE_NATIVE != chrome::HOST_DESKTOP_TYPE_ASH)
command_updater_.UpdateCommandEnabled(IDC_TOGGLE_ASH_DESKTOP, true);
#endif
#if defined(USE_ASH)
command_updater_.UpdateCommandEnabled(IDC_MINIMIZE_WINDOW, true);
#endif

// Page-related commands
command_updater_.UpdateCommandEnabled(IDC_EMAIL_PAGE_LOCATION, true);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/accelerator_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ const AcceleratorMapping kAcceleratorMap[] = {
{ ui::VKEY_BROWSER_REFRESH, ui::EF_SHIFT_DOWN, IDC_RELOAD_IGNORING_CACHE },
{ ui::VKEY_BROWSER_FAVORITES, ui::EF_NONE, IDC_SHOW_BOOKMARK_MANAGER },
{ ui::VKEY_BROWSER_STOP, ui::EF_NONE, IDC_STOP },
// Not implemented inside Ash to allow web pages to capture the key.
{ ui::VKEY_M, ui::EF_CONTROL_DOWN, IDC_MINIMIZE_WINDOW },
#else // OS_CHROMEOS
{ ui::VKEY_DELETE, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
IDC_CLEAR_BROWSING_DATA },
Expand Down

0 comments on commit fa94127

Please sign in to comment.