Tk Source Code

View Ticket
Login
Ticket UUID: 1fb7af623ac45badc115df0b3515a4e552400508
Title: Add support for buttons 4 and 5 to Windows
Type: Patch Version: core-8-6-branch
Submitter: chrstphrchvz Created on: 2019-07-19 05:06:37
Subsystem: 01. Bindings Assigned To: fvogel
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2020-06-22 13:19:21
Resolution: Fixed Closed By: chrstphrchvz
    Closed on: 2020-06-22 13:19:21
Description:

The attached patch attempts to add support for mouse buttons 4 and 5 to Tk for Windows. (These buttons are called XButton1 and XButton2 in the Windows API documentation.) Currently, Tk programs on Windows do not detect these buttons at all, even with <ButtonPress>. I believe that these buttons, if not repurposed by the user, should be available to Tk programs, as they currently are on Aqua (as buttons 4 and 5) and X11 (as buttons 8 and 9). One common binding for these buttons is for back/forward navigation in browsers.

I am not sure to what extent this functionality has been requested in the past, or whether any efforts were made to implement support for them (either in Tk or extensions). I found one discussion from 2011 on comp.lang.tcl: https://groups.google.com/forum/#!msg/comp.lang.tcl/LpEULzrhjBI/gbNOyXyYMX4J. The non-functionality of buttons 4 and 5 on Windows was more recently mentioned in discussions regarding TIP #474.

I believe support for these buttons can be added to 8.6, and not have to wait for 8.7. There is indeed a likelihood that existing programs bind to buttons 4 and 5 to allow scrolling on X11, yet fail to restrict those bindings to when [tk windowingsystem] is "x11"; however, those programs already exhibit this issue on Aqua. (I informed the author of TIP #474 that their existing implementation would exhibit this issue. Edit: maybe it doesn't affect Windows…)

There is one detail in the implementation I am not very certain of: whether wparam should only be assigned MK_XBUTTON1 or MK_XBUTTON2, rather than MAKEWPARAM(MK_XBUTTON1, XBUTTON1) or MAKEWPARAM(MK_XBUTTON2, XBUTTON2), in the following:

@@ -1783,6 +1787,14 @@ TkWinResendEvent(
        msg = WM_RBUTTONDOWN;
        wparam = MK_RBUTTON;
        break;
+    case Button4:
+       msg = WM_XBUTTONDOWN;
+       wparam = MAKEWPARAM(MK_XBUTTON1, XBUTTON1);
+       break;
+    case Button5:
+       msg = WM_XBUTTONDOWN;
+       wparam = MAKEWPARAM(MK_XBUTTON2, XBUTTON2);
+       break;
     default:
        return 0;
     }

I do not know exactly what TkWinResendEvent() is used for, but I imagine that the way I have written this allows whatever processing the event to check which button is pressed from HIWORD(wparam) (e.g. using the GET_XBUTTON_WPARAM(wparam) macro), rather than only by checking for MK_XBUTTON1 or MK_XBUTTON2.

One script for manually testing this:

package require Tk

label .l -text "Click here" -height 10 -width 30 pack .l bind .l <4> {puts "Button 4 clicked"} bind .l <5> {puts "Button 5 clicked"}

To test this without an actual 5-button mouse, a tool to emulate the button presses can be used, such as AutoHotKey with a script that maps keypresses to buttons 4 and 5:

F6::XButton1

F7::XButton2

I have successfully tested this implementation on Windows 10 1903 x64, based on top of recent core-8-6-branch [3e5c0ebb].

User Comments: chrstphrchvz added on 2020-06-22 13:19:21:

Francois, thank you for specifying which model mouse it is, that clears things up for me. From https://www.logitech.com/assets/49085/wireless-mouse-m560-quick-start-guide.pdf it turns out the default tilt wheel behavior behavior on Windows 8 (and higher, presumably) is to do browser back/forward, i.e. <4>/<5> in Tk 8.6.10, which is not what I expected. The default behavior it describes for Windows 7, where the tilt wheel scrolls horizontally and the shoulder buttons do browser back/forward, was what I expected. Maybe the Logitech SetPoint utility has a way to customize for anyone who really wanted the Windows 7 behavior.


fvogel added on 2020-06-22 08:07:03:

I completely overlooked this remark; this is not a result that I expected. I don't think I ever made it clear here that the buttons in question are typically shoulder buttons on premium mice, not tilt wheels. As of 8.6.10, a tilt wheel should produce <Shift-MouseWheel> events on Windows: see [eb29967e88]. If that mouse produces <4> and <5> on that system, then I wouldn't expect it to be able to do horizontal scrolling in e.g. a web browser.
Correct. This mouse is a Logitech M560 used with Win 10. I confirm tilting its wheel triggers <4> (tilt toward the left) or <5> (tilt toward the right). Tilting indeed does not allow to scroll a web page but is bound to previous/next page in the browser history (atleast with IE11).


chrstphrchvz added on 2020-06-22 00:09:15:

<fvogel> One of my systems is equipped with Windows 10 and a mouse which wheel can be tilted, and indeed with the patch titing left triggers <Button-4> and tilting right triggers <Button-5>.

I completely overlooked this remark; this is not a result that I expected. I don't think I ever made it clear here that the buttons in question are typically shoulder buttons on premium mice, not tilt wheels. As of 8.6.10, a tilt wheel should produce <Shift-MouseWheel> events on Windows: see [eb29967e88]. If that mouse produces <4> and <5> on that system, then I wouldn't expect it to be able to do horizontal scrolling in e.g. a web browser.


jan.nijtmans added on 2019-07-25 07:19:21:
So, after some experimenting, I'm convinced too.

Thanks!  Merged to 8.6 and trunk.

jan.nijtmans added on 2019-07-24 12:18:25:
Yes, there are 2 choices. Either use 8/9 for the Xbuttons, and keep Buttons 4-8 for X11 only (horizontal/vertical scroll wheel), either use 4/5 for the Xbuttons and move the scroll wheeel on X11 somewhere else.

My patch investigates the first solution, but I didn't make up my mind yet for the final decision. I agree with your arguments: Button 4/5 would make more sense on Windows and Mac, 8/9 makes more sense on X11

chrstphrchvz added on 2019-07-24 11:02:30:

About your question regarding whether wparam should only be assigned MK_XBUTTON1 or MK_XBUTTON2, rather than MAKEWPARAM(MK_XBUTTON1, XBUTTON1) or MAKEWPARAM(MK_XBUTTON2, XBUTTON2), yes it is indeed a bit unclear. I think (from reading the MSDN documentation that your code should be fine as is. Moreover, since you tested it and it works it should be even finer, isn't it?

Both approaches worked when I tried them, so I haven't concluded the MAKEWPARAM(…) approach is necessary, even though I imagine it is safer.

Actually, I now think the reason I could not tell whether the choice of MAKEWPARAM(…) vs XBUTTON1/XBUTTON2 made a difference is that I've probably not exercised the change to TkWinResendEvent() at all; leaving it out that change entirely didn't seem any different.


chrstphrchvz added on 2019-07-24 10:52:43:

Thanks for the feedback, Jan.

I haven't figured out whether I agree these buttons should be immediately assigned 8 and 9 on Windows instead of 4 and 5. I posted on Tcl-Core to try discussing some higher-level issues on when and how platform-neutral mouse behaviors should be released (my hope is they can all be released simultaneously, as I think that would be the least confusing for the outside world).

I think some of my confusion is over whether:

  1. Tk views X11 as the "flagship" or "authoritative" platform that others should emulate, or
  2. it views X11 emulation just as the pragmatic way of working on non-X11.
(I understand it is likely not strictly one view or the other.)

With the former, programs should see X11 conventions; whereas the with the latter, programs should not necessarily even be aware there is X11 emulation. The former has platform neutrality as a byproduct of adherence to X11 conventions, while the latter can instead go with decidedly non-X11 abstractions.

An example of what the latter view enables is that, if all platforms had consistent button numbering and used <MouseWheel> for scrolling, then instead of leaving buttons 4-7 unused on all platforms, it may be less strange to reassign buttons 8 and higher on X11 to buttons beginning at 4.

I also think that TIP #474's proposed solution of using <MouseWheel> on X11 is toward having non-X11 abstractions per the latter view. I think the former view of making non-X11 platforms closely emulate X11 would instead suggest eliminating <MouseWheel> and have all platforms use 4-7 for scrolling.

Regarding this patch, I concede that having these buttons as 8 and 9 is probably better than not having them at all. (Though, playing devil's advocate might argue that supporting buttons beyond the left and sometimes right button is bad—on any platform—because it allows programs to be written containing functionality that might depend on unusual hardware, with 4th and 5th buttons being even more uncommon than middle buttons. I imagine Tk and other toolkits reject this argument by erring on the side of empowering rather than restricting program authors. Still, I notice there have been tickets filed for issues using older Tk programs, written in the age of three button mice, that couldn't forsee a future of Macs, inexpensive laptops, and touchscreen devices that lack a built-in way of emulating these buttons. Fixing bindings in existing programs, especially dormant or abandoned ones, I imagine is often easier said than done.)

I guess a few reasons I stuck with 4 and 5 for my patch are:

  1. that there are other Windows apps and widget toolkits that already call these buttons 4 and 5, under the idea that Tk 8.6 reflects rather than abstracts button numbers, so without documenting their availability under a more explicit "cross-platform" moniker, using 8 and 9 is potentially confusing to those more familiar with Windows than X11;
  2. that 4 and 5 seemed like the only obvious and easy options to use in the implementation, as the existing behavior in Windows (and Aqua) is to update the xbutton.state bitmask (which only appears to accommodate buttons 1-5), rather than ever returning the button as a plain integer (though Jan's recent branch for [38dc27bd] probably illustrates what sort of restructuring is needed to address this); and
  3. because 4 and 5 are "first class citizens" in terms of being as easy for programs to bind to as buttons 1-3 using "numbered" events (as described in [38dc27bd]).


jan.nijtmans added on 2019-07-23 07:44:46:

See also: [38dc27bd1d0ecd682aaf]: It appears that Buttons 6 and 7 are used on X11 as horizontal scroll wheel, so Button 8 and 9 are a logical choice for those XButton1/XButton2 buttons.


jan.nijtmans added on 2019-07-23 07:00:11:
Thanks, all, for this work. Yes, I think that support for those buttons should be added. But ....

Since on X11 those buttons already appear to workas Buttons 8 and 9, and the Windows and OSX implemenetation are only wrappers providing a simulated X11 environment: I would propose to change this patch to map the Win32 events to buttons 8 and 9 instead of Buttons 4 and 5. No problem to do that for Tk 8.6: It already works on X11, such a change makes it work on Win32 too.

Advantage: Since this makes Win32 works as X11, it's only a portability bug, which can be done without a TIP, IMHO: The reason for the possible TIP would be to reason about the conflict with Buttons 4 and 5 on X11, but if there is no such conflict there is nothing to discuss about.

Then, since the current Mac implementation maps those buttons to Button 4 and 5, I think that's actually wrong. So, yes, I would expect a TIP (and implementation) changing the OSX situation. That would resolve the button conflicts on all platforms: Button 4 and 5 can be kept as mousewheel buttons on X11 and unused on other platforms. That would free the way for TIP #474.

fvogel added on 2019-07-22 20:19:13:
You have convinced me. Any other opinions perhaps?

chrstphrchvz added on 2019-07-22 08:14:46:

Thank you for testing this, providing feedback, and helping shepherd this, François.

I thought we should have some new tests exercising <Button-4> and <Button-5>, so I had a look at what we already have first. It turns out that some tests already do this, e.g. bind-27.6 and 27.7. These particular tests are not constrained to any specific platform and they do not fail before your patch. This works because the event is artificially generated by Tk, not by the OS, whereas your patch plugs at the OS level. Not sure we can add new tests really checking your patch, what do you think?

For reasons similar to my remark in [eb29967e], such a test seems possible, but it would probably take a while for me to cargo-cult one together myself, since there doesn't appear to be an existing test for mouse buttons 1-3 to easily adapt.

…aside of Windows, do you know what is the exact status for <Button-4>, <Button-5>, <ButtonPress> on Linux and macOS? What I have in sight here is cross-platofrm consistency, of course.

My guess is that Tk opted to preserve the numbering used by each underlying windowing system, rather than abstract it away. As a result, the only physical button with a consistent software numbering on all platforms is button 1. Admittedly, the goal of this patch so far is not to address the inconsistency of the buttons' numbering, but I believe that is a valid issue and might deserve an RFE. (I already opened one regarding whether there should be a cross-platform way of using the middle and right mouse buttons: [6ff8f7b0].)

The 4th and 5th physical mouse buttons are already available in Aqua and X11: they correspond to software buttons 4 and 5 on Aqua, and buttons 8 and 9 on X11 (at least by default in many modern OS' distributions of X11; long ago, these buttons might have required manual configuration; and XQuartz currently translates any button event from Aqua which isn't the left or right button to act as the middle button, i.e. XQuartz programs see them all as button 2). With this patch, the numbering of buttons 4 and 5 on Windows will at least be with consistent with Aqua.

Software buttons 1-5, regardless of their physical purpose, are available in as "numbered" events like <ButtonPress-1>, <ButtonPress-2>, etc. Any button including those numbered above 5 can be used through "unnumbered" events, like <ButtonPress> or <ButtonRelease>, and checking the value of %b.

About your question regarding whether wparam should only be assigned MK_XBUTTON1 or MK_XBUTTON2, rather than MAKEWPARAM(MK_XBUTTON1, XBUTTON1) or MAKEWPARAM(MK_XBUTTON2, XBUTTON2), yes it is indeed a bit unclear. I think (from reading the MSDN documentation that your code should be fine as is. Moreover, since you tested it and it works it should be even finer, isn't it?

Both approaches worked when I tried them, so I haven't concluded the MAKEWPARAM(…) approach is necessary, even though I imagine it is safer.

Last point I wanted to mention: I see this patch as the implementation of a new feature, therefore merging it will require a TIP in my opinion.

I think it's understandable that this feature might normally meet the threshold for a TIP (the earlier discussion I found mentioned a TIP being a likely requirement), so I'm not completely opposed to going through that process. However, I would argue that this patch improves feature parity between platforms, rather than provides a completely new or platform-specific feature, or diverges from other platforms. Aqua and X11 already provide access to these buttons; Windows is the odd one out here, and this patch addresses that situation. To my knowledge no TIPs were required to support these buttons on Aqua or X11 (although I imagine whatever changes that brought support for these buttons on Aqua or X11 might not have deliberately intended to do so). Should I ask on Tcl-Core if a TIP is necessary in this case?


fvogel added on 2019-07-21 15:10:56:
Test result:

One of my systems is equipped with Windows 10 and a mouse which wheel can be tilted, and indeed with the patch titing left triggers <Button-4> and tilting right triggers <Button-5>.

fvogel added on 2019-07-21 09:26:43:

I have committed your patch in branch bug-1fb7af623a for easier testing.

Thanks for your checks with the proposed implementation of TIP #474. I also believe this implementation is a bit incorrect. As for fixing it, I don't think it's desirable to add platform-specific code in generic files, but this is another discussion that should happen in the frame of TIP #474.

Back to your patch here in this ticket. I thought we should have some new tests exercising <Button-4> and <Button-5>, so I had a look at what we already have first. It turns out that some tests already do this, e.g. bind-27.6 and 27.7. These particular tests are not constrained to any specific platform and they do not fail before your patch. This works because the event is artificially generated by Tk, not by the OS, whereas your patch plugs at the OS level. Not sure we can add new tests really checking your patch, what do you think?

I didn't try so far (I lack a readily available test script...): aside of Windows, do you know what is the exact status for <Button-4>, <Button-5>, <ButtonPress> on Linux and macOS? What I have in sight here is cross-platofrm consistency, of course.

About your question regarding whether wparam should only be assigned MK_XBUTTON1 or MK_XBUTTON2, rather than MAKEWPARAM(MK_XBUTTON1, XBUTTON1) or MAKEWPARAM(MK_XBUTTON2, XBUTTON2), yes it is indeed a bit unclear. I think (from reading the MSDN documentation that your code should be fine as is. Moreover, since you tested it and it works it should be even finer, isn't it?

Last point I wanted to mention: I see this patch as the implementation of a new feature, therefore merging it will require a TIP in my opinion.


chrstphrchvz added on 2019-07-20 15:00:13:

I have reuploaded the patch as ASCII with LF line endings, which hopefully should make it easier to apply (the earlier upload had UCS-2LE with CRLF line endings).


chrstphrchvz added on 2019-07-20 14:52:32:

I'm a bit unsure about the impact of this improvement on TIP #474, which in its implementation (in the generic code, see [77c390bae2700c72]) turns button-4 and button-5 into a <MouseWheel> event.

Here's the thread with my feedback to the author of TIP #474, for reference: https://sourceforge.net/p/tcl/mailman/message/36628988/

The issue that I predicted, and observed on Aqua when trying the TIP #474 implementation, was that <4> and <5> would be converted to <MouseWheel>, and no longer seen as <4> or <5> by the program. Essentially, clicking the extra buttons would become equivalent to rotating the scrollwheel by one or more notches on non-X11. I strongly suspected this issue also applied to Windows, but had not confirmed it; and I wasn't yet aware that I wouldn't be able to confirm it, due to support for buttons 4 and 5 not even being implemented on Windows.

However, after properly trying both my patch and the TIP #474 implementation simultaneously, I don't observe the issue on Windows, as I had on Aqua; the program still sees <4> and <5> instead of <MouseWheel>. I guess the Windows code never gets to the part of UpdateButtonEventState() affected by TIP #474. But it's still probably clearer/safer to make sure that any <4> or <5> usage in generic code checks that the windowingsystem is X11 if those buttons are being used for scrolling.


fvogel added on 2019-07-20 13:29:16:

This looks nice at a first cursive sight.

I'm a bit unsure about the impact of this improvement on TIP #474, which in its implementation (in the generic code, see [77c390bae2700c72]) turns button-4 and button-5 into a <MouseWheel> event.


Attachments: