After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776031 - W32: Winkey+down minimizes maximized window instead of restoring it
W32: Winkey+down minimizes maximized window instead of restoring it
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
3.22.x
Other Windows
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-12-13 06:58 UTC by LRN
Modified: 2017-01-10 12:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GDK W32: Use keyboard hook to detect AeroSnap combinations better v1 (7.95 KB, patch)
2016-12-25 17:47 UTC, LRN
none Details | Review
GDK W32: Use keyboard hook to detect AeroSnap combinations better v2 (7.95 KB, patch)
2017-01-06 10:21 UTC, LRN
committed Details | Review

Description LRN 2016-12-13 06:58:37 UTC
How to reproduce in gtk-3-22:
* Maximize a window
* Press Winkey+down

Expected result:
* The window is restored

Actual result:
* The window is restored, then immediately minimized

Cause:
The GTK+ Aerosnap code was written with the assumption that Windows WM *does not* perform any Aerosnap actions in response to Winkey+<something> on layered GTK+ windows. While debugging a different bug, i've discovered that on Windows 10 that is not true: using Winkey+down in a maximized state does result in Windows WM restoring the window on its own volition (an appropriate WM_SYSCOMMAND is received). Then GTK+ Aerosnap code kicks in and responds to Winkey+down by minimizing the window, since by that point it is not maximized anymore.

If anyone has gtk+-3.22.4 and Windows 7 and/or 8, i'd like to ask to confirm that this does or does not occur. Depending on that we should either remove custom Winkey+down handling on maximized windows unconditionally, or do so only on Windows 10.
Comment 1 Patrick Griffis (tingping) 2016-12-19 07:45:41 UTC
I cannot reproduce using gtk3-demo version 3.22.4 from msys2 on Windows 8.1.

The maximized window is just returned to the unmaximized state.
Comment 2 LRN 2016-12-19 07:46:39 UTC
From IRC earlier:

nacho: LRN, win+arrow down the windows is restored
nacho: LRN, when restored, win+array down again, then it goes to the task bar


Therefore my conclusion is that this is Winodws 10-only.
Comment 3 LRN 2016-12-19 16:44:50 UTC
What a nasty bug it turned out to be.

It seems that while Windows doesn't do most AeroSnap actions on our windows, it still catches and handles all winkey combinations - meaning that we get messages in this order:
* winkey is down
* downarrow is up
* winkey is up
Note the absence of "downarrow is down" message - because Windows WM intercepts it. Also, for windows where AeroSnap *does* work, it's triggered on the arrow key being down, not up.

Because of this it is somewhat difficult to ignore some instances of winkey combinations, because by the time we get "downarrow is up" message, Windows WM already handled the winkey+down combination and restored the window (thus we can't distinguish between "winkey+down on a non-maximized window" and "winkey+down on a maximized window").

One possible way of working around this is to install a low-level keyboard hook and use it to catch keypresses. The hook is invoked before Windows WM gets the chance to intercept the key being pressed.

After that point things get a bit fuzzy. I think that i can reliably detect keypresses that affect GDK windows (as opposed to other windows in the session - the hook is global and can't be installed on one thread only). Currently my best guess is that we should do winkey combination detection in the hook callback, regardless of which window has keyboard focus. Then, when a winkey combination is pressed, we should post (not send) a custom message to the window, if it's one of ours. And we code our windows to do our own AeroSnap only when this message is received, instead of catching WM_KEYUP.
Comment 4 LRN 2016-12-25 17:47:30 UTC
Created attachment 342465 [details] [review]
GDK W32: Use keyboard hook to detect AeroSnap combinations better v1

Windows WM handles AeroSnap for normal windows on keydown. We did this
on keyup only because we do not get a keydown message, even if Windows WM
does nothing with a combination. However, in some specific cases it DOES
do something - and we have no way to detect that. Specifically, winkey+downarrow
causes maximized window to be restored by WM, and GDK fails to detect that. Then
GDK gets a keyup message, figures that winkey+downarrow was pressed and released,
and handles the combination - by minimizing the window.

To overcome this, install a low-level keyboard hook (high-level ones have
the same problem as normal message loop - they don't get messages when
Windows WM handles combinations) and use it to detect interesting key combinations
before Windows WM has a chance to block them from being processed.

Once an interesting combination is detected, post a message to the window, which
will be handled in due order.

It should be noted that this code handles key repetitions in a very crude manner.

The downside is that AeroSnap will not work if hook installation function call fails.
Also, this is a global hook, and if the hook procedure does something wrong, bad things
can happen.
Comment 5 Ignacio Casal Quinteiro (nacho) 2017-01-06 10:10:12 UTC
Review of attachment 342465 [details] [review]:

See the minor comments.

::: gdk/win32/gdkevents-win32.c
@@ +301,3 @@
 
+static LRESULT
+check_keystroke_for_interesting_stuff (WPARAM message,

I definitely do not like this name, can you be more precise or factor it out?

@@ +2403,3 @@
 
+  if (msg->message == aerosnap_message)
+    {

no need for brackets for single line
Comment 6 LRN 2017-01-06 10:21:45 UTC
Created attachment 343002 [details] [review]
GDK W32: Use keyboard hook to detect AeroSnap combinations better v2

* Renamed the function in question to "low_level_keystroke_handler"
* Removed unneded braces, wrapped the line while at it
Comment 7 Ignacio Casal Quinteiro (nacho) 2017-01-09 14:16:17 UTC
Review of attachment 343002 [details] [review]:

Let's get it in. If it produces some problem we can always fix it.
Comment 8 Fan, Chun-wei 2017-01-10 06:58:56 UTC
Hi,

I'd say go ahead too, it's not fun but it seems it has to go this way, and we probably need to see whether future Windows 10 updates change things...

Patrick, can you try out with this new patch on Windows 8.1 (or if someone has 7, can he try it out too)?

With blessings, thank you, and cheers!
Comment 9 LRN 2017-01-10 12:56:29 UTC
Tested this in a Windows 7 VM. Winkey combinations seem to be working.

Attachment 343002 [details] pushed into gtk-3-22 as eece8a7dd2405f76829031f3d6dd5e39fb5dc542