We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2310 - description for SDL_MouseButtonEvent.button is misleading
Summary: description for SDL_MouseButtonEvent.button is misleading
Status: RESOLVED DUPLICATE of bug 2472
Alias: None
Product: SDL
Classification: Unclassified
Component: events (show other bugs)
Version: 2.0.3
Hardware: x86_64 Linux
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-16 12:12 UTC by Stijn van Drongelen
Modified: 2015-02-19 05:30 UTC (History)
4 users (show)

See Also:

Attachments
Simple test to print the mouse button number (1.44 KB, text/plain)
2015-01-27 17:31 UTC, Daniel Gibson 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Stijn van Drongelen 2013-12-16 12:12:02 UTC 
See http://hg.libsdl.org/SDL/file/405d84eedad8/src/video/x11/SDL_x11events.c#l711:

    case ButtonPress:{
            int ticks = 0;
            if (X11_IsWheelEvent(display,&xevent,&ticks)) {
                SDL_SendMouseWheel(data->window, 0, 0, ticks);
            } else {
                SDL_SendMouseButton(data->window, 0, SDL_PRESSED, xevent.xbutton.button);
            }
        }
        break;

    case ButtonRelease:{
            SDL_SendMouseButton(data->window, 0, SDL_RELEASED, xevent.xbutton.button);
        }
        break;

In the case of my own Logitech M570 mouse, which apparently has 20 buttons, X calls my "up" and "down" button 8 and 9. Because SDL blindly copies these IDs, the documentation of SDL_MouseButtonEvent.button ("may be one of SDL_BUTTON_*") is now meaningless. Users who depend on this to mean that button can only be one of those five values (e.g. the Haskell bindings for SDL2, a work in progress) will end up crashing.

I suppose the documentation needs to reflect that .button may be something else than described in SDL_BUTTON_*.
Comment 1 Turo Lamminen 2013-12-19 13:23:30 UTC 
The behavior is also different between platforms. I have a Logitech MX518 with three extra buttons (back, forward and home). SDL 2.0.1 reports them as follows:

Windows reports back & forward as 4 & 5 as documentation says. Home is not reported at all.

Mac OS X 10.8 reports back & forward as 4 & 5 and home as 6. Documentation is misleading.

Linux (Debian testing) reports back & forward as 8 & 9 and home as 10. Totally different from both documentation and also from other platforms.
Comment 2 Daniel Gibson 2015-01-27 17:29:03 UTC 
I can confirm  this behavior, on both Linux Mint 17.1 KDE and Debian Wheezy with XFCE. Also with two different mice: Logitech MX518 and some cheap Gigabyte Mouse).
I tested with SDL2 2.0 (I think) on Wheezy and 2.0.2 and latest HG (9319:ad4f430cc9f5) on Mint.

Both report the "forward"/"back" buttons (aka X1 and X2) as 8 and 9.
One additional button on the MX518 (zoom or something?) is reported as 10.

I think it's an actual bug that X1 and X2 are not reported as SDL_BUTTON_X1 and SDL_BUTTON_X2 in the SDL_MouseButtonEvent.

I have no idea why the "Importance" was set to "API change" - the API docs may be unclear on what happens when pressing a mouse button > X2, but documenting that wouldn't change/break the API - and neither would fixing this bug.
Thus I changed it to "major".

From the code it's not surprising that this happens: In
  SDL_x11events.c "static void X11_DispatchEvent(_THIS)", "case ButtonPress:"
there is some mousewheel magic that I haven't looked closely at and then:
  SDL_SendMouseButton(data->window, 0, SDL_PRESSED, xevent.xbutton.button);
(Similar for ButtonRelease)

So the xevent.xbutton.button value is just used unchanged to generate the SDL event.
But AFAIK X11 buttons 4 and 5 are used for horizontal scrolling and 6 and 7 for vertical scrolling - so it kinda makes sense that the next "real" (non-wheel) button gets the value 8.
Not sure if X11 always assigns it like this or only with [XYZ]AxisMapping enabled...

Apart from this problem, the documentation should indeed be more clear about SDL_MouseButtonEvent.button's value that can not only take the given constants but can be > SDL_BUTTON_X2 (I guess everything up to 32 or so should be ok for the SDL_BUTTON() macro/ SDL_GetMouseState()).
Comment 3 Daniel Gibson 2015-01-27 17:31:53 UTC 
Created attachment 2006 [details]
Simple test to print the mouse button number
Comment 4 Stijn van Drongelen 2015-02-01 13:08:23 UTC 
> I have no idea why the "Importance" was set to "API change" - the API
> docs may be unclear on what happens when pressing a mouse button >
> X2, but documenting that wouldn't change/break the API - and neither
> would fixing this bug. Thus I changed it to "major".

In my opinion, updating the documentation of a C library does change
the the API, because the documentation _is_ the API. There is nothing
on the language level to give a complete and thorough description of
the API, so you have to document it separately to even have an API
in the first place.

I don't mean to start a debate on "what is an API", but I think that
I saw that importance tag a year ago and thought it was fitting for
the reason stated above.
Comment 5 Daniel Gibson 2015-02-01 18:44:07 UTC 
Well, that is not completely unreasonable, but mapping the events to the existing SDL_BUTTON_X1/2 constants would at least fix most of this bug and comply with the API (in contrast to the current behavior).

I can however imagine that the maintainers ignored this bug because they assumed this issue demanded breaking the API and didn't want to break the API anytime soon.

In this particular case I'd prefer "API breakage" by mentioning that more buttons can be supported but just don't have predefined constants vs waiting for the next major API-breaking SDL release to support more than 5 buttons.
Especially because the implementation already does  that anyway and it doesn't change any function interfaces or anything.
Comment 6 Luke 2015-02-01 20:54:48 UTC 
Possible dupe here:
https://bugzilla.libsdl.org/show_bug.cgi?id=2472
Comment 7 Daniel Gibson 2015-02-02 07:50:22 UTC 
Yeah, pretty sure it's the same bug.
Comment 8 Ryan C. Gordon 2015-02-19 05:30:52 UTC 
Marking this as a dupe of Bug #2472 (which has more discussion, too).

*** This bug has been marked as a duplicate of bug 2472 ***