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

Thanks,
Krishna

On 05-Sep-2018, at 5:09 PM, Krishna Addepalli <krishna.addepa...@oracle.com <mailto:krishna.addepa...@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.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>>; Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>>; swing-dev@openjdk.java.net <mailto:swing-dev@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.addepa...@oracle.com <mailto:krishna.addepa...@oracle.com>>; Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>>; swing-dev@openjdk.java.net <mailto:swing-dev@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.addepa...@oracle.com>;
swing-dev@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.sadhuk...@oracle.com>
    <mailto:prasanta.sadhuk...@oracle.com>;
swing-dev@openjdk.java.net
    <mailto:swing-dev@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.addepa...@oracle.com>
    <mailto:krishna.addepa...@oracle.com>; swing-dev@openjdk.java.net
    <mailto:swing-dev@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.addepa...@oracle.com>
        <mailto:krishna.addepa...@oracle.com>;
        swing-dev@openjdk.java.net
<mailto:swing-dev@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/>

            Krishna

            *From:*Prasanta Sadhukhan
            *Sent:* Monday, August 27, 2018 4:50 PM
            *To:* Krishna Addepalli <krishna.addepa...@oracle.com>
            <mailto:krishna.addepa...@oracle.com>;
            swing-dev@openjdk.java.net
<mailto:swing-dev@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/>

                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.

Reply via email to