Skip to contentSkip to conversation

ui/cocoa.m: prevent stuck command key when going into full screen mode

PATCH Superseded John Arbuckle 3 participants 4 messages Jun 19, 2018 – Jul 1, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ui/cocoa.m: prevent stuck command key when going into full screen mode

When the user pushes Command-F in QEMU while the mouse is ungrabbed, QEMU goes into full screen mode. When the user finally releases the command key, it is sent to the guest as an event. The makes the guest operating system think the command key is down when it is really up. To prevent this situation from happening, we simply drop the first command key event after the user has gone into full screen mode using Command-F.

---
 ui/cocoa.m | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
ui/cocoa.m +17 -1
index 2991ed4..024aba2 100644
@@ -287,6 +287,7 @@ static void handleAnyDeviceErrors(Error * err)
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
     BOOL isMouseDeassociated;
+    BOOL prevent_stuck_command_key;
 }
 - (void) switchSurface:(DisplaySurface *)surface;
 - (void) grabMouse;
@@ -330,7 +331,7 @@ QemuCocoaView *cocoaView;
         screen.bitsPerPixel = 32;
         screen.width = frameRect.size.width;
         screen.height = frameRect.size.height;
-
+        prevent_stuck_command_key = NO;
     }
     return self;
 }
@@ -552,6 +553,14 @@ QemuCocoaView *cocoaView;
 }
 
 - (void) toggleModifier: (int)keycode {
+
+    /* Prevents the command key from being sent to the guest */
+    if ((keycode == Q_KEY_CODE_META_L || keycode == Q_KEY_CODE_META_R) &&
+        prevent_stuck_command_key == YES) {
+        prevent_stuck_command_key = NO;
+        return;
+    }
+
     // Toggle the stored state.
     modifiers_state[keycode] = !modifiers_state[keycode];
     // Send a keyup or keydown depending on the state.
@@ -691,6 +700,13 @@ QemuCocoaView *cocoaView;
 
             // forward command key combos to the host UI unless the mouse is grabbed
             if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
+                /*
+                 * Prevent the command key from being stuck down in the guest
+                 * when using Command-F for full screen mode
+                 */
+                if (keycode == Q_KEY_CODE_F) {
+                    prevent_stuck_command_key = YES;
+                }
                 [NSApp sendEvent:event];
                 return;
             }
Signature · 1 line
2.7.2
#1 lore raw
This series failed build test on s390x host. Please find the details below.

Hi,

This series failed build test on s390x host. Please find the details below.

N/A. Internal error while reading log file

---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [email protected]

#2 lore raw
In reply to John Arbuckle
Quoted 3 lines · John Arbuckle

When the user pushes Command-F in QEMU while the mouse is ungrabbed, QEMU goes into full screen mode. When the user finally releases the command key, it is sent to the guest as an event.

What is "it" here? This sentences implies that "it" is the key-release event for the command key, but the following sentence suggests it's the key-press event.

Quoted 61 lines

The makes the guest operating system
think the command key is down when it is really up. To prevent this situation from happening, we simply drop the first command key event after the user has gone into full screen mode using Command-F.

---
 ui/cocoa.m | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
ui/cocoa.m +17 -2
index 2991ed4..024aba2 100644
@@ -287,6 +287,7 @@ static void handleAnyDeviceErrors(Error * err)
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
     BOOL isMouseDeassociated;
+    BOOL prevent_stuck_command_key;
 }
 - (void) switchSurface:(DisplaySurface *)surface;
 - (void) grabMouse;
@@ -330,7 +331,7 @@ QemuCocoaView *cocoaView;
         screen.bitsPerPixel = 32;
         screen.width = frameRect.size.width;
         screen.height = frameRect.size.height;
-
+        prevent_stuck_command_key = NO;
     }
     return self;
 }
@@ -552,6 +553,14 @@ QemuCocoaView *cocoaView;
 }
 
 - (void) toggleModifier: (int)keycode {
+
+    /* Prevents the command key from being sent to the guest */
+    if ((keycode == Q_KEY_CODE_META_L || keycode == Q_KEY_CODE_META_R) &&
+        prevent_stuck_command_key == YES) {
+        prevent_stuck_command_key = NO;
+        return;
+    }
+
     // Toggle the stored state.
     modifiers_state[keycode] = !modifiers_state[keycode];
     // Send a keyup or keydown depending on the state.
@@ -691,6 +700,13 @@ QemuCocoaView *cocoaView;
 
             // forward command key combos to the host UI unless the mouse is grabbed
             if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
+                /*
+                 * Prevent the command key from being stuck down in the guest
+                 * when using Command-F for full screen mode
+                 */
+                if (keycode == Q_KEY_CODE_F) {
+                    prevent_stuck_command_key = YES;
+                }
                 [NSApp sendEvent:event];
                 return;
             }
--
Quoted 1 line

2.7.2

I can sort of see why this might work, but the flow of what's going wrong isn't clear from your commit message, and the change looks really inelegant.

thanks
-- PMM

#3 lore raw
Quoted 8 lines · Peter Maydell
Quoted 3 lines · John Arbuckle

When the user pushes Command-F in QEMU while the mouse is ungrabbed, QEMU goes into full screen mode. When the user finally releases the command key, it is sent to the guest as an event.

What is "it" here? This sentences implies that "it" is the key-release event for the command key, but the following sentence suggests it's the key-press event.

It is the key release event for the command key.

Quoted 58 lines
Quoted 58 lines

The makes the guest operating system
think the command key is down when it is really up. To prevent this situation from happening, we simply drop the first command key event after the user has gone into full screen mode using Command-F.

---
ui/cocoa.m | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)
ui/cocoa.m +17 -4
index 2991ed4..024aba2 100644
@@ -287,6 +287,7 @@ static void handleAnyDeviceErrors(Error * err)
    BOOL isFullscreen;
    BOOL isAbsoluteEnabled;
    BOOL isMouseDeassociated;
+    BOOL prevent_stuck_command_key;
 }
- (void) switchSurface:(DisplaySurface *)surface;
- (void) grabMouse;
@@ -330,7 +331,7 @@ QemuCocoaView *cocoaView;
        screen.bitsPerPixel = 32;
        screen.width = frameRect.size.width;
        screen.height = frameRect.size.height;
-
+        prevent_stuck_command_key = NO;
    }
    return self;
 }
@@ -552,6 +553,14 @@ QemuCocoaView *cocoaView;
 }
 
- (void) toggleModifier: (int)keycode {
+
+    /* Prevents the command key from being sent to the guest */
+    if ((keycode == Q_KEY_CODE_META_L || keycode == Q_KEY_CODE_META_R) &&
+        prevent_stuck_command_key == YES) {
+        prevent_stuck_command_key = NO;
+        return;
+    }
+
    // Toggle the stored state.
    modifiers_state[keycode] = !modifiers_state[keycode];
    // Send a keyup or keydown depending on the state.
@@ -691,6 +700,13 @@ QemuCocoaView *cocoaView;
 
            // forward command key combos to the host UI unless the mouse is grabbed
            if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
+                /*
+                 * Prevent the command key from being stuck down in the guest
+                 * when using Command-F for full screen mode
+                 */
+                if (keycode == Q_KEY_CODE_F) {
+                    prevent_stuck_command_key = YES;
+                }
                [NSApp sendEvent:event];
Quoted 8 lines
Quoted 4 lines
ui/cocoa.m +0 -1
                return;
            }
--

2.7.2

I can sort of see why this might work, but the flow of what's going wrong isn't clear from your commit message, and the change looks really inelegant.

What would be a more suitable solution to this problem? I realize using a flag to indicate this situation isn't the prettiest, but it does get the job done.