chromium/src
0
Fork 0
Code Activity

Reland "shortcuts: Deprecate Ctrl+Alt+/ for show shortcut viewer"

Browse Source 
This is a reland of commit 3cdd1da998

Fix addressed: Linus ChromiumOS MSan builds are flaky for spokenfeedback
browser_tests. Disable the test for MSan builds only.

Original change's description:
> shortcuts: Deprecate Ctrl+Alt+/ for show shortcut viewer
>
> Shift + Search + s is the new shortcut to open the shortcut app.
> This allows web apps to now consume Ctrl+Alt+/ for their own
> target handlers.
> This is part of the process to deprecate all non-search based system
> accelerators.
>
> This CL also updates the deprecation notification message.
>
> Screenshots:
> screenshot.googleplex.com/VeotYoBEF2BjCDX
> https://screenshot.googleplex.com/6AScLFHqF7CwZ55
>
> Bug: b/216049298, b/288313762
> Change-Id: Ia1225dc8a748348f6a19f24eb7558611840c338f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4602855
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: David Padlipsky <dpad@google.com>
> Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1161493}

Bug: b/216049298, b/288313762
Change-Id: I9a7bee9e54fe99393ef4e6e16a10ed01269ffbf8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4641249
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: David Padlipsky <dpad@google.com>
Cr-Commit-Position: refs/heads/main@{#1162019}
This commit is contained in:
Jimmy
2023-06-24 00:06:25 +00:00
committed by Chromium LUCI CQ
parent 6c0e2216d1
commit 03a88a98ae
17 changed files with 75 additions and 58 deletions

2
ash/accelerators/accelerator_controller_impl.cc

@ -1535,7 +1535,7 @@ AcceleratorControllerImpl::MaybeDeprecatedAcceleratorPressed(
ShowDeprecatedAcceleratorNotification(
deprecated_data->uma_histogram_name,
deprecated_data->notification_message_id,
deprecated_data->old_shortcut_id, deprecated_data->new_shortcut_id);
deprecated_data->new_shortcut_id);
if (!deprecated_data->deprecated_enabled)
return AcceleratorProcessingStatus::STOP;

11
ash/accelerators/accelerator_notifications.cc

@ -66,15 +66,11 @@ void EnsureNoWordBreaks(std::u16string* shortcut_text) {
// Gets the notification message after it formats it in such a way that there
// are no line breaks in the middle of the shortcut texts.
std::u16string GetNotificationText(int message_id,
int old_shortcut_id,
int new_shortcut_id) {
std::u16string old_shortcut = l10n_util::GetStringUTF16(old_shortcut_id);
std::u16string GetNotificationText(int message_id, int new_shortcut_id) {
std::u16string new_shortcut = l10n_util::GetStringUTF16(new_shortcut_id);
EnsureNoWordBreaks(&old_shortcut);
EnsureNoWordBreaks(&new_shortcut);
return l10n_util::GetStringFUTF16(message_id, new_shortcut, old_shortcut);
return l10n_util::GetStringFUTF16(message_id, new_shortcut);
}
std::unique_ptr<Notification> CreateNotification(
@ -185,12 +181,11 @@ const char kHighContrastToggleAccelNotificationId[] =
void ShowDeprecatedAcceleratorNotification(const char* notification_id,
int message_id,
int old_shortcut_id,
int new_shortcut_id) {
const std::u16string title =
l10n_util::GetStringUTF16(IDS_DEPRECATED_SHORTCUT_TITLE);
const std::u16string message =
GetNotificationText(message_id, old_shortcut_id, new_shortcut_id);
GetNotificationText(message_id, new_shortcut_id);
auto on_click_handler = base::MakeRefCounted<HandleNotificationClickDelegate>(
base::BindRepeating([]() {
if (!Shell::Get()->session_controller()->IsUserSessionBlocked())

1
ash/accelerators/accelerator_notifications.h

@ -21,7 +21,6 @@ ASH_EXPORT extern const char kKeyboardShortcutHelpPageUrl[];
ASH_EXPORT void ShowDeprecatedAcceleratorNotification(
const char* notification_id,
int message_id,
int old_shortcut_id,
int new_shortcut_id);
ASH_EXPORT void ShowDockedMagnifierNotification();

21
ash/accelerators/accelerator_table.cc

@ -35,8 +35,16 @@ namespace ash {
// shortcut_viewer_strings.grdp.
const AcceleratorData kDeprecatedAccelerators[] = {
{true, ui::VKEY_ESCAPE, ui::EF_SHIFT_DOWN,
AcceleratorAction::kShowTaskManager}};
AcceleratorAction::kShowTaskManager},
{true, ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
AcceleratorAction::kShowShortcutViewer},
{true, ui::VKEY_OEM_2,
ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN | ui::EF_SHIFT_DOWN,
AcceleratorAction::kShowShortcutViewer}};
// `kShowShortcutViewer` has two accelerators that are deprecated but use the
// same message.
const size_t kNumDeprecatedAcceleratorsDuplicate = 1u;
const size_t kDeprecatedAcceleratorsLength = std::size(kDeprecatedAccelerators);
const DeprecatedAcceleratorData kDeprecatedAcceleratorsData[] = {
@ -44,13 +52,18 @@ const DeprecatedAcceleratorData kDeprecatedAcceleratorsData[] = {
// completely in M94.
{AcceleratorAction::kShowTaskManager,
"Ash.Accelerators.Deprecated.ShowTaskManager",
IDS_DEPRECATED_SHOW_TASK_MANAGER_MSG, IDS_SHORTCUT_TASK_MANAGER_OLD,
IDS_SHORTCUT_TASK_MANAGER_NEW, false}};
IDS_DEPRECATED_SHOW_TASK_MANAGER_MSG, IDS_SHORTCUT_TASK_MANAGER_NEW,
false},
{AcceleratorAction::kShowShortcutViewer,
"Ash.Accelerators.Deprecated.ShowShortcutViewer",
IDS_DEPRECATED_SHOW_SHORTCUT_VIEWER_MSG,
IDS_SHORTCUT_SHOW_SHORTCUT_VIEWER_NEW, false}};
const size_t kDeprecatedAcceleratorsDataLength =
std::size(kDeprecatedAcceleratorsData);
static_assert(kDeprecatedAcceleratorsLength ==
static_assert(kDeprecatedAcceleratorsLength -
kNumDeprecatedAcceleratorsDuplicate ==
kDeprecatedAcceleratorsDataLength,
"Deprecated accelerator tables must be kept in sync");

3
ash/accelerators/accelerator_table.h

@ -66,9 +66,6 @@ struct DeprecatedAcceleratorData {
// them about the deprecation.
int notification_message_id;
// The ID of the localized old deprecated shortcut key.
int old_shortcut_id;
// The ID of the localized new shortcut key.
int new_shortcut_id;

4
ash/accelerators/accelerator_table_unittest.cc

@ -16,10 +16,10 @@ namespace ash {
namespace {
// The number of non-Search-based accelerators.
constexpr int kNonSearchAcceleratorsNum = 111;
constexpr int kNonSearchAcceleratorsNum = 109;
// The hash of non-Search-based accelerators. See HashAcceleratorData().
constexpr char kNonSearchAcceleratorsHash[] =
"84486b92f69cda0ecf8052e3836473b6";
"f59bc0d21b6c243361b02c8cbae92f0e";
struct Cmp {
bool operator()(const AcceleratorData& lhs,

6
ash/accelerators/ash_accelerator_configuration_unittest.cc

@ -186,7 +186,7 @@ TEST_F(AshAcceleratorConfigurationTest, DeprecatedAccelerators) {
const DeprecatedAcceleratorData deprecated_data[] = {
{AcceleratorAction::kShowTaskManager,
/*uma_histogram_name=*/"deprecated.showTaskManager",
/*notification_message_id=*/1, /*old_shortcut_id=*/1,
/*notification_message_id=*/1,
/*new_shortcut_id=*/2, /*deprecated_enabled=*/true},
};
@ -264,7 +264,7 @@ TEST_F(AshAcceleratorConfigurationTest,
const DeprecatedAcceleratorData deprecated_data[] = {
{AcceleratorAction::kShowTaskManager,
/*uma_histogram_name=*/"deprecated.showTaskManager",
/*notification_message_id=*/1, /*old_shortcut_id=*/1,
/*notification_message_id=*/1,
/*new_shortcut_id=*/2, /*deprecated_enabled=*/true},
};
@ -960,7 +960,7 @@ TEST_F(AshAcceleratorConfigurationTest, AddAcceleratorDeprecatedConflict) {
const DeprecatedAcceleratorData deprecated_data[] = {
{AcceleratorAction::kShowTaskManager,
/*uma_histogram_name=*/"deprecated.showTaskManager",
/*notification_message_id=*/1, /*old_shortcut_id=*/1,
/*notification_message_id=*/1,
/*new_shortcut_id=*/2, /*deprecated_enabled=*/true},
};

13
ash/ash_strings.grd

@ -3476,7 +3476,7 @@ Connect your device to power.
Shift+Esc
</message>
<message name="IDS_SHORTCUT_TASK_MANAGER_NEW" desc="the new shortcut to open the task manager.">
Search+Esc
search+esc
</message>
<message name="IDS_SHORTCUT_IME_BUBBLE_OLD" desc="the old deprecated shortcut to show the input options menu bubble in the shelf.">
Alt+Shift+K
@ -3484,16 +3484,23 @@ Connect your device to power.
<message name="IDS_SHORTCUT_IME_BUBBLE_NEW" desc="the new shortcut to show the input options menu bubble in the shelf.">
Search+Shift+K
</message>
<message name="IDS_SHORTCUT_SHOW_SHORTCUT_VIEWER_NEW" desc="the new shortcut to open the shortcut viewer app.">
ctrl+search+s
</message>
<message name="IDS_DEPRECATED_SHOW_TASK_MANAGER_MSG"
desc="Notification message to tell users about the deprecation of Shift+ESC shortcut to show the task manager.">
The shortcut to open the task manager has changed. Please use <ph name="NEW_SHORTCUT">$1<ex>Search+Esc</ex></ph> instead of <ph name="OLD_SHORTCUT">$2<ex>Shift+Esc</ex></ph>.
Open Task Manager app with "<ph name="NEW_SHORTCUT">$1<ex>Search+Esc</ex></ph>".
</message>
<message name="IDS_DEPRECATED_SHOW_SHORTCUT_VIEWER_MSG"
desc="Notification message to tell users about the deprecation of Ctrl+Alt+/ shortcut to show the shortcut viewer app.">
Open Key Shortcuts app with "<ph name="NEW_SHORTCUT">$1<ex>ctrl + search + s</ex></ph>".
</message>
<message name="IDS_DEPRECATED_SHOW_IME_BUBBLE_MSG"
desc="Notification message to tell users about the deprecation of Alt+Shift+K shortcut to show the input options menu bubble in the shelf.">
The shortcut to show the input options menu bubble in the shelf has changed. Please use <ph name="NEW_SHORTCUT">$1<ex>Search+Shift+K</ex></ph> instead of <ph name="OLD_SHORTCUT">$2<ex>Alt+Shift+K</ex></ph>.
</message>
<message name="IDS_DEPRECATED_SHORTCUT_TITLE" desc="Notification title to tell users that the keyboard accelerator is deprecated.">
Shortcut change
Use the updated shortcut
</message>
<!-- Notification that an accelerator was pressed, so the user knows how to toggle it back -->

1
ash/ash_strings_grd/IDS_DEPRECATED_SHORTCUT_TITLE.png.sha1 Normal file

@ -0,0 +1 @@
ce995a8fe20d389e1a7ab1deafae9ad428e28f84

1
ash/ash_strings_grd/IDS_DEPRECATED_SHOW_SHORTCUT_VIEWER_MSG.png.sha1 Normal file

@ -0,0 +1 @@
ce995a8fe20d389e1a7ab1deafae9ad428e28f84

1
ash/ash_strings_grd/IDS_DEPRECATED_SHOW_TASK_MANAGER_MSG.png.sha1 Normal file

@ -0,0 +1 @@
fa5ae08fee370de82e1ede77a0b0b8597c44bdda

1
ash/ash_strings_grd/IDS_SHORTCUT_SHOW_SHORTCUT_VIEWER_NEW.png.sha1 Normal file

@ -0,0 +1 @@
ce995a8fe20d389e1a7ab1deafae9ad428e28f84

1
ash/ash_strings_grd/IDS_SHORTCUT_TASK_MANAGER_NEW.png.sha1 Normal file

@ -0,0 +1 @@
fa5ae08fee370de82e1ede77a0b0b8597c44bdda

5
ash/public/cpp/accelerators.cc

@ -187,10 +187,7 @@ const AcceleratorData kAcceleratorData[] = {
{true, ui::VKEY_V, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
AcceleratorAction::kFocusPip},
{true, ui::VKEY_HELP, ui::EF_NONE, AcceleratorAction::kShowShortcutViewer},
{true, ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
AcceleratorAction::kShowShortcutViewer},
{true, ui::VKEY_OEM_2,
ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
{true, ui::VKEY_S, ui::EF_CONTROL_DOWN | ui::EF_COMMAND_DOWN,
AcceleratorAction::kShowShortcutViewer},
{true, ui::VKEY_F14, ui::EF_NONE, AcceleratorAction::kShowShortcutViewer},
{true, ui::VKEY_N, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,

2
ash/shortcut_viewer/keyboard_shortcut_viewer_metadata.cc

@ -665,7 +665,7 @@ const std::vector<ash::KeyboardShortcutItem>& GetKeyboardShortcutItemList() {
IDS_KSV_DESCRIPTION_KEYBOARD_SHORTCUT_HELPER,
{},
// |accelerator_ids|
{{ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN}}},
{{ui::VKEY_S, ui::EF_CONTROL_DOWN | ui::EF_COMMAND_DOWN}}},
{// |categories|
{ShortcutCategory::kTabAndWindow},

56
chrome/browser/ash/accessibility/spoken_feedback_browsertest.cc

@ -2248,13 +2248,13 @@ IN_PROC_BROWSER_TEST_F(DeskTemplatesSpokenFeedbackTest, DeskTemplatesBasic) {
sm_.Replay();
}
// TODO(jimmyxgong): Update this suite after the old keyboard shortcut viewer
// app is removed.
class ShortcutsAppSpokenFeedbackTest : public LoggedInSpokenFeedbackTest {
public:
ShortcutsAppSpokenFeedbackTest() {
scoped_feature_list_.InitAndDisableFeature(
{features::kOnlyShowNewShortcutsApp});
scoped_feature_list_.InitWithFeatures(
{::features::kShortcutCustomizationApp,
features::kOnlyShowNewShortcutsApp},
{});
}
ShortcutsAppSpokenFeedbackTest(const ShortcutsAppSpokenFeedbackTest&) =
delete;
@ -2266,37 +2266,41 @@ class ShortcutsAppSpokenFeedbackTest : public LoggedInSpokenFeedbackTest {
base::test::ScopedFeatureList scoped_feature_list_;
};
IN_PROC_BROWSER_TEST_F(ShortcutsAppSpokenFeedbackTest, KeyboardShortcutViewer) {
// TODO(b/288602247): Linux ChromiumOS MSan is flaky for spoken feedback tests
#if defined(MEMORY_SANITIZER)
#define MAYBE_ShortcutCustomization DISABLED_ShortcutCustomization
#else
#define MAYBE_ShortcutCustomization ShortcutCustomization
#endif
IN_PROC_BROWSER_TEST_F(ShortcutsAppSpokenFeedbackTest,
MAYBE_ShortcutCustomization) {
EnableChromeVox();
sm_.Call([this]() {
SendKeyPressWithControlAndAlt(ui::VKEY_OEM_2 /* forward slash */);
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GURL("chrome://shortcut-customization")));
});
sm_.ExpectSpeech("Shortcuts, window");
sm_.ExpectSpeech("Search shortcuts");
// Move through all tabs; make a few expectations along the way.
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.ExpectSpeech("Popular Shortcuts, tab");
sm_.ExpectSpeech("1 of 6");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.ExpectSpeech("Accessibility, tab");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.ExpectSpeech("Popular Shortcuts");
sm_.ExpectSpeech("Tab");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.ExpectSpeech("General");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.ExpectSpeech("Accessibility");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.ExpectSpeech("Keyboard settings");
// Moving forward again should dive into the list of shortcuts for the
// category.
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_RIGHT); });
sm_.ExpectSpeech("Copy selected content to the clipboard, Ctrl plus c");
sm_.ExpectSpeech("List item");
sm_.ExpectSpeech("1 of 21");
sm_.ExpectSpeech("Popular Shortcuts");
sm_.ExpectSpeech("Tab");
sm_.ExpectSpeech("List");
sm_.ExpectSpeech("with 21 items");
sm_.ExpectSpeech("General controls");
sm_.Call([this]() { SendKeyPressWithSearch(ui::VKEY_DOWN); });
sm_.ExpectSpeech("Open slash close Launcher");
sm_.ExpectSpeech("row 1 column 1");
sm_.Replay();
}
} // namespace ash

4
chrome/browser/ui/ash/keyboard_shortcut_viewer_metadata_unittest.cc

@ -24,9 +24,9 @@
namespace {
// The total number of Ash accelerators.
constexpr int kAshAcceleratorsTotalNum = 148;
constexpr int kAshAcceleratorsTotalNum = 147;
// The hash of Ash accelerators.
constexpr char kAshAcceleratorsHash[] = "f7a44d1ca086c9feb1910632c2691b1a";
constexpr char kAshAcceleratorsHash[] = "89078a8d5b54743039000bf852cd7307";
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
// Internal builds add an extra accelerator for the Feedback app.
// The total number of Chrome accelerators (available on Chrome OS).