Removing X11 native_event uses for key events.

ui/events/event.h

 // 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.
 
 #ifndef UI_EVENTS_EVENT_H_
 #define UI_EVENTS_EVENT_H_
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/event_types.h"
@@ skipping 569 matching lines @@
 //    VKEY_Q for the key beside Tab, while French uses VKEY_A.
 // -- code_ is in one-to-one correspondence with a physical keyboard
 //    location, and does not vary depending on key layout.
 //
 // For a character event,
 // -- is_char_ is true.
 // -- type() is ET_KEY_PRESSED.
 // -- character_ is a UTF-16 character value.
 // -- key_code_ is conflated with character_ by some code, because both
 //    arrive in the wParam field of a Windows event.
-// -- code_ is "".
+// -- code_ is the empty string.
 //
 class EVENTS_EXPORT KeyEvent : public Event {
  public:
   // Create a KeyEvent from a NativeEvent. For Windows this native event can
   // be either a keystroke message (WM_KEYUP/WM_KEYDOWN) or a character message
   // (WM_CHAR). Other systems have only keystroke events.
   explicit KeyEvent(const base::NativeEvent& native_event);
 
   // Create a keystroke event.
   KeyEvent(EventType type, KeyboardCode key_code, int flags);
@@ skipping 28 matching lines @@
   // This bypasses the normal mapping from keystroke events to characters,
   // which allows an I18N virtual keyboard to fabricate a keyboard event that
   // does not have a corresponding KeyboardCode (example: U+00E1 Latin small
   // letter A with acute, U+0410 Cyrillic capital letter A).
   void set_character(base::char16 character) { character_ = character; }
 
   // Gets the character generated by this key event. It only supports Unicode
   // BMP characters.
   base::char16 GetCharacter() const;
 
+  // If this is a keystroke event with key_code_ VKEY_RETURN, returns '\r';
+  // otherwise returns the same as GetCharacter().
+  base::char16 GetUnmodifiedText() const;

[sky, 2014/09/02 18:23:15]

My initial reaction is that all of this is a mess. Do we really need all these
variants that are similar yet differ in some subtle way? Can't the subtleties be
moved to the call sites so that the code in event.h is clean and without
idiosyncrasies?

[kpschoedel, 2014/09/02 19:27:26]

That can be done, but these correspond to repeated patterns. Historically there
have been ad-hoc transformations of keystroke and/or character events with
duplicate code, or worse, *near* duplicate code with no explanation, and I've
encountered at least one bug due to someone making a change that was not carried
through everywhere. On top of that, callers of ui::KeyEvent::key_code() have not
generally been clear about which subset of ui::KeyboardCode (e.g. VKEY_SHIFT vs
VKEY_[LR]SHIFT) they expect (or indeed superset ui::KeyboardCode union UCS-2),
with the meaning of key_code_ not being documented, and defined only by implied
Windows emulation.

So the attempt to clarify and unify this is the motivation for the additional
accessors here. My own preference is still to have them as ui::KeyEvent members.
As an alternate proposal, I'd suggest the common , repeated operations be
provided as functions under ui/events/keycodes, or at least some designated
single place, with other current duplication (e.g.
content::GetWindowsKeyCodeWithoutLocation() and
DevToolsEventForwarder::VirtualKeyCodeWithoutLocation() are the same function)
eventually migrated.

[Elliot Glaysher, 2014/09/02 19:31:07]

One thing to watch out for (which you might already know) is that the vkey
constants aren't actually the same on all platforms. (I found this out the hard
way.) I would assume that in the long term, we probably want to standardize on
the windows constants. (Which is what I did in
mojo/services/public/interfaces/input_events/input_key_codes.mojom.)

[kpschoedel, 2014/09/02 19:47:16]

I suspect that's an accidental drift, and looking into that and merging
ui/events/keycodes/keyboard_codes_* is on the list of things I want to do, but
really I'd like it better if they could be defined in just one place… blink and
pdfium have their own copies, too.

+
+  // If the Control key is down in the event, returns a layout-independent
+  // character (corresponding to US layout); otherwise returns the same
+  // as GetUnmodifiedText().
+  base::char16 GetText() const;
+
   // Gets the platform key code. For XKB, this is the xksym value.
   void set_platform_keycode(uint32 keycode) { platform_keycode_ = keycode; }
   uint32 platform_keycode() const { return platform_keycode_; }
+
+  // Gets the associated (Windows-based) KeyboardCode for this key event.
+  // Historically, this has also been used to obtain the character associated
+  // with a character event, because both use the Window message 'wParam' field.
+  // This should be avoided; if necessary for backwards compatibility, use
+  // GetConflatedWindowsKeyCode().
   KeyboardCode key_code() const { return key_code_; }
 
   // True if this is a character event, false if this is a keystroke event.
   bool is_char() const { return is_char_; }
 
   // This is only intended to be used externally by classes that are modifying
   // events in an EventRewriter.
   void set_key_code(KeyboardCode key_code) { key_code_ = key_code; }
 
+  // Returns the same value as key_code(), except that located codes are
+  // returned in place of non-located ones (e.g. VKEY_LSHIFT or VKEY_RSHIFT
+  // instead of VKEY_SHIFT). This is a hybrid of semantic and physical
+  // for legacy DOM reasons.
+  KeyboardCode GetLocatedWindowsKeyboardCode() const;
+
+  // For a keystroke event, returns the same value as key_code().
+  // For a character event, returns the same value as GetCharacter().
+  // This exists for backwards compatibility with Windows key events.
+  uint16 GetConflatedWindowsKeyCode() const;
+
   // Returns true for [Alt]+<num-pad digit> Unicode alt key codes used by Win.
   // TODO(msw): Additional work may be needed for analogues on other platforms.
   bool IsUnicodeKeyCode() const;
 
   std::string code() const { return code_; }
 
   // Normalizes flags_ so that it describes the state after the event.
   // (Native X11 event flags describe the state before the event.)
   void NormalizeFlags();
 
   // Returns true if the key event has already been processed by an input method
   // and there is no need to pass the key event to the input method again.
   bool IsTranslated() const;
   // Marks this key event as translated or not translated.
   void SetTranslated(bool translated);
 
  protected:
   friend class KeyEventTestApi;
 
   // This allows a subclass TranslatedKeyEvent to be a non character event.
   void set_is_char(bool is_char) { is_char_ = is_char; }
 
  private:
+  // True if the key press originated from a 'right' key (VKEY_RSHIFT, etc.).
+  bool IsRightSideKey() const;
+
   KeyboardCode key_code_;
 
   // String of 'code' defined in DOM KeyboardEvent (e.g. 'KeyA', 'Space')
   // http://www.w3.org/TR/uievents/#keyboard-key-codes.
   //
   // This value represents the physical position in the keyboard and can be
   // converted from / to keyboard scan code like XKB.
   std::string code_;
 
   // True if this is a character event, false if this is a keystroke event.
   bool is_char_;
 
   // The platform related keycode value. For XKB, it's keysym value.
   // For now, this is used for CharacterComposer in ChromeOS.
   uint32 platform_keycode_;
 
   // String of 'key' defined in DOM KeyboardEvent (e.g. 'a', 'â')
   // http://www.w3.org/TR/uievents/#keyboard-key-codes.
   //
   // This value represents the text that the key event will insert to input
   // field. For key with modifier key, it may have specifial text.
   // e.g. CTRL+A has '\x01'.
-  base::char16 character_;
+  mutable base::char16 character_;
 
   // Parts of our event handling require raw native events (see both the
   // windows and linux implementations of web_input_event in content/). Because
   // mojo instead serializes and deserializes events in potentially different
   // processes, we need to have a mechanism to keep track of this data.
   scoped_ptr<ExtendedKeyEventData> extended_key_event_data_;
 
   static bool IsRepeated(const KeyEvent& event);
 
   static KeyEvent* last_key_event_;
@@ skipping 68 matching lines @@
 
   const GestureEventDetails& details() const { return details_; }
 
  private:
   GestureEventDetails details_;
 };
 
 }  // namespace ui
 
 #endif  // UI_EVENTS_EVENT_H_