Better detection of AltGr on Windows

Try to properly detect the fake CtrlL+AltR sequence Windows sends
when pressing AltGr. This allows us to send more accurate key
events over to the server.

3 files changed, 69 insertions, 39 deletions

diff --git a/doc/keyboard-test.txt b/doc/keyboard-test.txt
index 914b8ebf..6375a9e5 100644
--- a/doc/keyboard-test.txt
+++ b/doc/keyboard-test.txt
@@ -66,7 +66,7 @@ Client (classic, non-raw, mode)
 
   - CapsLock, NumLock
   - Shift, Ctrl
-  - Alt, AltGr, Super [Win, X11] (FIXME: AltGr broken on Win)
+  - Alt, AltGr, Super [Win, X11] (FIXME: Ctrl+AltGr broken on Win)
   - Meta [X11]
   - Left/right identification (FIXME: broken for Shift on Win)
   - CmdL => AltL, CmdR => SuperL, AltL => ModeSwitch, AltR => Level3Shift [OS X]
@@ -74,8 +74,7 @@ Client (classic, non-raw, mode)
 
   - CapsLock, Shift and AltGr affect symbol lookup
   - Ctrl does not affect symbol lookup
-  - CtrlL+AltR is fake released to compensate for Windows' AltGr magic [Win]
-  - Ctrl+Alt+<ANY> sends the same symbol as <ANY> (note behaviour above though)
+  - Ctrl+Alt+<ANY> sends the same symbol as <ANY>
   - Ctrl+AltGr+<ANY> sends the same symbol as AltGr+<ANY> (FIXME: broken on Win)
   - "Shift press, A press, Shift release, A release" should not send "a release"
 
@@ -128,8 +127,6 @@ Client (raw mode)
   - Multimedia keys
   - Non-character keys
 
-- CtrlL+AltR is fake released to compensate for Windows' AltGr magic [Win]
-
 Client (both modes)
 -------------------
 
diff --git a/vncviewer/Viewport.cxx b/vncviewer/Viewport.cxx
index 7af19aa7..ba0b0e93 100644
--- a/vncviewer/Viewport.cxx
+++ b/vncviewer/Viewport.cxx
@@ -115,6 +115,9 @@ static const WORD SCAN_FAKE = 0xaa;
 Viewport::Viewport(int w, int h, const rfb::PixelFormat& serverPF, CConn* cc_)
   : Fl_Widget(0, 0, w, h), cc(cc_), frameBuffer(NULL),
     lastPointerPos(0, 0), lastButtonMask(0),
+#ifdef WIN32
+    altGrArmed(false),
+#endif
     menuCtrlKey(false), menuAltKey(false), cursor(NULL)
 {
 #if !defined(WIN32) && !defined(__APPLE__)
@@ -187,6 +190,9 @@ Viewport::~Viewport()
   // Unregister all timeouts in case they get a change tro trigger
   // again later when this object is already gone.
   Fl::remove_timeout(handlePointerTimeout, this);
+#ifdef WIN32
+  Fl::remove_timeout(handleAltGrTimeout, this);
+#endif
 
   Fl::remove_system_handler(handleSystemEvent);
 
@@ -734,26 +740,6 @@ void Viewport::handleKeyPress(int keyCode, rdr::U32 keySym)
   }
 #endif
 
-#ifdef WIN32
-  // Ugly hack alert!
-  //
-  // Windows doesn't have a proper AltGr, but handles it using fake
-  // Ctrl+Alt. Unfortunately X11 doesn't generally like the combination
-  // Ctrl+Alt+AltGr, which we usually end up with when Xvnc tries to
-  // get everything in the correct state. Cheat and temporarily release
-  // Ctrl and Alt when we send some other symbol.
-  if (downKeySym.count(0x1d) && downKeySym.count(0xb8)) {
-    vlog.debug("Faking release of AltGr (Ctrl_L+Alt_R)");
-    try {
-      cc->writer()->keyEvent(downKeySym[0x1d], 0x1d, false);
-      cc->writer()->keyEvent(downKeySym[0xb8], 0xb8, false);
-    } catch (rdr::Exception& e) {
-      vlog.error("%s", e.str());
-      exit_vncviewer(e.str());
-    }
-  }
-#endif
-
   // Because of the way keyboards work, we cannot expect to have the same
   // symbol on release as when pressed. This breaks the VNC protocol however,
   // so we need to keep track of what keysym a key _code_ generated on press
@@ -777,20 +763,6 @@ void Viewport::handleKeyPress(int keyCode, rdr::U32 keySym)
     vlog.error("%s", e.str());
     exit_vncviewer(e.str());
   }
-
-#ifdef WIN32
-  // Ugly hack continued...
-  if (downKeySym.count(0x1d) && downKeySym.count(0xb8)) {
-    vlog.debug("Restoring AltGr state");
-    try {
-      cc->writer()->keyEvent(downKeySym[0x1d], 0x1d, true);
-      cc->writer()->keyEvent(downKeySym[0xb8], 0xb8, true);
-    } catch (rdr::Exception& e) {
-      vlog.error("%s", e.str());
-      exit_vncviewer(e.str());
-    }
-  }
-#endif
 }
 
 
@@ -862,6 +834,30 @@ int Viewport::handleSystemEvent(void *event, void *data)
 
     keyCode = ((msg->lParam >> 16) & 0xff);
 
+    // Windows doesn't have a proper AltGr, but handles it using fake
+    // Ctrl+Alt. However the remote end might not be Windows, so we need
+    // to merge those in to a single AltGr event. We detect this case
+    // by seeing the two key events directly after each other with a very
+    // short time between them (<50ms).
+    if (self->altGrArmed) {
+      self->altGrArmed = false;
+      Fl::remove_timeout(handleAltGrTimeout);
+
+      if (isExtended && (keyCode == 0x38) && (vKey == VK_MENU) &&
+          ((msg->time - self->altGrCtrlTime) < 50)) {
+        // FIXME: We fail to detect this if either Ctrl key is
+        //        first manually pressed as Windows then no longer
+        //        sends the fake Ctrl down event. It does however
+        //        happily send real Ctrl events even when AltGr
+        //        is already down.
+        vlog.debug("Detected AltGr combination");
+        self->handleKeyPress(0xb8, XK_ISO_Level3_Shift);
+        return 1;
+      }
+
+      self->handleKeyPress(0x1d, XK_Control_L);
+    }
+
     if (keyCode == SCAN_FAKE) {
       vlog.debug("Ignoring fake key press (virtual key 0x%02x)", vKey);
       return 1;
@@ -921,6 +917,14 @@ int Viewport::handleSystemEvent(void *event, void *data)
     if ((keySym == XK_Shift_L) && (keyCode == 0x36))
       keySym = XK_Shift_R;
 
+    // Possible start of AltGr sequence? (see above)
+    if ((keyCode == 0x1d) && (keySym == XK_Control_L)) {
+      self->altGrArmed = true;
+      self->altGrCtrlTime = msg->time;
+      Fl::add_timeout(0.1, handleAltGrTimeout, self);
+      return 1;
+    }
+
     self->handleKeyPress(keyCode, keySym);
 
     return 1;
@@ -934,6 +938,14 @@ int Viewport::handleSystemEvent(void *event, void *data)
 
     keyCode = ((msg->lParam >> 16) & 0xff);
 
+    // We can't get a release in the middle of an AltGr sequence, so
+    // abort that detection
+    if (self->altGrArmed) {
+      self->altGrArmed = false;
+      Fl::remove_timeout(handleAltGrTimeout);
+      self->handleKeyPress(0x1d, XK_Control_L);
+    }
+
     if (keyCode == SCAN_FAKE) {
       vlog.debug("Ignoring fake key release (virtual key 0x%02x)", vKey);
       return 1;
@@ -1046,6 +1058,18 @@ int Viewport::handleSystemEvent(void *event, void *data)
   return 0;
 }
 
+#ifdef WIN32
+void Viewport::handleAltGrTimeout(void *data)
+{
+  Viewport *self = (Viewport *)data;
+
+  assert(self);
+
+  self->altGrArmed = false;
+  self->handleKeyPress(0x1d, XK_Control_L);
+}
+#endif
+
 void Viewport::initContextMenu()
 {
   contextMenu->clear();
diff --git a/vncviewer/Viewport.h b/vncviewer/Viewport.h
index a4b7d8b7..3895fc7f 100644
--- a/vncviewer/Viewport.h
+++ b/vncviewer/Viewport.h
@@ -76,6 +76,10 @@ private:
 
   static int handleSystemEvent(void *event, void *data);
 
+#ifdef WIN32
+  static void handleAltGrTimeout(void *data);
+#endif
+
   void initContextMenu();
   void popupContextMenu();
 
@@ -94,6 +98,11 @@ private:
   typedef std::map<int, rdr::U32> DownMap;
   DownMap downKeySym;
 
+#ifdef WIN32
+  bool altGrArmed;
+  unsigned int altGrCtrlTime;
+#endif
+
   rdr::U32 menuKeySym;
   int menuKeyCode, menuKeyFLTK;
   Fl_Menu_Button *contextMenu;