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