Do not consume an ET_KEY_RELEASED event in NativeTextfieldViews and NativeComboboxViews

ui/views/controls/textfield/native_textfield_views.cc

 // Copyright (c) 2012 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 "ui/views/controls/textfield/native_textfield_views.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 476 matching lines @@
 
 bool NativeTextfieldViews::HandleKeyPressed(const KeyEvent& e) {
   TextfieldController* controller = textfield_->GetController();
   bool handled = false;
   if (controller)
     handled = controller->HandleKeyEvent(textfield_, e);
   return handled || HandleKeyEvent(e);
 }
 
 bool NativeTextfieldViews::HandleKeyReleased(const KeyEvent& e) {
-  return true;
+  return false;  // crbug.com/127520

[oshima, 2012/05/10 15:36:37]

Are we sure we don't want to return true when KeyPress was handled? just double
checking.

[Yusuke Sato, 2012/05/11 02:16:22]

Unconditionally returning false would be better, I believe.

For example, when Shift+Alt+X+ET_KEY_PRESSED is always consumed by the key press
handler and Shift+Alt+ET_KEY_RELEASED is registered as an accelerator,
Shift+Alt+X press/release should not trigger the Shift+Alt accelerator. However,
returning true for the X release makes it very hard for the accelerator manager
to distinguish Shift+Alt+X and Shit+Alt.

[oshima, 2012/05/11 17:23:03]

Does it distinguish it now? I'm probably OK with this change unless this cause
some issue, but we may want to think about long term plan.

[Yusuke Sato, 2012/05/14 04:34:49]

Yes. ui::AcceleratorManager (ui/base/accelerators/accelerator_manager.h) has an
ability to distinguish between Shift+Alt+X and Shift+Alt. In short, 'bool
AcceleratorManager::ShouldHandle(const Accelerator& accelerator) const' method
in the manager class returns true when Alt is released after Shift+Alt are
pressed, but returns false when Alt is released after Shift+Alt+<whatever> are
pressed.

 }
 
 void NativeTextfieldViews::HandleFocus() {
   GetRenderText()->set_focused(true);
   is_cursor_visible_ = true;
   SchedulePaint();
   OnCaretBoundsChanged();
   // Start blinking cursor.
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
@@ skipping 631 matching lines @@
 
 #if defined(USE_AURA)
 // static
 NativeTextfieldWrapper* NativeTextfieldWrapper::CreateWrapper(
     Textfield* field) {
   return new NativeTextfieldViews(field);
 }
 #endif
 
 }  // namespace views