Re: [PATCH] ui/cocoa: Do not raise keys before QEMU resigns active

From: BALATON Zoltan <balaton@eik.bme.hu>
To: Akihiko Odaki <akihiko.odaki@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [PATCH] ui/cocoa: Do not raise keys before QEMU resigns active
Date: Sun, 14 Mar 2021 13:55:22 +0100 (CET)	[thread overview]
Message-ID: <924e0f6-1981-8c7f-4568-a03c2b17d02c@eik.bme.hu> (raw)
In-Reply-To: <20210314070147.34731-1-akihiko.odaki@gmail.com>

On Sun, 14 Mar 2021, Akihiko Odaki wrote:
> ui/cocoa used to raise all keys before it resigns active to prevent a
> stuck key problem caused by key up events it does not see while it is
> inactive. The problem is solved by checking -[NSEvent modifierFlags] in
> commit 6d73bb643aa725348aabe6a885ac5fb0b7f70252, which is better
> because it handles the case that key *down* events are missed while it
> is inactive.

Does that commit also handle other keys than modifier keys? It's unlikely 
those get stuck but is that possible? The most likely case to happen is 
pressing Cmd+Tab to switch away and Cmd may get stuck. Can Tab get also 
stuck? Or what if you hold down space or some other key while switching 
away either with Cmd+Tab or with the mouse? That may not be a common case 
but is this here to handle that or they are already handled elsewhere so 
this is really not necessary to prevent stuck keys?

Regards,
BALATON Zoltan

> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> ui/cocoa.m | 20 --------------------
> 1 file changed, 20 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index a7848ae0a30..ac8989947f5 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -326,7 +326,6 @@ - (BOOL) isAbsoluteEnabled;
> - (float) cdx;
> - (float) cdy;
> - (QEMUScreen) gscreen;
> -- (void) raiseAllKeys;
> @end
>
> QemuCocoaView *cocoaView;
> @@ -996,18 +995,6 @@ - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
> - (float) cdx {return cdx;}
> - (float) cdy {return cdy;}
> - (QEMUScreen) gscreen {return screen;}
> -
> -/*
> - * Makes the target think all down keys are being released.
> - * This prevents a stuck key problem, since we will not see
> - * key up events for those keys after we have lost focus.
> - */
> -- (void) raiseAllKeys
> -{
> -    with_iothread_lock(^{
> -        qkbd_state_lift_all_keys(kbd);
> -    });
> -}
> @end
>
>
> @@ -1143,13 +1130,6 @@ - (BOOL)windowShouldClose:(id)sender
>     return NO;
> }
>
> -/* Called when QEMU goes into the background */
> -- (void) applicationWillResignActive: (NSNotification *)aNotification
> -{
> -    COCOA_DEBUG("QemuCocoaAppController: applicationWillResignActive\n");
> -    [cocoaView raiseAllKeys];
> -}
> -
> /* We abstract the method called by the Enter Fullscreen menu item
>  * because Mac OS 10.7 and higher disables it. This is because of the
>  * menu item's old selector's name toggleFullScreen:
>