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

[Roman Kennke]

Am Montag, den 12.11.2012, 18:29 +0400 schrieb 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?

This should not be the case. We only removed handling of additional bits
in the event-state mask that *cannot be mouse buttons*. Buttons > 5 are
still reported and handled using button number, just not in the bitmask.

>  Did you 
> run any tests? And specifically, have you tried an advanced mouse 
> (gaming mice, I believe, always have plenty of buttons)?

Yes, we tried a many-button-mouse, however we do not have a specific
many-button test. If you have any specific test in mind that we should
run, please point us to it.

>  Did they still 
> work fine after you fix, and are able to send events for all their 
> buttons to Java?

Well, yeah, as I said, we still get events for everything. What is fixed
is only the way we handle the button-mask. It is simply not possible to
handle more than 5 buttons using this mask, because we would look at
something that is not buttons. (Just look at the input event masks in
XConstants. Bits 8-12 are button masks, the rest are used for different
purposes.)


> 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.

Yeah this is a little strange. We decided to make
XlibUtil.getButtonMask() 1-based so that incoming button numbers can
easily be converted to button masks (X11 and Java number buttons
starting from 1). This is what happens in this place (correct me if I am
wrong). In some other places we have 0-based loops, which is why we have
to add 1 there. Maybe it would be preferable to make those loops 1-based
so that the code at least looks the same. ?


> 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?

We believe the mouse button logic is currently broken (see above
discussion on why this is the case). We will likely run into similar
bugs in the future if we leave half of it 'intact'. I would prefer to
not leave bugs intact ;-)

Best regards,
Roman

> 
> --
> 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
> >
>