Index: chrome/common/extensions/extension_unittest.cc |
=================================================================== |
--- chrome/common/extensions/extension_unittest.cc (revision 129600) |
+++ chrome/common/extensions/extension_unittest.cc (working copy) |
@@ -945,14 +945,16 @@ |
// test |command_name| being blank as it is used as a key in the manifest, |
// so it can't be blank (and we DCHECK when it is). |
{ false, None, "command", "", "" }, |
- { false, None, "command", "ctrl+f", "" }, |
+ { false, None, "command", "Ctrl+f", "" }, |
{ false, None, "command", "", "description" }, |
// Ctrl+Alt is not permitted, see MSDN link in comments in Parse function. |
{ false, None, "command", "Ctrl+Alt+F", "description" }, |
- // Unsupported shortcuts/too many. |
- { false, None, "command", "F10", "description" }, |
- { false, None, "command", "Ctrl+1", "description" }, |
- { false, None, "command", "Ctrl+F+G", "description" }, |
+ // Unsupported shortcuts/too many, or missing modifier. |
+ { false, None, "command", "A", "description" }, |
+ { false, None, "command", "F10", "description" }, |
+ { false, None, "command", "Ctrl+1", "description" }, |
+ { false, None, "command", "Ctrl+F+G", "description" }, |
+ { false, None, "command", "Ctrl+Alt+Shift+G", "description" }, |
// Basic tests. |
{ true, CtrlF, "command", "Ctrl+F", "description" }, |
{ true, ShiftF, "command", "Shift+F", "description" }, |
@@ -967,35 +969,50 @@ |
{ true, CtrlShiftF, "command", "F+Shift+Ctrl", "description" }, |
{ true, AltShiftF, "command", "F+Alt+Shift", "description" }, |
{ true, AltShiftF, "command", "F+Shift+Alt", "description" }, |
- // Case insensitivity is OK. |
- { true, CtrlF, "command", "Ctrl+F", "description" }, |
- { true, CtrlF, "command", "cTrL+f", "description" }, |
- // Extra spaces are fine. |
- { true, CtrlShiftF, "command", " Ctrl + Shift +F", "description" }, |
- // Minus is equivalent to plus. |
- { true, CtrlShiftF, "command", "Ctrl+Shift-F", "description" }, |
+ // Case insensitivity is not OK. |
+ { false, CtrlF, "command", "Ctrl+f", "description" }, |
+ { false, CtrlF, "command", "cTrL+F", "description" }, |
// Skipping description is OK for browser- and pageActions. |
- { true, CtrlF, "browserAction", "Ctrl+F", "" }, |
- { true, CtrlF, "pageAction", "Ctrl+F", "" }, |
+ { true, CtrlF, "_execute_browser_action", "Ctrl+F", "" }, |
+ { true, CtrlF, "_execute_page_action", "Ctrl+F", "" }, |
}; |
// TODO(finnur): test Command/Options on Mac when implemented. |
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) { |
+ // First parse the command as a simple string. |
scoped_ptr<DictionaryValue> command(new DictionaryValue); |
- command->SetString("key", kTests[i].key); |
+ command->SetString("suggested_key", kTests[i].key); |
command->SetString("description", kTests[i].description); |
+ SCOPED_TRACE(std::string("Command name: |") + kTests[i].command_name + |
+ "| key: |" + kTests[i].key + |
+ "| description: |" + kTests[i].description + |
+ "| index: " + base::IntToString(i)); |
+ |
Extension::ExtensionKeybinding keybinding; |
string16 error; |
bool result = |
keybinding.Parse(command.get(), kTests[i].command_name, i, &error); |
- SCOPED_TRACE(std::string("Command name: |") + kTests[i].command_name + |
- "| key: |" + kTests[i].key + |
- "| description: |" + kTests[i].description + |
- "| index: " + base::IntToString(i)); |
+ EXPECT_EQ(kTests[i].expected_result, result); |
+ if (result) { |
+ EXPECT_STREQ(kTests[i].description, keybinding.description().c_str()); |
+ EXPECT_STREQ(kTests[i].command_name, keybinding.command_name().c_str()); |
+ EXPECT_EQ(kTests[i].accelerator, keybinding.accelerator()); |
+ } |
+ // Now parse the command as a dictionary of multiple values. |
+ command.reset(new DictionaryValue); |
+ DictionaryValue* key_dict = new DictionaryValue(); |
+ key_dict->SetString("default", kTests[i].key); |
+ key_dict->SetString("windows", kTests[i].key); |
+ key_dict->SetString("mac", kTests[i].key); |
+ command->Set("suggested_key", key_dict); |
+ command->SetString("description", kTests[i].description); |
+ |
+ result = keybinding.Parse(command.get(), kTests[i].command_name, i, &error); |
+ |
EXPECT_EQ(kTests[i].expected_result, result); |
if (result) { |
EXPECT_STREQ(kTests[i].description, keybinding.description().c_str()); |
@@ -1005,6 +1022,78 @@ |
} |
} |
+TEST(ExtensionTest, ExtensionKeybindingParsingFallback) { |
+ std::string description = "desc"; |
+ std::string command_name = "foo"; |
+ |
+ // Test that platform specific keys are honored on each platform, despite |
+ // fallback being given. |
+ scoped_ptr<DictionaryValue> command(new DictionaryValue); |
+ DictionaryValue* key_dict = new DictionaryValue(); |
+ key_dict->SetString("default", "Ctrl+Shift+D"); |
+ key_dict->SetString("windows", "Ctrl+Shift+W"); |
+ key_dict->SetString("mac", "Ctrl+Shift+M"); |
+ key_dict->SetString("linux", "Ctrl+Shift+L"); |
+ key_dict->SetString("chromeos", "Ctrl+Shift+C"); |
+ command->Set("suggested_key", key_dict); |
+ command->SetString("description", description); |
+ |
+ Extension::ExtensionKeybinding keybinding; |
+ string16 error; |
+ EXPECT_TRUE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ EXPECT_STREQ(description.c_str(), keybinding.description().c_str()); |
+ EXPECT_STREQ(command_name.c_str(), keybinding.command_name().c_str()); |
+ |
+#if defined(OS_WIN) |
+ ui::Accelerator accelerator(ui::VKEY_W, true, true, false); |
+#elif defined(OS_MACOSX) |
+ ui::Accelerator accelerator(ui::VKEY_M, true, true, false); |
+#elif defined(OS_CHROMEOS) |
+ ui::Accelerator accelerator(ui::VKEY_C, true, true, false); |
+#elif defined(OS_LINUX) |
+ ui::Accelerator accelerator(ui::VKEY_L, true, true, false); |
+#else |
+ ui::Accelerator accelerator(ui::VKEY_D, true, true, false); |
+#endif |
+ EXPECT_EQ(accelerator, keybinding.accelerator()); |
+ |
+ // Misspell a platform. |
+ key_dict->SetString("windosw", "Ctrl+M"); |
+ EXPECT_FALSE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ EXPECT_TRUE(key_dict->Remove("windosw", NULL)); |
+ |
+ // Now remove platform specific keys (leaving just "default") and make sure |
+ // every platform falls back to the default. |
+ EXPECT_TRUE(key_dict->Remove("windows", NULL)); |
+ EXPECT_TRUE(key_dict->Remove("mac", NULL)); |
+ EXPECT_TRUE(key_dict->Remove("linux", NULL)); |
+ EXPECT_TRUE(key_dict->Remove("chromeos", NULL)); |
+ EXPECT_TRUE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ EXPECT_EQ(ui::VKEY_D, keybinding.accelerator().key_code()); |
+ |
+ // Now remove "default", leaving no option but failure. Or, in the words of |
+ // the immortal Adam Savage: "Failure is always an option". |
+ EXPECT_TRUE(key_dict->Remove("default", NULL)); |
+ EXPECT_FALSE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ |
+ // Now add only a valid platform that we are not running on to make sure devs |
+ // are notified of errors on other platforms. |
+#if defined(OS_WIN) |
+ key_dict->SetString("mac", "Ctrl+Shift+M"); |
+#else |
+ key_dict->SetString("windows", "Ctrl+Shift+W"); |
+#endif |
+ EXPECT_FALSE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ |
+ // Make sure Mac specific keys are not processed on other platforms. |
+#if !defined(OS_MACOSX) |
+ key_dict->SetString("windows", "Command+Shift+M"); |
+ EXPECT_FALSE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+ key_dict->SetString("windows", "Options+Shift+M"); |
+ EXPECT_FALSE(keybinding.Parse(command.get(), command_name, 0, &error)); |
+#endif |
+} |
+ |
// These last 2 tests don't make sense on Chrome OS, where extension plugins |
// are not allowed. |
#if !defined(OS_CHROMEOS) |