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