<Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut Panel folders under on JDK9

Krishna Addepalli krishna.addepalli at oracle.com
Thu Oct 4 02:58:19 UTC 2018


Thanks for the review Sergey. Could you also add your review to the CSR?

Krishna

-----Original Message-----
From: Sergey Bylokhov 
Sent: Thursday, October 4, 2018 7:12 AM
To: Krishna Addepalli <krishna.addepalli at oracle.com>
Cc: swing-dev at openjdk.java.net
Subject: Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut Panel folders under on JDK9

Looks fine.

On 26/09/2018 04:50, Krishna Addepalli wrote:
> Hi Sergey,
> 
> Per our conversation, I have fixed the test to run on Mac and Linux as 
> well. Here is the new webrev:
> http://cr.openjdk.java.net/~kaddepalli/8182041/webrev05/
> <http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev05/>
> 
> Thanks,
> 
> Krishna
> 
> *From:*Krishna Addepalli
> *Sent:* Monday, September 17, 2018 10:44 PM
> *To:* Sergey Bylokhov <sergey.bylokhov at oracle.com>
> *Cc:* swing-dev at openjdk.java.net
> *Subject:* Re: <Swing Dev> [12]RFR:JDK-8182041- FIle Chooser Shortcut 
> Panel folders under on JDK9
> 
> Hi Sergey,
> 
> Thanks for checking on Mac. I realised that there is no code which 
> filters any inaccessible links, nor there are any special folders that 
> are returned on Mac. It simply returns the user home folder in the array.
> 
> I have made the test to run only on Windows.
> 
> Here is the new webrev: 
> http://cr.openjdk.java.net/~kaddepalli/8182041/webrev04/
> <http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev04/>
> 
> Thanks,
> 
> Krishna
> 
> 
> 
>     On 11-Sep-2018, at 1:08 PM, Sergey Bylokhov
>     <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
> 
>     Hi, Krishna.
>     Please recheck the new "ShellFolderQueriesSecurityManagerTest", it
>     does not work when I run it on macOS.
> 
>     On 10/09/2018 06:08, Krishna Addepalli wrote:
> 
>         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/%7Ekaddepalli/8182041/webrev03/><http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev03/>
>         Thanks,
>         Krishna
> 
>             On 05-Sep-2018, at 5:09 PM, Krishna Addepalli
>             <krishna.addepalli at oracle.com
>             <mailto:krishna.addepalli at oracle.com><mailto:krishna.addepalli at oracle.com>>
>             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 <sergey.bylokhov at oracle.com
>             <mailto:sergey.bylokhov at oracle.com><mailto:sergey.bylokhov at oracle.com>>;
>             Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com
>             <mailto:prasanta.sadhukhan at oracle.com><mailto:prasanta.sadhukhan at oracle.com>>;swing-dev at openjdk.java.net
>             <mailto:swing-dev at openjdk.java.net><mailto:swing-dev at openjdk.java.net>
>             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 <krishna.addepalli at oracle.com
>             <mailto:krishna.addepalli at oracle.com><mailto:krishna.addepalli at oracle.com>>;
>             Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com
>             <mailto:prasanta.sadhukhan at oracle.com><mailto:prasanta.sadhukhan at oracle.com>>;swing-dev at openjdk.java.net
>             <mailto:swing-dev at openjdk.java.net><mailto:swing-dev at openjdk.java.net>
>             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
>                     <krishna.addepalli at oracle.com
>                     <mailto:krishna.addepalli at oracle.com>>;
>                     swing-dev at openjdk.java.net
>                     <mailto:swing-dev at openjdk.java.net>
>                     *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
>                     <prasanta.sadhukhan at oracle.com
>                     <mailto:prasanta.sadhukhan at oracle.com>>
>                     <mailto:prasanta.sadhukhan at oracle.com>;
>                     swing-dev at openjdk.java.net
>                     <mailto:swing-dev at openjdk.java.net>
>                     <mailto:swing-dev at openjdk.java.net>
>                     *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
>                     <krishna.addepalli at oracle.com
>                     <mailto:krishna.addepalli at oracle.com>>
>                     <mailto:krishna.addepalli at oracle.com>;
>                     swing-dev at openjdk.java.net
>                     <mailto:swing-dev at openjdk.java.net>
>                     <mailto:swing-dev at openjdk.java.net>
>                     *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
>                     <krishna.addepalli at oracle.com
>                     <mailto:krishna.addepalli at oracle.com>>
>                     <mailto:krishna.addepalli at oracle.com>;
>                     swing-dev at openjdk.java.net
>                     <mailto:swing-dev at openjdk.java.net>
>                     <mailto:swing-dev at openjdk.java.net>
>                     *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/>
> 
>                     
> <http://cr.openjdk.java.net/%7Ekaddepalli/8182041/webrev01/>
> 
>                     Krishna
> 
>                     *From:*Prasanta Sadhukhan
>                     *Sent:* Monday, August 27, 2018 4:50 PM
>                     *To:* Krishna Addepalli
>                     <krishna.addepalli at oracle.com
>                     <mailto:krishna.addepalli at oracle.com>>
>                     <mailto:krishna.addepalli at oracle.com>;
>                     swing-dev at openjdk.java.net
>                     <mailto:swing-dev at openjdk.java.net>
>                     <mailto:swing-dev at openjdk.java.net>
>                     *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/>
> 
>                     
> <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.
> 
> 
> 
>     --
>     Best regards, Sergey.
> 


--
Best regards, Sergey.


More information about the swing-dev mailing list