Fixes to key codes in keyboard events generated by wxMSW.

Only set Unicode key code if the event corresponds to a character key and set
it to (newly added) WXK_NONE for the other ones to avoid nonsensical values in
it for e.g. "Home" key presses.

Also set non-Unicode key to WXK_NONE for the characters that can't be
represented in the current locale. This is consistent with wxGTK and avoids
conflicts between special key values and Unicode keys.

Clearly document the above behaviour.

Notice that implementing the correct behaviour in wxMSW involved untangling
previously interwoven WM_KEY{DOWN,UP} and WM_CHAR messages handlers. Clearly
separate them now as they get different input (key codes for the former,
characters for the latter) and especially don't try to convert from both kinds
of input using a single wxCharCodeMSWToWX() function. As this function doesn't
need to distinguish between keys and characters any more it can simply return
the converted value in all cases instead of returning 0 sometimes to indicate
a character value instead of a key. Simplify the code using this function
accordingly.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@65522 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

docs/changes.txt

@@ -413,6 +413,7 @@ All (GUI):
 - wxHTML: render in RTL order inside RTL window (Richard Bullington-McGuire).
 - wxRibbon: added EVT_RIBBONGALLERY_CLICKED event (John Roberts).
 - Add support for CP-866 encoding to wxEncodingConverter (madnut).
+- Consistency fixes for keyboard events across all major ports.
 
 MSW:
 

include/wx/msw/window.h

@@ -347,7 +347,7 @@ public:
     bool HandleMouseMove(int x, int y, WXUINT flags);
     bool HandleMouseWheel(WXWPARAM wParam, WXLPARAM lParam);
 
-    bool HandleChar(WXWPARAM wParam, WXLPARAM lParam, bool isASCII = false);
+    bool HandleChar(WXWPARAM wParam, WXLPARAM lParam);
     bool HandleKeyDown(WXWPARAM wParam, WXLPARAM lParam);
     bool HandleKeyUp(WXWPARAM wParam, WXLPARAM lParam);
 #if wxUSE_ACCEL
@@ -571,9 +571,21 @@ protected:
                              const wxString& ttip);
 #endif // wxUSE_TOOLTIPS
 
-    // the helper functions used by HandleChar/KeyXXX methods
+    // This is used by CreateKeyEvent() and also for wxEVT_CHAR[_HOOK] event
-    wxKeyEvent CreateKeyEvent(wxEventType evType, int id,
+    // creation. Notice that this method doesn't initialize wxKeyEvent
-                              WXLPARAM lParam = 0, WXWPARAM wParam = 0) const;
+    // m_keyCode and m_uniChar fields.
+    void InitAnyKeyEvent(wxKeyEvent& event,
+                         WXWPARAM wParam,
+                         WXLPARAM lParam) const;
+
+    // Helper functions used by HandleKeyXXX() methods and some derived
+    // classes, wParam and lParam have the same meaning as in WM_KEY{DOWN,UP}.
+    //
+    // NB: evType here must be wxEVT_KEY_{DOWN,UP} as wParam here contains the
+    //     virtual key code, not character!
+    wxKeyEvent CreateKeyEvent(wxEventType evType,
+                              WXWPARAM wParam,
+                              WXLPARAM lParam = 0) const;
 
 
     // default OnEraseBackground() implementation, return true if we did erase
@@ -645,7 +657,15 @@ private:
 // ---------------------------------------------------------------------------
 
 // key codes translation between wx and MSW
-WXDLLIMPEXP_CORE int wxCharCodeMSWToWX(int keySym, WXLPARAM lParam = 0);
+
+// Translate MSW virtual key code to wx key code. lParam is used to distinguish
+// between numpad and extended version of the keys, extended is assumed by
+// default if lParam == 0.
+WXDLLIMPEXP_CORE int wxCharCodeMSWToWX(WXWORD vk, WXLPARAM lParam = 0);
+
+// Translate wxKeyCode enum element (passed as int for compatibility reasons)
+// to MSW virtual key code. isExtended is set to true if the key corresponds to
+// a non-numpad version of a key that exists both on numpad and outside it.
 WXDLLIMPEXP_CORE WXWORD wxCharCodeWXToMSW(int id, bool *isExtended = NULL);
 
 // window creation helper class: before creating a new HWND, instantiate an

interface/wx/event.h

@@ -1272,14 +1272,49 @@ public:
     wxKeyEvent(wxEventType keyEventType = wxEVT_NULL);
 
     /**
-        Returns the virtual key code. ASCII events return normal ASCII values,
+        Returns the key code of the key that generated this event.
-        while non-ASCII events return values such as @b WXK_LEFT for the left
-        cursor key. See ::wxKeyCode for a full list of the virtual key codes.
 
-        Note that in Unicode build, the returned value is meaningful only if
+        ASCII symbols return normal ASCII values, while events from special
-        the user entered a character that can be represented in current
+        keys such as "left cursor arrow" (@c WXK_LEFT) return values outside of
-        locale's default charset. You can obtain the corresponding Unicode
+        the ASCII range. See ::wxKeyCode for a full list of the virtual key
-        character using GetUnicodeKey().
+        codes.
+
+        Note that this method returns a meaningful value only for special
+        non-alphanumeric keys or if the user entered a character that can be
+        represented in current locale's default charset. Otherwise, e.g. if the
+        user enters a Japanese character in a program not using Japanese
+        locale, this method returns @c WXK_NONE and GetUnicodeKey() should be
+        used to obtain the corresponding Unicode character.
+
+        Using GetUnicodeKey() is in general the right thing to do if you are
+        interested in the characters typed by the user, GetKeyCode() should be
+        only used for special keys (for which GetUnicodeKey() returns @c
+        WXK_NONE). To handle both kinds of keys you might write:
+        @code
+            void MyHandler::OnChar(wxKeyEvent& event)
+            {
+                if ( event.GetUnicodeKey() != WXK_NONE )
+                {
+                    // It's a printable character
+                    wxLogMessage("You pressed '%c'", event.GetUnicodeKey());
+                }
+                else
+                {
+                    // It's a special key, deal with all the known ones:
+                    switch ( keycode )
+                    {
+                        case WXK_LEFT:
+                        case WXK_RIGHT:
+                            ... move cursor ...
+                            break;
+
+                        case WXK_F1:
+                            ... give help ...
+                            break;
+                    }
+                }
+            }
+        @endcode
     */
     int GetKeyCode() const;
 

samples/keyboard/keyboard.cpp

@@ -348,9 +348,9 @@ wxString GetKeyName(const wxKeyEvent &event)
     const char* virt = GetVirtualKeyCodeName(keycode);
     if ( virt )
         return virt;
-    if ( keycode > 0 && keycode < 27 )
+    if ( keycode > 0 && keycode < 32 )
         return wxString::Format("Ctrl-%c", (unsigned char)('A' + keycode - 1));
-    if ( keycode >= 27 && keycode < 128 )
+    if ( keycode >= 32 && keycode < 128 )
         return wxString::Format("'%c'", (unsigned char)keycode);
 #if wxUSE_UNICODE
     return wxString::Format("'%c'", event.GetUnicodeKey());
@@ -371,7 +371,7 @@ void MyFrame::LogEvent(const wxString& name, wxKeyEvent& event)
                    "    none   "
 #endif
 #ifdef wxHAS_RAW_KEY_CODES
-                   "  %7lu    0x%lx"
+                   "  %7lu    0x%08lx"
 #else
                    "  not-set    not-set"
 #endif

src/msw/combobox.cpp

@@ -300,7 +300,7 @@ bool wxComboBox::MSWProcessEditMsg(WXUINT msg, WXWPARAM wParam, WXLPARAM lParam)
             // fall through
 
         case WM_SYSCHAR:
-            return HandleChar(wParam, lParam, true /* isASCII */);
+            return HandleChar(wParam, lParam);
 
         case WM_SYSKEYDOWN:
         case WM_KEYDOWN:

src/msw/listctrl.cpp

@@ -2340,10 +2340,7 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result)
                     {
                         eventType = wxEVT_COMMAND_LIST_KEY_DOWN;
 
-                        // wxCharCodeMSWToWX() returns 0 if the key is an ASCII
+                        event.m_code = wxCharCodeMSWToWX(wVKey);
-                        // value which should be used as is
-                        int code = wxCharCodeMSWToWX(wVKey);
-                        event.m_code = code ? code : wVKey;
                     }
 
                     event.m_itemIndex =

src/msw/treectrl.cpp

@@ -2655,18 +2655,7 @@ bool wxTreeCtrl::MSWHandleSelectionKey(unsigned vkey)
 bool wxTreeCtrl::MSWHandleTreeKeyDownEvent(WXWPARAM wParam, WXLPARAM lParam)
 {
     wxTreeEvent keyEvent(wxEVT_COMMAND_TREE_KEY_DOWN, this);
-
+    keyEvent.m_evtKey = CreateKeyEvent(wxEVT_KEY_DOWN, wParam, lParam);
-    int keyCode = wxCharCodeMSWToWX(wParam);
-
-    if ( !keyCode )
-    {
-        // wxCharCodeMSWToWX() returns 0 to indicate that this is a
-        // simple ASCII key
-        keyCode = wParam;
-    }
-
-    keyEvent.m_evtKey = CreateKeyEvent(wxEVT_KEY_DOWN, keyCode,
-                                       lParam, wParam);
 
     bool processed = HandleTreeEvent(keyEvent);
 

src/msw/window.cpp

@@ -3135,32 +3135,36 @@ WXLRESULT wxWindowMSW::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM l
 
         case WM_SYSKEYDOWN:
         case WM_KEYDOWN:
-            // If this has been processed by an event handler, return 0 now
+            // Generate the key down event in any case.
-            // (we've handled it).
             m_lastKeydownProcessed = HandleKeyDown((WORD) wParam, lParam);
             if ( m_lastKeydownProcessed )
             {
+                // If it was processed by an event handler, we stop here,
+                // notably we intentionally don't generate char event then.
                 processed = true;
             }
-
+            else // key down event not handled
-            if ( !processed )
             {
+                // Examine the event to decide whether we need to generate a
+                // char event for it ourselves or let Windows do it. Window
+                // mostly only does it for the keys which produce printable
+                // characters (although there are exceptions, e.g. VK_ESCAPE or
+                // VK_BACK (but not VK_DELETE)) while we do it for all keys
+                // except the modifier ones (the wisdom of this is debatable
+                // but by now this decision is enshrined forever due to
+                // backwards compatibility).
                 switch ( wParam )
                 {
-                    // we consider these messages "not interesting" to OnChar, so
+                    // No wxEVT_CHAR events are generated for these keys at all.
-                    // just don't do anything more with them
                     case VK_SHIFT:
                     case VK_CONTROL:
                     case VK_MENU:
                     case VK_CAPITAL:
                     case VK_NUMLOCK:
                     case VK_SCROLL:
-                        processed = true;
-                        break;
 
-                    // avoid duplicate messages to OnChar for these ASCII keys:
+                    // Windows will send us WM_CHAR for these ones so we'll
-                    // they will be translated by TranslateMessage() and received
+                    // generate wxEVT_CHAR for them later when we get it.
-                    // in WM_CHAR
                     case VK_ESCAPE:
                     case VK_SPACE:
                     case VK_RETURN:
@@ -3192,10 +3196,6 @@ WXLRESULT wxWindowMSW::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM l
                     case VK_OEM_COMMA:
                     case VK_OEM_MINUS:
                     case VK_OEM_PERIOD:
-                        // but set processed to false, not true to still pass them
-                        // to the control's default window proc - otherwise
-                        // built-in keyboard handling won't work
-                        processed = false;
                         break;
 
 #ifdef VK_APPS
@@ -3207,8 +3207,28 @@ WXLRESULT wxWindowMSW::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM l
 #endif // VK_APPS
 
                     default:
-                        // do generate a CHAR event
+                        if ( (wParam >= '0' && wParam <= '9') ||
-                        processed = HandleChar((WORD)wParam, lParam);
+                                (wParam >= 'A' && wParam <= 'Z') )
+                        {
+                            // We'll get WM_CHAR for those later too.
+                            break;
+                        }
+
+                        // But for the rest we won't get WM_CHAR later so we do
+                        // need to generate the event right now.
+                        wxKeyEvent event(wxEVT_CHAR);
+                        InitAnyKeyEvent(event, wParam, lParam);
+
+                        // Set the "extended" bit in lParam because we want to
+                        // generate CHAR events with WXK_HOME and not
+                        // WXK_NUMPAD_HOME even if the "Home" key on numpad was
+                        // pressed.
+                        event.m_keyCode = wxCharCodeMSWToWX
+                                          (
+                                            wParam,
+                                            lParam | (KF_EXTENDED << 16)
+                                          );
+                        processed = HandleWindowEvent(event);
                 }
             }
             if (message == WM_SYSKEYDOWN)  // Let Windows still handle the SYSKEYs
@@ -3242,7 +3262,7 @@ WXLRESULT wxWindowMSW::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM l
             }
             else
             {
-                processed = HandleChar((WORD)wParam, lParam, true);
+                processed = HandleChar((WORD)wParam, lParam);
             }
             break;
 
@@ -5607,24 +5627,18 @@ void wxWindowMSW::GenerateMouseLeave()
 // keyboard handling
 // ---------------------------------------------------------------------------
 
-// create the key event of the given type for the given key - used by
+void
-// HandleChar and HandleKeyDown/Up
+wxWindowMSW::InitAnyKeyEvent(wxKeyEvent& event,
-wxKeyEvent wxWindowMSW::CreateKeyEvent(wxEventType evType,
+                             WXWPARAM wParam,
-                                       int id,
+                             WXLPARAM lParam) const
-                                       WXLPARAM lParam,
-                                       WXWPARAM wParam) const
 {
-    wxKeyEvent event(evType);
     event.SetId(GetId());
     event.m_shiftDown = wxIsShiftDown();
     event.m_controlDown = wxIsCtrlDown();
     event.m_altDown = (HIWORD(lParam) & KF_ALTDOWN) == KF_ALTDOWN;
 
-    event.SetEventObject((wxWindow *)this); // const_cast
+    event.SetEventObject(const_cast<wxWindow *>(this));
-    event.m_keyCode = id;
+
-#if wxUSE_UNICODE
-    event.m_uniChar = (wxChar) wParam;
-#endif
     event.m_rawCode = (wxUint32) wParam;
     event.m_rawFlags = (wxUint32) lParam;
 #ifndef __WXWINCE__
@@ -5635,34 +5649,70 @@ wxKeyEvent wxWindowMSW::CreateKeyEvent(wxEventType evType,
     const wxPoint mousePos = ScreenToClient(wxGetMousePosition());
     event.m_x = mousePos.x;
     event.m_y = mousePos.y;
+}
+
+wxKeyEvent
+wxWindowMSW::CreateKeyEvent(wxEventType evType,
+                            WXWPARAM wParam,
+                            WXLPARAM lParam) const
+{
+    // Catch any attempts to use this with WM_CHAR, it wouldn't work because
+    // wParam is supposed to be a virtual key and not a character here.
+    wxASSERT_MSG( evType != wxEVT_CHAR && evType != wxEVT_CHAR_HOOK,
+                    "CreateKeyEvent() can't be used for char events" );
+
+    wxKeyEvent event(evType);
+    InitAnyKeyEvent(event, wParam, lParam);
+
+    event.m_keyCode = wxCharCodeMSWToWX(wParam, lParam);
+#if wxUSE_UNICODE
+    if ( event.m_keyCode < WXK_START )
+    {
+        // It's an ASCII character, set Unicode key code to the same value
+        // for compatibility (both with the previous versions of wx and with
+        // the other ports), even if it's not very useful for these events as
+        // Unicode character is/should be mostly used by EVT_CHAR handlers.
+        event.m_uniChar = event.m_keyCode;
+    }
+#endif // wxUSE_UNICODE
 
     return event;
 }
 
 // isASCII is true only when we're called from WM_CHAR handler and not from
 // WM_KEYDOWN one
-bool wxWindowMSW::HandleChar(WXWPARAM wParam, WXLPARAM lParam, bool isASCII)
+bool wxWindowMSW::HandleChar(WXWPARAM wParam, WXLPARAM lParam)
 {
-    int keycode;
+    wxKeyEvent event(wxEVT_CHAR);
-    if ( isASCII )
+    InitAnyKeyEvent(event, wParam, lParam);
-    {
-        keycode = wParam;
-    }
-    else // we're called from WM_KEYDOWN
-    {
-        // don't pass lParam to wxCharCodeMSWToWX() here because we don't want
-        // to get numpad key codes: CHAR events should use the logical keys
-        // such as WXK_HOME instead of WXK_NUMPAD_HOME which is for KEY events
-        keycode = wxCharCodeMSWToWX(wParam);
-        if ( keycode == 0 )
-        {
-            // it's ASCII and will be processed here only when called from
-            // WM_CHAR (i.e. when isASCII = true), don't process it now
-            return false;
-        }
-    }
 
-    wxKeyEvent event(CreateKeyEvent(wxEVT_CHAR, keycode, lParam, wParam));
+#if wxUSE_UNICODE
+    // TODO: wParam uses UTF-16 so this is incorrect for characters outside of
+    //       the BMP, we should use WM_UNICHAR to handle them.
+    event.m_uniChar = wParam;
+#endif // wxUSE_UNICODE
+
+    // Set non-Unicode key code too for compatibility if possible.
+    if ( wParam < 0x80 )
+    {
+        // It's an ASCII character, no need to translate it.
+        event.m_keyCode = wParam;
+    }
+    else
+    {
+        // Check if this key can be represented (as a single character) in the
+        // current locale.
+        const wchar_t wc = wParam;
+        char ch;
+        if ( wxConvLibc.FromWChar(&ch, 1, &wc, 1) != wxCONV_FAILED )
+        {
+            // For compatibility continue to provide the key code in this field
+            // even though using GetUnicodeKey() is recommended now.
+            event.m_keyCode = static_cast<unsigned char>(ch);
+        }
+        //else: Key can't be represented in the current locale, leave m_keyCode
+        //      as WXK_NONE and use GetUnicodeKey() to access the character.
+    }
 
     // the alphanumeric keys produced by pressing AltGr+something on European
     // keyboards have both Ctrl and Alt modifiers which may confuse the user
@@ -5671,7 +5721,7 @@ bool wxWindowMSW::HandleChar(WXWPARAM wParam, WXLPARAM lParam, bool isASCII)
     // KEY_DOWN event would still have the correct modifiers if they're really
     // needed)
     if ( event.m_controlDown && event.m_altDown &&
-            (keycode >= 32 && keycode < 256) )
+            (event.m_keyCode >= 32 && event.m_keyCode < 256) )
     {
         event.m_controlDown =
         event.m_altDown = false;
@@ -5682,29 +5732,13 @@ bool wxWindowMSW::HandleChar(WXWPARAM wParam, WXLPARAM lParam, bool isASCII)
 
 bool wxWindowMSW::HandleKeyDown(WXWPARAM wParam, WXLPARAM lParam)
 {
-    int id = wxCharCodeMSWToWX(wParam, lParam);
+    wxKeyEvent event(CreateKeyEvent(wxEVT_KEY_DOWN, wParam, lParam));
-
-    if ( !id )
-    {
-        // normal ASCII char
-        id = wParam;
-    }
-
-    wxKeyEvent event(CreateKeyEvent(wxEVT_KEY_DOWN, id, lParam, wParam));
     return HandleWindowEvent(event);
 }
 
 bool wxWindowMSW::HandleKeyUp(WXWPARAM wParam, WXLPARAM lParam)
 {
-    int id = wxCharCodeMSWToWX(wParam, lParam);
+    wxKeyEvent event(CreateKeyEvent(wxEVT_KEY_UP, wParam, lParam));
-
-    if ( !id )
-    {
-        // normal ASCII char
-        id = wParam;
-    }
-
-    wxKeyEvent event(CreateKeyEvent(wxEVT_KEY_UP, id, lParam, wParam));
     return HandleWindowEvent(event);
 }
 
@@ -6116,9 +6150,7 @@ const struct wxKeyMapping
 
 } // anonymous namespace
 
-// Returns 0 if was a normal ASCII value, not a special key. This indicates that
+int wxCharCodeMSWToWX(WXWORD vk, WXLPARAM lParam)
-// the key should be ignored by WM_KEYDOWN and processed by WM_CHAR instead.
-int wxCharCodeMSWToWX(int vk, WXLPARAM lParam)
 {
     // check the table first
     for ( size_t n = 0; n < WXSIZEOF(gs_specialKeys); n++ )
@@ -6193,7 +6225,9 @@ int wxCharCodeMSWToWX(int vk, WXLPARAM lParam)
             break;
 
         default:
-            wxk = 0;
+            // must be a simple alphanumeric key and the values of them
+            // coincide in Windows and wx
+            wxk = vk;
     }
 
     return wxk;
@@ -6462,7 +6496,7 @@ wxKeyboardHook(int nCode, WORD wParam, DWORD lParam)
     if ( nCode != HC_NOREMOVE && ((hiWord & KF_UP) == 0) )
     {
         int id = wxCharCodeMSWToWX(wParam, lParam);
-        if ( id != 0 )
+        if ( id >= WXK_START )
         {
             wxKeyEvent event(wxEVT_CHAR_HOOK);
             if ( (HIWORD(lParam) & KF_ALTDOWN) == KF_ALTDOWN )
@@ -7132,12 +7166,10 @@ bool wxWindowMSW::UnregisterHotKey(int hotkeyId)
 
 bool wxWindowMSW::HandleHotKey(WXWPARAM wParam, WXLPARAM lParam)
 {
-    int hotkeyId = wParam;
-    int virtualKey = HIWORD(lParam);
     int win_modifiers = LOWORD(lParam);
 
-    wxKeyEvent event(CreateKeyEvent(wxEVT_HOTKEY, virtualKey, wParam, lParam));
+    wxKeyEvent event(CreateKeyEvent(wxEVT_HOTKEY, HIWORD(lParam)));
-    event.SetId(hotkeyId);
+    event.SetId(wParam);
     event.m_shiftDown = (win_modifiers & MOD_SHIFT) != 0;
     event.m_controlDown = (win_modifiers & MOD_CONTROL) != 0;
     event.m_altDown = (win_modifiers & MOD_ALT) != 0;