Thanks, Pete! Keeping an eye on JDK-8160893 status as well, as we discussed.

Regards,
Anton.

On 7/18/2016 11:47 PM, Pete Brunet wrote:
Anton, If you'd like to check out the changeset I just pushed it.

Pete

On 7/18/16 3:25 PM, Pete Brunet wrote:
JPRT ran OK.

On 7/15/16 9:24 AM, Pete Brunet wrote:
On 7/15/16 8:42 AM, Alexandr Scherbatiy wrote:
On 7/15/2016 4:39 AM, Pete Brunet wrote:
Hi Alexandr, Thanks very much for the review.  I updated the webrev.
See
http://cr.openjdk.java.net/~ptbrunet/JDK-8145207/webrev.01/
   The fix looks good to me.
Thank you Alexandr.

Looking for one more +1.
I forgot that Anton already has reviewed this.
  From your separate email you suggested the following in order to
provide
the AccessibleAction interface without changing the public API of
JList.AccessibleJList.AccessibleJListChild:

Just create a private subclass of the AccessibleJListChild which
implements AccessibleAction and return it in all places where new
instance of the AccessibleJListChild is returned.

I made that change.
   You can create an enhancement  that JList.AccessibleJListChild
should implement AccessibleAction interface which can be fixed in JDK
9 only.
Good Idea.  I opened JDK-8161483.  Not sure if an RFE will get into 9 at
this point in time so that work might end up in 10.

Pete
   Thanks,
   Alexandr.
On 7/4/16 4:14 AM, Alexandr Scherbatiy wrote:
On 6/18/2016 5:31 AM, Pete Brunet wrote:
Please review the following patch.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145207
Patch: http://cr.openjdk.java.net/~ptbrunet/JDK-8145207/webrev.00/

This fixes the following functionality that was not working with the
JList of ListDemo of SwingSet2.
- start VoiceOver
- start SwingSet2
- start the ListDemo
- press Tab until focus is on the list, should hear VO when changing
selections with up/down arrow
- when interacting with list should hear that there are 30 (total)
items, not 26 (visible) items
- when using control+option+up/downarrow should be able to move to and
select (control+option+spacebar) non-visible items past the 26th
visible
item
- should be able to multi-select both visible and invisible items
using
control+option+command+return and VO should read the item just added
- should be able to shift extend items with shift up or shift down
arrow
and VO should announce the item just added or removed
CAccessibility:
   639                             childrenAndRoles.clear();
   640                             childrenAndRoles.addAll(newArray);

- Is it possible just to assign the newArray to the childrenAndRoles?
Is it necessary that the childrenAndRoles has final keyword?
I made those changes.
- Please, format the code on lines 630-631 to romevo unnessary spaces
in round brackets.
I prefer to keep the spaces in this kind of split line.  Over the years
I've found that style easier to read.
CAccessible:

- static method getActiveDescendant() is not used in the CAccessible
class but only in CAccessibility. Is it possible to move it to the
CAccessibility class?
It's there because it accesses the private field, activeDescendant.
- Please, split the long lines. You may use static imports for
constants.
Done.
JavaComponentAccessibility:
   716     if (returnValue == -1) {
   717         return NSNotFound;
   718     } else {
   719         return returnValue;
   720     }

   - This can be written shorter: return (returnValue == -1) ?
NSNotFound : returnValue;
Done.
   998     if ([self isSelectable:[ThreadUtilities getJNIEnv]]) {
   999         return YES;
1000     } else {
1001         return NO;
1002     }

- Is there a macros which can convert jboolean to BOOL?
I could not find one.
- Could you also split the modified lines where it is possible?
Done.

Pete
Thanks,
Alexandr.

Pete



Reply via email to