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 <[email protected]>
*Cc:* [email protected]
*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
<[email protected] <mailto:[email protected]>> 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
<[email protected]
<mailto:[email protected]><mailto:[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]
<mailto:[email protected]><mailto:[email protected]>>;
Prasanta Sadhukhan <[email protected]
<mailto:[email protected]><mailto:[email protected]>>;[email protected]
<mailto:[email protected]><mailto:[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]
<mailto:[email protected]><mailto:[email protected]>>;
Prasanta Sadhukhan <[email protected]
<mailto:[email protected]><mailto:[email protected]>>;[email protected]
<mailto:[email protected]><mailto:[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]
<mailto:[email protected]>>;
[email protected]
<mailto:[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]>>
<mailto:[email protected]>;
[email protected]
<mailto:[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]>>
<mailto:[email protected]>;
[email protected]
<mailto:[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]>>
<mailto:[email protected]>;
[email protected]
<mailto:[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/>
<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]>>
<mailto:[email protected]>;
[email protected]
<mailto:[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/>
<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.
