<AWT Dev> Bug 853079: focus problems with openjdk 1.7.0 under gnome3 when selcted keyboard is not the first in keyboard list

[Anthony Petrov]

Hi Mario, Roman,

Thanks for additional details. I'm not really an expert in the mouse 
events handling area, but the fix looks scary. I recall we put a lot of 
effort in supporting mice with many buttons in Java (by many I mean like 
7, 10, and even more buttons). Looking at the former 
XConstants.buttonsMask array and the new XlibUtil.getButtonMask(), it 
/seems/ (please keep in mind, I lack deep expertise in this area) that 
you effectively drop support for multi-buttons mice. Is this so? Did you 
run any tests? And specifically, have you tried an advanced mouse 
(gaming mice, I believe, always have plenty of buttons)? Did they still 
work fine after you fix, and are able to send events for all their 
buttons to Java?

A little nit, in XWindow.java:
> -                ((mouseButtonClickAllowed & XConstants.buttonsMask[lbutton]) != 0) ) // No up-button in the drag-state
> +                ((mouseButtonClickAllowed & XlibUtil.getButtonMask(lbutton)) != 0) ) // No up-button in the drag-state

Shouldn't it read lbutton+1 in the second line? The same question for 
line 750.

Overall, is it possible to somehow simplify the fix? Perhaps we only 
want to fix the code that actually installs the window grab, and leave 
all the mouse buttons logic intact?

--
best regards,
Anthony

On 11/12/12 17:58, Roman Kennke wrote:
> Am Donnerstag, den 08.11.2012, 18:00 +0100 schrieb Mario Torre:
>> Hi all!
>>
>> I would like to propose a fix for this bug:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=853079
>>
>> The root cause of the bug is an event being generated that sets the
>> grabWindowRef on the window confusing the internal events state.
>>
>> Usually, when a mouse press is detected the grabWindowRef is set.
>>
>> However, in some cases (like the reproducer for bug in the report, but
>> there are likely other conditions that have similar behaviour), the
>> mouse events carry some flags that the actual code mistakenly interpret
>> causing the grabWindowRef to not be unset.
>>
>> The proposed patch address the code for the button event mapping:
>>
>> http://cr.openjdk.java.net/~neugens/853079/webrev.01/
>>
>> We have no easy way to automatically provide a test case for this bug
>> unfortunately.
>>
>> I could reproduce the bug with OpenJDK 6, 7 and 8 as well as Oracle JDK,
>> so it's a long lasting bug. I'm not sure if it's worth to eventually
>> backport to 7 as well if approved.
>
> Let me add some little explanations (I worked with Mario on this
> bugfix). The underlying problem is that the bitmask that we get in every
> event (event-state) has 5 bits for button state (for the first 5
> buttons). All other bits are used for other stuff. However, we get
> number-of-buttons from X11 reported as 10, and hence look at 10 buttons
> in that bitmask, which is wrong. In our particular case, the
> KeymapStateMask was set (bit #14) and this lead to button-release
> handler assuming not all buttons have been released and hence screwing
> up the grab-state. However, the problem is more general: if we consider
> more than 5 buttons in that mask, we risk to run into problems like
> that. The fix limits to 5 buttons when dealing with the event-state
> mask. (And, this is very reasonable: the other buttons are usually not
> real buttons, but stuff like mouse-wheel or touch-scroll thingies.)
>
> Kind regards,
> Roman
>