Bug 380257 - SelectionEvent#stateMask should consistently include mouse buttons (was: Shift+Click on Button includes button bit)
Summary: SelectionEvent#stateMask should consistently include mouse buttons (was: Shif...
Status: CLOSED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.8   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks: 233427
  Show dependency tree
 
Reported: 2012-05-22 08:58 EDT by Markus Keller CLA
Modified: 2019-09-07 17:23 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2012-05-22 08:58:55 EDT
(From bug 233427 comment #3)
> This is Kubuntu 11.10 Oneiric Ocelot with KDE 4.7.4 running I20120315-1300 with
> 1.7.0_03-b04 Oracle Java

Shift+Click on a Button doesn't seem to include the Shift modifier in SelectionEvent#stateMask.

(In reply to bug 233427 comment #3)
> This was once claimed to be fixed (see bug 65679 comment 21) but either it
> wasn't tested correctly or got broken again.
Comment 1 Carolyn MacLeod CLA 2012-05-22 12:30:38 EDT
Markus, can you try the following snippet on your Linux GTK to see if shift+click works for you (it works for me on Ubuntu 10.04).
This is a slightly-modified Snippet25 that uses Button Selection instead of Shell KeyUp/KeyDown. If you get stateMask SHIFT printed in the Console when you SHIFT+click on the Button, then maybe your code is just not looking at the stateMask quite correctly? (Note that stateMask=0xa0000 on Linux, and stateMask=0x20000 on Windows, when SHIFT is pressed on mouse click).

import org.eclipse.swt.*;
import org.eclipse.swt.layout.*;
import org.eclipse.swt.widgets.*;

public class Snippet25 {

static String stateMask (int stateMask) {
	String string = "";
	if ((stateMask & SWT.CTRL) != 0) string += " CTRL";
	if ((stateMask & SWT.ALT) != 0) string += " ALT";
	if ((stateMask & SWT.SHIFT) != 0) string += " SHIFT";
	if ((stateMask & SWT.COMMAND) != 0) string += " COMMAND";
	return string;
}

static String character (char character) {
	switch (character) {
		case 0: 		return "'\\0'";
		case SWT.BS:	return "'\\b'";
		case SWT.CR:	return "'\\r'";
		case SWT.DEL:	return "DEL";
		case SWT.ESC:	return "ESC";
		case SWT.LF:	return "'\\n'";
		case SWT.TAB:	return "'\\t'";
	}
	return "'" + character +"'";
}

static String keyCode (int keyCode) {
	switch (keyCode) {
		
		/* Keyboard and Mouse Masks */
		case SWT.ALT: 		return "ALT";
		case SWT.SHIFT: 	return "SHIFT";
		case SWT.CONTROL:	return "CONTROL";
		case SWT.COMMAND:	return "COMMAND";
			
		/* Non-Numeric Keypad Keys */
		case SWT.ARROW_UP:		return "ARROW_UP";
		case SWT.ARROW_DOWN:	return "ARROW_DOWN";
		case SWT.ARROW_LEFT:	return "ARROW_LEFT";
		case SWT.ARROW_RIGHT:	return "ARROW_RIGHT";
		case SWT.PAGE_UP:		return "PAGE_UP";
		case SWT.PAGE_DOWN:		return "PAGE_DOWN";
		case SWT.HOME:			return "HOME";
		case SWT.END:			return "END";
		case SWT.INSERT:		return "INSERT";

		/* Virtual and Ascii Keys */
		case SWT.BS:	return "BS";
		case SWT.CR:	return "CR";		
		case SWT.DEL:	return "DEL";
		case SWT.ESC:	return "ESC";
		case SWT.LF:	return "LF";
		case SWT.TAB:	return "TAB";
		case SWT.SPACE:	return "SPACE";
	
		/* Functions Keys */
		case SWT.F1:	return "F1";
		case SWT.F2:	return "F2";
		case SWT.F3:	return "F3";
		case SWT.F4:	return "F4";
		case SWT.F5:	return "F5";
		case SWT.F6:	return "F6";
		case SWT.F7:	return "F7";
		case SWT.F8:	return "F8";
		case SWT.F9:	return "F9";
		case SWT.F10:	return "F10";
		case SWT.F11:	return "F11";
		case SWT.F12:	return "F12";
		case SWT.F13:	return "F13";
		case SWT.F14:	return "F14";
		case SWT.F15:	return "F15";
		
		/* Numeric Keypad Keys */
		case SWT.KEYPAD_ADD:		return "KEYPAD_ADD";
		case SWT.KEYPAD_SUBTRACT:	return "KEYPAD_SUBTRACT";
		case SWT.KEYPAD_MULTIPLY:	return "KEYPAD_MULTIPLY";
		case SWT.KEYPAD_DIVIDE:		return "KEYPAD_DIVIDE";
		case SWT.KEYPAD_DECIMAL:	return "KEYPAD_DECIMAL";
		case SWT.KEYPAD_CR:			return "KEYPAD_CR";
		case SWT.KEYPAD_0:			return "KEYPAD_0";
		case SWT.KEYPAD_1:			return "KEYPAD_1";
		case SWT.KEYPAD_2:			return "KEYPAD_2";
		case SWT.KEYPAD_3:			return "KEYPAD_3";
		case SWT.KEYPAD_4:			return "KEYPAD_4";
		case SWT.KEYPAD_5:			return "KEYPAD_5";
		case SWT.KEYPAD_6:			return "KEYPAD_6";
		case SWT.KEYPAD_7:			return "KEYPAD_7";
		case SWT.KEYPAD_8:			return "KEYPAD_8";
		case SWT.KEYPAD_9:			return "KEYPAD_9";
		case SWT.KEYPAD_EQUAL:		return "KEYPAD_EQUAL";

		/* Other keys */
		case SWT.CAPS_LOCK:		return "CAPS_LOCK";
		case SWT.NUM_LOCK:		return "NUM_LOCK";
		case SWT.SCROLL_LOCK:	return "SCROLL_LOCK";
		case SWT.PAUSE:			return "PAUSE";
		case SWT.BREAK:			return "BREAK";
		case SWT.PRINT_SCREEN:	return "PRINT_SCREEN";
		case SWT.HELP:			return "HELP";
	}
	return character ((char) keyCode);
}

public static void main (String [] args) {
	Display display = new Display ();
	Shell shell = new Shell (display);
	shell.setLayout(new GridLayout());
	Button button = new Button (shell, SWT.PUSH);
	button.setText("Button");
	Listener listener = new Listener () {
		public void handleEvent (Event e) {
			String string = e.type == SWT.KeyDown ? "DOWN:" : "UP  :";
			string += " stateMask=0x" + Integer.toHexString (e.stateMask) + stateMask (e.stateMask) + ",";
			string += " keyCode=0x" + Integer.toHexString (e.keyCode) + " " + keyCode (e.keyCode) + ",";
			string += " character=0x" + Integer.toHexString (e.character) + " " + character (e.character);
			if (e.keyLocation != 0) {
				string +=  " location="; 
				if (e.keyLocation == SWT.LEFT) string +=  "LEFT"; 
				if (e.keyLocation == SWT.RIGHT) string +=  "RIGHT"; 
				if (e.keyLocation == SWT.KEYPAD) string +=  "KEYPAD"; 
			}
			System.out.println (string);
		}
	};
	button.addListener (SWT.Selection, listener);
	shell.pack ();
	shell.open ();
	while (!shell.isDisposed ()) {
		if (!display.readAndDispatch ()) display.sleep ();
	}
	display.dispose ();
}
}
Comment 2 Markus Keller CLA 2012-05-22 13:28:32 EDT
> (Note that stateMask=0xa0000 on Linux, and
> stateMask=0x20000 on Windows, when SHIFT is pressed on mouse click).

That's exactly the bug. SWT clients should not see any platform differences here.

SelectionEvent#stateMask says:
	/**
	 * The state of the keyboard modifier keys at the time
	 * the event was generated.
	 */

=> The SWT implementation is wrong on GTK, since it also includes SWT.BUTTON1 in the stateMask.
Comment 3 Carolyn MacLeod CLA 2012-05-22 15:25:03 EDT
Interesting. All other event classes that use stateMask (i.e. MouseEvent, KeyEvent, GestureEvent, TouchEvent) have the following javadoc:

	/**
	 * The state of the keyboard modifier keys and mouse masks 
	 * at the time the event was generated.
	 * 
	 * @see org.eclipse.swt.SWT#MODIFIER_MASK
	 * @see org.eclipse.swt.SWT#BUTTON_MASK
	 */

I would like to discuss with SSQ first, to be sure the SelectionEvent#stateMask javadoc is correct (i.e. intentionally different from other XxxEvent#stateMask javadoc). Maybe we should be reporting the button(s) on the other platforms?

In the meantime, the following should work for you no matter what we decide to do:
   if ((stateMask & SWT.MODIFIER_MASK) == SWT.SHIFT) ...
Comment 4 Markus Keller CLA 2016-04-15 11:40:59 EDT
(In reply to Carolyn MacLeod from comment #3)
Thanks. Any change can break unaware clients on some platforms, so I agree the best solution would be to make all platforms consistent to include mouse button state everywhere.

Note that this should be checked/fixed in all widgets on all platforms, but better in an early milestone.

On Windows, I already see quite some inconsistency:
- clicking the up/down arrows in a Spinner sends stateMask == SWT.BUTTON1
- clicking a Button sends stateMask == 0
- clicking and holding a Button or a Link and then pressing Enter sends stateMask == SWT.BUTTON1
- clicking or doubleclicking an item in a Table sends stateMask == SWT.BUTTON1

I've added the BUTTON_MASK to the Javadoc and included a note that this is currently not reliable: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=a6692a3b635791185c17a3a407eabb00baf3cf1c


> In the meantime, the following should work for you no matter what we decide
> to do:
>    if ((stateMask & SWT.MODIFIER_MASK) == SWT.SHIFT) ...

Done that with bug 233427 comment 9.
Comment 5 Eclipse Genie CLA 2019-09-07 17:23:47 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.