MacViews: Fix accelerator handling while Omnibox is in focus.

ui/views/controls/textfield/textfield.cc

Index: ui/views/controls/textfield/textfield.cc
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index 4b0bef22f724ba3c3d7bd4defe8aab6874b4560c..c7bf227dc62dbc7589b1025d17202f1d565080f6 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -92,7 +92,11 @@ ui::TextEditCommand GetCommandForKeyEvent(const ui::KeyEvent& event) {
     return ui::TextEditCommand::INVALID_COMMAND;
 
   const bool shift = event.IsShiftDown();
+#if defined(OS_MACOSX)
+  const bool control = event.IsCommandDown();

[themblsha, 2016/11/22 14:33:48]

Fixes TextfieldTest.CutCopyPaste. "Ensure [Ctrl]+[Shift]+[Insert] is a no-op"
triggered Paste because it looked at Control modifier on Mac and actually sent
Cmd+Shift+Insert.

[tapted, 2016/11/23 02:09:30]

This doesn't look right -- E.g. -[BridgedContentView copy:] uses
EF_CONTROL_DOWN, (but also an edit command so it won't end up here).

I'd suggest something like

const bool control = event.IsControlDown() || event.IsCommandDown();

(i.e. no #ifdef guard - do that on all platforms - IsCommandDown should only
come up on mac anyway)


It might also be possible that the test doesn't make sense on Mac, or should
really be dispatching the event differently (e.g. EventGenerator::WINDOW), since
in "real" code (outside of tests) this is handled elsewhere on Mac.

[themblsha, 2016/11/23 15:16:39]

Just EventGenerator::WINDOW is not enough, but in conjunction with this fix and
Textfield::GetAcceleratorForCommandId() it works fine.

+#else
   const bool control = event.IsControlDown();
+#endif
   const bool alt = event.IsAltDown() || event.IsAltGrDown();
   switch (event.key_code()) {
     case ui::VKEY_Z:
@@ -1264,6 +1268,14 @@ void Textfield::InsertText(const base::string16& new_text) {
 }
 
 void Textfield::InsertChar(const ui::KeyEvent& event) {
+#if defined(OS_MACOSX)
+  // MockInputMethod::DispatchKeyEvent() will try to incorrectly call
+  // TestTextfield::InsertChar(), even though when Command is pressed text input
+  // shouldn't work at all.
+  if (event.IsCommandDown())
+    return;
+#endif

[themblsha, 2016/11/22 14:33:48]

Fixes TextfieldTest.KeysWithModifiersTest. MockInputMethod dispatches Cmd+2
KeyDown event directly to InsertChar().

[tapted, 2016/11/23 02:09:30]

This test already behaves correctly on CrOS. I think the right fix for this is
to change ui/events/base_event_utils.cc and add a `kSystemKeyModifierMask` for
Mac that is just EF_COMMAND_DOWN  -- (Mac *shouldn't* include EF_ALT_DOWN for
the same reasons as described in the comment for IsSystemKeyModifier - i.e. Alt
is used to input extended characters on Mac).

[themblsha, 2016/11/23 15:16:39]

Whoa, this really fixes it. Thanks!

+
   if (read_only()) {
     OnEditFailed();
     return;