<Swing Dev> RfR JDK-8145207 [macosx] JList, VO can't access non-visible list items

[Pete Brunet]

Hi Alexandr, Thanks very much for the review.  I updated the webrev.  See
http://cr.openjdk.java.net/~ptbrunet/JDK-8145207/webrev.01/

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

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