Am Freitag, den 14.12.2012, 16:35 +0400 schrieb Artem Ananiev:
> On 12/14/2012 4:10 PM, Roman Kennke wrote:
> > Am Freitag, den 14.12.2012, 15:35 +0400 schrieb Artem Ananiev:
> >> On 12/10/2012 11:57 PM, Mario Torre wrote:
> >>> Hello Anthony,
> >>>
> >>> Sorry for the delay, but I've been pretty busy lately.
> >>>
> >>> Here is the new webrev with the corrections you requested:
> >>>
> >>> http://cr.openjdk.java.net/~neugens/853079/webrev.02/
> >>
> >> It looks good, just a single question:
> >>
> >> XBaseWindow.isFullRelease(): could you provide scenario, when this
> >> method is not the same as "return buttonState == 0", please?
> >
> > I am not sure. IIRC, the button mask for button-release events in X11 is
> > the mask *before* the release (i.e. for a full-release the
> > currently-released button is still in there). That's why buttonState==0
> > would not be correct in this case.
>
> Here is the code:
>
> 1036 if (button < 0 || button > buttonsNumber) {
> 1037 return buttonState == 0;
> 1038 } else {
> 1039 return buttonState == XlibUtil.getButtonMask(button);
> 1040 }
>
> If 0 <= button <= buttonsNumber, getButtonMask() will return 0, right?
No. The button will be the number of the button being released. If we
assume it's button #1, then the resulting button mask will be 1 << 8. If
button#1 is the only button, and it's being released, then the
buttonState will be 1<<8 too. This is a bit funny in X11, the
buttonState is the state *before* the event, i.e. on button-press we get
0, on button release 1<<8. Therefore the above code is correct. Notice
that if the button is outside the range that is supported in the mask,
i.e. > 5 (it cannot really be <= 0), the first clause is true as well,
because we do not have any button state. However, those are usually not
real buttons, but stuff like scrollwheels.
Best regards,
Roman
>
> Thanks,
>
> Artem
>
> > Roman
> >
> >>
> >> Thanks,
> >>
> >> Artem
> >>
> >>> Is it OK now for pushing to jdk8 awt-gate?
> >>>
> >>> I need a bug ID too.
> >>>
> >>> Cheers,
> >>> Mario
> >>>
> >>> 2012/11/14 Mario Torre <neug...@redhat.com>:
> >>>> Il giorno mer, 14/11/2012 alle 21.35 +0400, Anthony Petrov ha scritto:
> >>>>> Roman, Mario,
> >>>>>
> >>>>> I agree with your reasoning regarding "prefer to
> >>>>> not leave bugs intact". :) I appreciate that you've tested with a
> >>>>> multi-button mouse, this confirms that the fix is safe. I believe that
> >>>>> we don't have any specific tests for this case, so your manual testing
> >>>>> should be enough. Thanks for this!
> >>>>>
> >>>>> I re-read the fix more precisely, and it actually looks good to me. Just
> >>>>> a few suggestions:
> >>>>>
> >>>>> src/solaris/classes/sun/awt/X11/XlibUtil.java
> >>>>>> 403 if (button <= 0 || button > 5) {
> >>>>>
> >>>>> Should we use XConstants.MAX_BUTTON_MASK instead of a hard-coded value
> >>>>> here?
> >>>>>
> >>>>>> 406 return 1 << 7 + button;
> >>>>>
> >>>>> I suggest to rewrite this as "1 << (7 + button)" for clarity.
> >>>>>
> >>>>> src/solaris/classes/sun/awt/X11/XConstants.java
> >>>>> Perhaps it makes sense to rename ALL_BUTTON_MASKS to ALL_BUTTONS_MASK
> >>>>> since it's only one mask for all the buttons.
> >>>>>
> >>>>> --
> >>>>> best regards,
> >>>>> Anthony
> >>>>
> >>>> Hi Anthony,
> >>>>
> >>>> Thanks for looking at that.
> >>>>
> >>>> I'll prepare an update with your suggestions and send it back to the
> >>>> list.
> >>>>
> >>>> Cheers,
> >>>> Mario
> >>>>
> >>>
> >>>
> >>>
> >>
> >
> >
>
