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