<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 anthony.petrov at oracle.com
Thu Dec 13 05:17:32 PST 2012


Hi Mario,

XConstants.java:
>  133     public static final int MAX_BUTTON_MASK = 5;

This is obviously not a mask, but an index. Please rename this constant 
to just MAX_BUTTONS, or MAX_BUTTON_INDEX, or MAX_BUTTONS_NUMBER - 
whichever you prefer.

Otherwise, the fix looks good to me. Note that you need at least one 
more reviewer in order to push this fix. Awt-dev@, anyone ?


I've filed the following bug for you:
8005018: X11: focus problems with openjdk 1.7.0 under gnome3 when 
selected keyboard is not the first in keyboard list

--
best regards,
Anthony

On 12/10/12 23:57, 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/
>
> Is it OK now for pushing to jdk8 awt-gate?
>
> I need a bug ID too.
>
> Cheers,
> Mario
>
> 2012/11/14 Mario Torre<neugens at 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
>>
>
>
>



More information about the awt-dev mailing list