Generate the same flags for modifier key events in wxGTK as in wxMSW.

Resolve the discrepancy between modifier flags of the key events corresponding
to the modifier key itself between wxMSW and wxGTK by changing the latter to
follow wxMSW behaviour.

Clearly document the now officially correct behaviour of the modifiers for the
key events corresponding to the modifiers keys themselves in the manual.

This fix also makes it unnecessary to work around this bug in the keyboard
unit test so remove it from there.

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

interface/wx/event.h

@@ -1257,19 +1257,31 @@ enum wxKeyCategoryFlags
     The modifiers keys are @c WXK_SHIFT, @c WXK_CONTROL, @c WXK_ALT and various
     @c WXK_WINDOWS_XXX from ::wxKeyCode enum.
 
+    Modifier keys events are special in one additional aspect: usually the
+    keyboard state associated with a key press is well defined, e.g.
+    wxKeyboardState::ShiftDown() returns @c true only if the Shift key was held
+    pressed when the key that generated this event itself was pressed. There is
+    an ambiguity for the key press events for Shift key itself however. By
+    convention, it is considered to be already pressed when it is pressed and
+    already released when it is released. In other words, @c wxEVT_KEY_DOWN
+    event for the Shift key itself will have @c wxMOD_SHIFT in GetModifiers()
+    and ShiftDown() will return true while the @c wxEVT_KEY_UP event for Shift
+    itself will not have @c wxMOD_SHIFT in its modifiers and ShiftDown() will
+    return false.
 
-    You may discover how the other keys on your system behave interactively by
-    running the @ref page_samples_keyboard wxWidgets sample and pressing some
-    keys in it.
 
-    @b Tip: be sure to call @c event.Skip() for events that you don't process in
+    @b Tip: You may discover the key codes and modifiers generated by all the
-    key event function, otherwise menu shortcuts may cease to work under Windows.
+        keys on your system interactively by running the @ref
+        page_samples_keyboard wxWidgets sample and pressing some keys in it.
 
     @note If a key down (@c EVT_KEY_DOWN) event is caught and the event handler
           does not call @c event.Skip() then the corresponding char event
-          (@c EVT_CHAR) will not happen.
+          (@c EVT_CHAR) will not happen. This is by design and enables the
-          This is by design and enables the programs that handle both types of
+          programs that handle both types of events to avoid processing the
-          events to be a bit simpler.
+          same key twice. As a consequence, if you do not want to suppress the
+          @c wxEVT_CHAR events for the keys you handle, always call @c
+          event.Skip() in your @c wxEVT_KEY_DOWN handler. Not doing may also
+          prevent accelerators defined using this key from working.
 
     @note For Windows programmers: The key and char events in wxWidgets are
           similar to but slightly different from Windows @c WM_KEYDOWN and

src/gtk/window.cpp

@@ -680,12 +680,57 @@ static void wxFillOtherKeyEventFields(wxKeyEvent& event,
 
     event.SetTimestamp( gdk_event->time );
     event.SetId(win->GetId());
+
     event.m_shiftDown = (gdk_event->state & GDK_SHIFT_MASK) != 0;
     event.m_controlDown = (gdk_event->state & GDK_CONTROL_MASK) != 0;
     event.m_altDown = (gdk_event->state & GDK_MOD1_MASK) != 0;
     event.m_metaDown = (gdk_event->state & GDK_META_MASK) != 0;
+
+    // Normally we take the state of modifiers directly from the low level GDK
+    // event but unfortunately GDK uses a different convention from MSW for the
+    // key events corresponding to the modifier keys themselves: in it, when
+    // e.g. Shift key is pressed, GDK_SHIFT_MASK is not set while it is set
+    // when Shift is released. Under MSW the situation is exactly reversed and
+    // the modifier corresponding to the key is set when it is pressed and
+    // unset when it is released. To ensure consistent behaviour between
+    // platforms (and because it seems to make slightly more sense, although
+    // arguably both behaviours are reasonable) we follow MSW here.
+    //
+    // Final notice: we set the flags to the desired value instead of just
+    // inverting them because they are not set correctly (i.e. in the same way
+    // as for the real events generated by the user) for wxUIActionSimulator-
+    // produced events and it seems better to keep that class code the same
+    // among all platforms and fix the discrepancy here instead of adding
+    // wxGTK-specific code to wxUIActionSimulator.
+    const bool isPress = gdk_event->type == GDK_KEY_PRESS;
+    switch ( gdk_event->keyval )
+    {
+        case GDK_Shift_L:
+        case GDK_Shift_R:
+            event.m_shiftDown = isPress;
+            break;
+
+        case GDK_Control_L:
+        case GDK_Control_R:
+            event.m_controlDown = isPress;
+            break;
+
+        case GDK_Alt_L:
+        case GDK_Alt_R:
+            event.m_altDown = isPress;
+            break;
+
+        case GDK_Meta_L:
+        case GDK_Meta_R:
+        case GDK_Super_L:
+        case GDK_Super_R:
+            event.m_metaDown = isPress;
+            break;
+    }
+
     event.m_rawCode = (wxUint32) gdk_event->keyval;
     event.m_rawFlags = 0;
+
     wxGetMousePosition( &x, &y );
     win->ScreenToClient( &x, &y );
     event.m_x = x;

tests/events/keyboard.cpp

@@ -110,13 +110,7 @@ struct KeyDesc
     int m_mods;
 };
 
-// These functions are only needed because of wx bug: currently, modifiers key
+// Helper for ModKeyDown().
-// events are inconsistent between platforms and wxMSW generates key down event
-// for e.g. WXK_CONTROL with wxMOD_CONTROL set and key up event with it unset
-// while wxGTK does exactly vice versa. So we provide these helpers to make it
-// possible to make the tests pass under all platforms for now but ideally they
-// should all be made to behave the same and this should become unnecessary.
-
 int GetModForKey(int keycode)
 {
     switch ( keycode )
@@ -131,39 +125,19 @@ int GetModForKey(int keycode)
     return wxMOD_NONE;
 }
 
-#ifdef __WXGTK__
+// Helper function to allow writing just ModKeyDown(WXK_CONTROL) instead of
-
+// more verbose KeyDesc(WXK_CONTROL, wxMOD_CONTROL).
-KeyDesc ModKeyDown(int keycode)
-{
-    // Second level bug: currently wxUIActionSimulator produces different
-    // modifiers than actually pressing the key. So while the above comment is
-    // true for keys pressed by user, when simulating them we do get the
-    // corresponding bit set for the modifier press events.
-    //
-    // Again, this is a bug and wxUIActionSimulator should be fixed to behave
-    // as the real events do but until this happens just work around this here.
-    return KeyDesc(keycode, GetModForKey(keycode));
-}
-
-KeyDesc ModKeyUp(int keycode)
-{
-    return KeyDesc(keycode, GetModForKey(keycode));
-}
-
-#else // Assume MSW-like behaviour for all the other platforms.
-
 KeyDesc ModKeyDown(int keycode)
 {
     return KeyDesc(keycode, GetModForKey(keycode));
 }
 
+// Another helper provided for symmetry with ModKeyDown() only.
 KeyDesc ModKeyUp(int keycode)
 {
     return KeyDesc(keycode);
 }
 
-#endif // Platforms.
-
 // Verify that the event object corresponds to our idea of what it should be.
 void TestEvent(int line, const wxKeyEvent& ev, const KeyDesc& desc)
 {