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

[Pete Brunet]

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