Hi Jay,

Thanks for the review.

I have incorporated the review comments. Please have a look.
Webrev:
http://cr.openjdk.java.net/~pbansal/8191957/webrev.03/

Regards,
Pankaj Bansal

-----Original Message-----
From: Jayathirth D V 
Sent: Thursday, March 22, 2018 3:45 PM
To: Pankaj Bansal; swing-dev@openjdk.java.net; Patrick Chen
Subject: RE: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Pankaj,

Also I think we need to add @key headful tag in jtreg comment.

Thanks,
Jay

-----Original Message-----
From: Jayathirth D V 
Sent: Thursday, March 22, 2018 3:43 PM
To: Pankaj Bansal; swing-dev@openjdk.java.net; Patrick Chen
Subject: Re: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Pankaj,

Instructions present in test case is :
        String instructions
        = "INSTRUCTIONS:" +
        "\n 1. This is a Windows 10 specific test. If you not on " +
        "Windows 10, just press Pass." +
        "\n 2. Make sure you have an External Drive attached to your " +
        "computer." +
        "\n 3. Open a JFileChooser by clicking on launch button." +
        "\n 4. In JFileChooser dropdown, there are two Desktop " +
        "locations." +
        "\n 5. One Desktop is child of My PC and one is parent of it." +
        "\n 6. Open the parent Desktop folder." +
        "\n 7. You should see the External Drive in the list of " +
        "files." +
        "\n 8. If the External drive name is empty (it does not have " +
        "any name), the name is incorrect, else it is correct." +
        "\n 9. Press Pass or Press Fail depending upon, whether the " +
        "External Drive name was correct or incorrect.";

Point 1 : Should be "This is a Windows 10 specific test. If you are not on 
Windows 10, just press Pass."
Point 8 & 9 is little bit confusing we can simplify it as : If the External 
drive name is empty (it does not have any name), press fail otherwise press 
pass.

Also as previously pointed out  "Also you have missed opening quote for the 
(Desktop") part of comment in Win32ShellFolderManager2.java." Needs to be 
corrected.

Thanks,
Jay

-----Original Message-----
From: Pankaj Bansal 
Sent: Wednesday, March 21, 2018 10:33 PM
To: Jayathirth D V; swing-dev@openjdk.java.net; Patrick Chen
Subject: RE: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Jay/Patrick,

Thanks for the review.

@jay
<< More inputs:
<<Since the test case is valid only when we have windows 10 system with some 
external drive making the test case manual is also a good idea.
Done. I have updated the webrev with the manual test case.

@Patrick
<<In your webreview : 
<<dir.getPath() is depreciated since java 8.0 I looked at the documentation for 
the File.getPath. I could not find anything related to this being deprecated. 
Could you please point out where did you find that this is deprecated?
Also, just to point out this function was being used earlier also. I have not 
added a new call to this function.

Webrev:
http://cr.openjdk.java.net/~pbansal/8191957/webrev.02/


Regards,
Pankaj Bansal
-----Original Message-----
From: Jayathirth D V
Sent: Wednesday, March 21, 2018 12:45 AM
To: Pankaj Bansal; swing-dev@openjdk.java.net
Subject: RE: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Pankaj,

More inputs:
Since the test case is valid only when we have windows 10 system with some 
external drive making the test case manual is also a good idea.

Thanks,
Jay

-----Original Message-----
From: Jayathirth D V
Sent: Wednesday, March 21, 2018 12:06 AM
To: Pankaj Bansal; swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Pankaj,

I see that you have mentioned this bug is specific to Windows 10 and test case 
needs to be run with some Removable drive.
It better to capture this information in test case also as a comment or in 
jtreg summary.

In test case we have verification part like :
                if (fsv.getSystemDisplayName(file).isEmpty()) {
                    throw new RuntimeException(
                            "External Disk " + file + "name is empty");
                }
This code runs throw all the files under Desktop in my Windows 7 PC. So do we 
have any property in FileSystemView using which we can know that the file is of 
type external drive and then just check system display name for the same.

If we can't determine the file type as external drive using some property in 
FileSystemView, still the RuntimeException message you are throwing("External 
Disk " + file + "name is empty") can be misleading. I would recommend you to 
throw an Exception like -  "System display name for" + file + "is empty".

Also you have missed opening quote for the (Desktop") part of comment in 
Win32ShellFolderManager2.java.

Thanks,
Jay

-----Original Message-----
From: Pankaj Bansal
Sent: Tuesday, March 20, 2018 11:11 PM
To: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi All, 

Does anyone else have any feedback on this?

Regards,
Pankaj Bansal

-----Original Message-----
From: Krishna Addepalli
Sent: Friday, March 16, 2018 4:48 PM
To: Pankaj Bansal; swing-dev@openjdk.java.net
Subject: RE: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name for 
external drives shown under Desktop

Hi Pankaj,

The changes look fine to me.

Thanks,
Krishna

-----Original Message-----
From: Pankaj Bansal
Sent: Friday, March 16, 2018 4:42 PM
To: Krishna Addepalli <krishna.addepa...@oracle.com>; swing-dev@openjdk.java.net
Subject: RE: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name

Hi Krishna,

Thanks for the review.

I have incorporated the review comments. Please have a look.
Webrev: http://cr.openjdk.java.net/~pbansal/8191957/webrev.01/

Regards,
Pankaj Bansal

-----Original Message-----
From: Krishna Addepalli
Sent: Friday, March 16, 2018 4:03 PM
To: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name

Hi Pankaj,

The fix looks fine to me. However I have couple points:

1. Please put a comment at the line of fix, about how Windows 10 treats the 
Desktop folder.
2. The test case can be a headless one, since you are not creating any gui 
components.

Thanks,
Krishna


-----Original Message-----
From: swing-dev-requ...@openjdk.java.net <swing-dev-requ...@openjdk.java.net>
Sent: Monday, March 12, 2018 5:30 PM
To: swing-dev@openjdk.java.net
Subject: swing-dev Digest, Vol 131, Issue 25

Send swing-dev mailing list submissions to
        swing-dev@openjdk.java.net

To subscribe or unsubscribe via the World Wide Web, visit
        http://mail.openjdk.java.net/mailman/listinfo/swing-dev
or, via email, send a message with subject or body 'help' to
        swing-dev-requ...@openjdk.java.net

You can reach the person managing the list at
        swing-dev-ow...@openjdk.java.net

When replying, please edit your Subject line so it is more specific than "Re: 
Contents of swing-dev digest..."


Today's Topics:

   1.  [11] JDK-8191957: JFileChooser shows empty name for external
      drives shown under Desktop (Pankaj Bansal)


----------------------------------------------------------------------

Message: 1
Date: Mon, 12 Mar 2018 01:52:35 -0700 (PDT)
From: Pankaj Bansal <pankaj.b.ban...@oracle.com>
To: swing-dev@openjdk.java.net
Subject: <Swing Dev> [11] JDK-8191957: JFileChooser shows empty name
        for external drives shown under Desktop
Message-ID: <065d066f-810c-4284-9de6-059f4235e61b@default>
Content-Type: text/plain; charset="us-ascii"

Hi All,

 

Please review the test only fix for JDK 11.

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8191957

        

webrev:

http://cr.openjdk.java.net/~pbansal/8191957/webrev.00/

 

Issue:

In windows 10, the "External Drives" or "Removable drives" names are empty when 
root directory "Desktop" is selected in JFileChooser. Only the icon is shown 
and name is empty.

Fix:

The system folder names are found by calling 
FileSystemView.getSystemDisplayName, which in turn calls  
Win32ShellFolderManager2.isFileSystemRoot. Here it was wrongly assumed that a 
drive will always be child of "DRIVES" or "MY PC". But in windows 10, the 
external drives are also shown as children of root directory "Desktop". Made 
changes to consider drives names which are children of "Desktop".

 

Note:

This is a windows 10 specific issues. 

You will need an removable drive attached to you system to test this.

 

Regards,

Pankaj Bansal

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: 
<http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20180312/784cc456/attachment-0001.html>

End of swing-dev Digest, Vol 131, Issue 25
******************************************

Reply via email to