Commit 2d90a308 authored by Wez's avatar Wez Committed by Commit Bot

Remove FixAltGraph feature and tests for old behaviour.

Bug: 823529
Change-Id: I5620f741d815619bc91bdf60ad09fa2dd24ef79c
Reviewed-on: https://chromium-review.googlesource.com/1013391Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551438}
parent 12d498ab
...@@ -1194,26 +1194,13 @@ class AltGraphEventTest ...@@ -1194,26 +1194,13 @@ class AltGraphEventTest
} }
const MSG msg_; const MSG msg_;
base::test::ScopedFeatureList feature_list_;
BYTE original_keyboard_state_[256] = {}; BYTE original_keyboard_state_[256] = {};
HKL original_keyboard_layout_ = nullptr; HKL original_keyboard_layout_ = nullptr;
}; };
} // namespace } // namespace
TEST_P(AltGraphEventTest, OldKeyEventAltGraphModifier) {
feature_list_.InitFromCommandLine("", "FixAltGraph");
// Old behaviour always sets AltGraph modifier whenever both Control and Alt
// are pressed.
KeyEvent event(msg_);
EXPECT_EQ(event.flags() & (EF_CONTROL_DOWN | EF_ALT_DOWN | EF_ALTGR_DOWN),
EF_CONTROL_DOWN | EF_ALT_DOWN | EF_ALTGR_DOWN);
}
TEST_P(AltGraphEventTest, KeyEventAltGraphModifer) { TEST_P(AltGraphEventTest, KeyEventAltGraphModifer) {
feature_list_.InitFromCommandLine("FixAltGraph", "");
KeyEvent event(msg_); KeyEvent event(msg_);
if (message_type() == WM_CHAR) { if (message_type() == WM_CHAR) {
// By definition, if we receive a WM_CHAR message when Control and Alt are // By definition, if we receive a WM_CHAR message when Control and Alt are
......
...@@ -277,10 +277,6 @@ base::LazyInstance<base::ThreadLocalStorage::Slot, ...@@ -277,10 +277,6 @@ base::LazyInstance<base::ThreadLocalStorage::Slot,
PlatformKeyMapInstanceTlsTraits> PlatformKeyMapInstanceTlsTraits>
g_platform_key_map_tls_lazy = LAZY_INSTANCE_INITIALIZER; g_platform_key_map_tls_lazy = LAZY_INSTANCE_INITIALIZER;
// TODO(crbug.com/25503): Controls Control+Alt vs AltGraph disambiguation.
const base::Feature kFixAltGraphModifier{"FixAltGraph",
base::FEATURE_ENABLED_BY_DEFAULT};
} // anonymous namespace } // anonymous namespace
PlatformKeyMap::PlatformKeyMap() {} PlatformKeyMap::PlatformKeyMap() {}
...@@ -359,8 +355,6 @@ DomKey PlatformKeyMap::DomKeyFromKeyboardCode(KeyboardCode key_code, ...@@ -359,8 +355,6 @@ DomKey PlatformKeyMap::DomKeyFromKeyboardCode(KeyboardCode key_code,
int PlatformKeyMap::ReplaceControlAndAltWithAltGraph(int flags) { int PlatformKeyMap::ReplaceControlAndAltWithAltGraph(int flags) {
if (!HasControlAndAlt(flags)) if (!HasControlAndAlt(flags))
return flags; return flags;
if (!IsFixAltGraphEnabled())
return flags;
return (flags & ~kControlAndAltFlags) | EF_ALTGR_DOWN; return (flags & ~kControlAndAltFlags) | EF_ALTGR_DOWN;
} }
...@@ -380,11 +374,6 @@ bool PlatformKeyMap::UsesAltGraph() { ...@@ -380,11 +374,6 @@ bool PlatformKeyMap::UsesAltGraph() {
return platform_key_map->has_alt_graph_; return platform_key_map->has_alt_graph_;
} }
// static
bool PlatformKeyMap::IsFixAltGraphEnabled() {
return base::FeatureList::IsEnabled(kFixAltGraphModifier);
}
void PlatformKeyMap::UpdateLayout(HKL layout) { void PlatformKeyMap::UpdateLayout(HKL layout) {
if (layout == keyboard_layout_) if (layout == keyboard_layout_)
return; return;
......
...@@ -37,12 +37,8 @@ class EVENTS_EXPORT PlatformKeyMap { ...@@ -37,12 +37,8 @@ class EVENTS_EXPORT PlatformKeyMap {
// If the supplied event has both Control and Alt modifiers set, then they // If the supplied event has both Control and Alt modifiers set, then they
// are replaced by AltGraph. This should only ever be applied to the flags // are replaced by AltGraph. This should only ever be applied to the flags
// for printable-character events. // for printable-character events.
// TODO(crbug.com/25503): Has no effect if FixAltGraph is not enabled.
static int ReplaceControlAndAltWithAltGraph(int flags); static int ReplaceControlAndAltWithAltGraph(int flags);
// TODO(crbug.com/25503): Returns true if we disambiguate AltGraph.
static bool IsFixAltGraphEnabled();
private: private:
friend class PlatformKeyMapTest; friend class PlatformKeyMapTest;
......
...@@ -387,25 +387,10 @@ class AltGraphModifierTest ...@@ -387,25 +387,10 @@ class AltGraphModifierTest
AltGraphModifierTest() : keymap_(GetPlatformKeyboardLayout(GetParam())) {} AltGraphModifierTest() : keymap_(GetPlatformKeyboardLayout(GetParam())) {}
protected: protected:
base::test::ScopedFeatureList feature_list_;
PlatformKeyMap keymap_; PlatformKeyMap keymap_;
}; };
TEST_P(AltGraphModifierTest, OldAltGraphModifierBehaviour) {
feature_list_.InitFromCommandLine("", "FixAltGraph");
// Regardless of the keyboard layout, modifier flags should be unchanged.
for (const auto& test_case : kAltGraphModifierTestCases) {
DomKeyAndFlags result = DomKeyAndFlagsFromKeyboardCode(
keymap_, test_case.key_code, test_case.flags);
EXPECT_EQ(test_case.flags, result.flags)
<< " for key_code=" << test_case.key_code;
}
}
TEST_P(AltGraphModifierTest, AltGraphModifierBehaviour) { TEST_P(AltGraphModifierTest, AltGraphModifierBehaviour) {
feature_list_.InitFromCommandLine("FixAltGraph", "");
// If the key generates a character under AltGraph then |result| should // If the key generates a character under AltGraph then |result| should
// report AltGraph, but not Control or Alt. // report AltGraph, but not Control or Alt.
for (const auto& test_case : kAltGraphModifierTestCases) { for (const auto& test_case : kAltGraphModifierTestCases) {
......
...@@ -365,9 +365,6 @@ int GetModifiersFromKeyState() { ...@@ -365,9 +365,6 @@ int GetModifiersFromKeyState() {
int modifiers = EF_NONE; int modifiers = EF_NONE;
if (ui::win::IsShiftPressed()) if (ui::win::IsShiftPressed())
modifiers |= EF_SHIFT_DOWN; modifiers |= EF_SHIFT_DOWN;
// TODO(crbug.com/25503): Handle Control+Alt vs AltGraph disambiguation, if
// enabled.
if (PlatformKeyMap::IsFixAltGraphEnabled()) {
if (ui::win::IsAltRightPressed() && PlatformKeyMap::UsesAltGraph()) { if (ui::win::IsAltRightPressed() && PlatformKeyMap::UsesAltGraph()) {
modifiers |= EF_ALTGR_DOWN; modifiers |= EF_ALTGR_DOWN;
} else { } else {
...@@ -379,14 +376,6 @@ int GetModifiersFromKeyState() { ...@@ -379,14 +376,6 @@ int GetModifiersFromKeyState() {
if (ui::win::IsAltPressed()) if (ui::win::IsAltPressed())
modifiers |= EF_ALT_DOWN; modifiers |= EF_ALT_DOWN;
} }
} else {
if (ui::win::IsCtrlPressed())
modifiers |= EF_CONTROL_DOWN;
if (ui::win::IsAltPressed())
modifiers |= EF_ALT_DOWN;
if (ui::win::IsCtrlPressed() && ui::win::IsAltPressed())
modifiers |= EF_ALTGR_DOWN;
}
if (ui::win::IsWindowsKeyPressed()) if (ui::win::IsWindowsKeyPressed())
modifiers |= EF_COMMAND_DOWN; modifiers |= EF_COMMAND_DOWN;
if (ui::win::IsNumLockOn()) if (ui::win::IsNumLockOn())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment