Hi Phil/Sergey, As per our conversation I have modified the fix to include following: 1. Adding final modifier to the new API. 2. Finding the usages of “fileChooserShortcutPanelFiles”, and replacing it with the new API - Fortunately, there is only one place in WindowsPlacesBar.java, and I have changed it. 3. Modified the comments for the return value to read as follows: “The array returned may be possibly empty if there are no appropriate Permissions.”
Here is the link to the new webrev: http://cr.openjdk.java.net/~kaddepalli/8182041/webrev03/ <http://cr.openjdk.java.net/~kaddepalli/8182041/webrev03/> Thanks, Krishna > On 05-Sep-2018, at 5:09 PM, Krishna Addepalli <[email protected]> > wrote: > > Hi Phil, > > Posting your question: Is it valid to fail the test, if it returns no > shortcut files? > > The test runs only on windows, and without security manager, it is expected > to return some well known shortcuts like "Desktop", "Network" etc, which is > why I'm failing if none of these are returned. > > Thanks, > Krishna > > -----Original Message----- > From: Krishna Addepalli > Sent: Wednesday, September 5, 2018 5:04 PM > To: Sergey Bylokhov <[email protected]>; Prasanta Sadhukhan > <[email protected]>; [email protected] > Subject: Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut Panel > folders under on JDK9 > > Hi Sergey, > > This method is a public API, and hence it is only meant for clients using the > API. Internally, WindowsPlacesBar.java directly calls > ShellFolder.get("fileChooserShortcutPanelFolders"). > Couple other classes Win32ShellFolderManager2.java and > ShellFolderManager.java also use it directly. > Ofcousre, it can be changed to call the method in FileSystemView.java, but, > this api is modeled after FileSystemView::getChooserComboBoxFiles(), which > also does something similar. > > Thanks, > Krishna > > -----Original Message----- > From: Sergey Bylokhov > Sent: Wednesday, September 5, 2018 4:28 AM > To: Krishna Addepalli <[email protected]>; Prasanta Sadhukhan > <[email protected]>; [email protected] > Subject: Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut Panel > folders under on JDK9 > >> BTW I am not sure do we need this refinement, or maybe this text is >> enough: "Returns an array of files representing the values which will >> be shown in the file chooser shortcuts panel." > > One more notes, if result of this method should be "shown in the file chooser > shortcuts panel", then this method should be called somewhere, isn't it? > > >> >> I also think that "which may be shown" is better than "which will be >> shown". >> >> >> On 30/08/2018 04:15, Krishna Addepalli wrote: >>> Hi Prasanta, >>> >>> Thanks for the review. Here is the link to CSR: >>> https://bugs.openjdk.java.net/browse/JDK-8210210 >>> >>> Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Thursday, August 30, 2018 12:28 PM >>> *To:* Krishna Addepalli <[email protected]>; >>> [email protected] >>> *Subject:* Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut >>> Panel folders under on JDK9 >>> >>> ok. in that case, fix looks good. You will need a CSR. >>> >>> I guess the test will fail compilation without fix so no exception >>> will be thrown but I guess we cannot escape that. >>> >>> Regards >>> Prasanta >>> >>> On 8/29/2018 6:50 PM, Krishna Addepalli wrote: >>> >>> I presume you are hinting at binary compatibility. I think, that >>> should not be an issue, since we are not modifying any of the >>> existing functions, but adding a new function. So, I don't think >>> there would be any linking issues. Correct me if I'm wrong. >>> >>> Thanks, >>> >>> Krishna >>> >>> *From:*Krishna Addepalli >>> *Sent:* Wednesday, August 29, 2018 5:29 PM >>> *To:* Prasanta Sadhukhan <[email protected]> >>> <mailto:[email protected]>; >>> [email protected] >>> <mailto:[email protected]> >>> *Subject:* RE: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser >>> Shortcut Panel folders under on JDK9 >>> >>> But we cannot have default methods in abstract classes. Only >>> interfaces allow that. >>> >>> Thanks, >>> >>> Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Wednesday, August 29, 2018 3:21 PM >>> *To:* Krishna Addepalli <[email protected]> >>> <mailto:[email protected]>; [email protected] >>> <mailto:[email protected]> >>> *Subject:* Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser >>> Shortcut Panel folders under on JDK9 >>> >>> On 8/29/2018 2:23 PM, Krishna Addepalli wrote: >>> >>> Hi Prasanta, >>> >>> The new method "getChooserShortcutPanelFiles", and the method >>> "getChooserComboBoxFiles" are present in FileSystemView class >>> itself, which makes it a default implementation already. >>> >>> I doubt that. If an application has extended jdk9 FileSystemView >>> class and has its own implementation for its OS, then it probably >>> will not link with jdk12 modified FileSystemView class as it >>> contains a new method. >>> Probably, you should consider adding "default" keyword to your >>> new >>> method. >>> >>> Regards >>> Prasanta >>> >>> As for the string "fileChooserShortcutPanelFolders", it goes >>> to >>> OS specific implementation and provides appropriate result as >>> per the underlying OS. >>> >>> Thanks, >>> >>> Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Wednesday, August 29, 2018 2:07 PM >>> *To:* Krishna Addepalli <[email protected]> >>> <mailto:[email protected]>; >>> [email protected] >>> <mailto:[email protected]> >>> *Subject:* Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser >>> Shortcut Panel folders under on JDK9 >>> >>> Hi Krishna, >>> >>> One more thing...it is mentioned here >>> >>> 58 * Java Licensees may want to provide a different >>> implementation of >>> >>> 59 * FileSystemView to better handle a given operating >>> system >>> >>> >>> If this class is ever extended by applications, then would it >>> not be source incompatible change as there is no default >>> >>> implementation of the new method? >>> >>> >>> Regards >>> >>> Prasanta >>> >>> On 8/27/2018 5:05 PM, Krishna Addepalli wrote: >>> >>> Hi Prasanta, >>> >>> Thanks for pointing those out. Corrected them in the new >>> webrev: >>> http://cr.openjdk.java.net/~kaddepalli/8182041/webrev01/ >>> >>> <http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev01/> >>> >>> Krishna >>> >>> *From:*Prasanta Sadhukhan >>> *Sent:* Monday, August 27, 2018 4:50 PM >>> *To:* Krishna Addepalli <[email protected]> >>> <mailto:[email protected]>; >>> [email protected] >>> <mailto:[email protected]> >>> *Subject:* Re: <Swing Dev> [12]RFR:JDK-8182041- FIle >>> Chooser >>> Shortcut Panel folders under on JDK9 >>> >>> Hi Krishna, >>> >>> Quick comments: >>> @since 10 should be @since 12 in API javadoc >>> Is ShellFolderQueriesSecurityManagerTest a manual test as >>> you mentioned >>> >>> @run main/manual/ >>> >>> >>> Regards >>> Prasanta >>> >>> On 8/27/2018 4:24 PM, Krishna Addepalli wrote: >>> >>> Hi All, >>> >>> Please review fix for JDK10 (the changes involve AWT >>> and >>> Swing): >>> >>> bug: https://bugs.openjdk.java.net/browse/JDK-8182041 >>> >>> webrev: >>> >>> http://cr.openjdk.java.net/~kaddepalli/8182041/webrev00/ >>> >>> <http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev00/> >>> >>> New API method was added to query shortcut panel >>> entries >>> for JFileChooser, since ShellFolder is internal class >>> which is not publicly accessible. >>> >>> Thanks, >>> >>> Krishna >>> >> >> > > > -- > Best regards, Sergey.
