notifyicon.go: improve NIN_KEYSELECT handling#140
Conversation
When the keyboard is used to select an icon in the taskbar notification area, Windows sends one or two NIN_KEYSELECT messages depending on which key was used. This second message was confusing walk because it was being interpreted as a request to cancel the menu. Now, upon encountering the first NIN_KEYSELECT, we set a message-based timer whose duration equals the system's double-click time. If a second NIN_KEYSELECT message is sent while the timer has yet to lapse, we ignore it. This isn't exactly a _great_ fix, but considering the fragility of sending multiple messages to indicate a particular keypress, it will hopefully suffice. I'm also tacking on a couple of minor tweaks to application.go. Updates tailscale/tailscale#17438 Signed-off-by: Aaron Klotz <aaron@tailscale.com>
b034f59 to
53ec33a
Compare
| ni.messageClickedPublisher.Publish() | ||
|
|
||
| case win.WM_TIMER: | ||
| if wParam == win.NIN_KEYSELECT { |
There was a problem hiding this comment.
Blocking: IIUC, this code only runs when ni.wndProc is explicitly invoked on line 81, and we replace the original timer ID with uintptr(tid.notificationCode()) at the call site. So I think this should work fine.
At the same time, as written, it looks like a normal message handler, so comparing wParam (which, again, should be the timer identifier for a normal WM_TIMER message) to win.NIN_KEYSELECT is confusing.
I think we should structure this differently and perhaps plumb the timer ID all the way here, so we can compare it against ni.keySelectTimerID.
But if I'm missing something and that's not possible (or not a good idea), could you please document that this is a fake message? Specifically, IIUC, the handler will not be invoked for any real WM_TIMER messages, and the only expected wParam value is win.NIN_KEYSELECT, so the check is fine.
When the keyboard is used to select an icon in the taskbar notification area, Windows sends one or two NIN_KEYSELECT messages depending on which key was used. This second message was confusing walk because it was being interpreted as a request to cancel the menu.
Now, upon encountering the first NIN_KEYSELECT, we set a message-based timer whose duration equals the system's double-click time. If a second NIN_KEYSELECT message is sent before the timer has lapsed, we ignore it. This isn't exactly a great fix, but considering the fragility of sending multiple messages to indicate a particular keypress, it will hopefully suffice.
I'm also tacking on a couple of minor tweaks to application.go.
Updates tailscale/tailscale#17438