<AWT Dev> [9] Review request for 8003399: JFileChooser gives wrong path to selected file when saving to Libraries folder on Windows 7

Semyon Sadetsky semyon.sadetsky at oracle.com
Fri May 22 14:51:42 UTC 2015


Sergey,

Thank you for the review.
the improved version: 
http://cr.openjdk.java.net/~ssadetsky/8003399/webrev.03/

--Semyon

On 5/22/2015 2:59 PM, Sergey Bylokhov wrote:
> Hi, Semyon.
> A few notes:
>  - You need to check all possible exceptions and nulls in the native 
> part of the fix(for example when you call JNU_NewStringPlatform). It 
> can be double checked using parfait.
>  - Please send a request about this comment "this is a temp fix until 
> java.io starts support Libraries" to the core-lib alias. If it will 
> not be supported will mean that out fix is not temporary.
>  - Small issues in using spaces in "if("
>
> On 21.05.15 18:15, Anton Tarasov wrote:
>> So, it looks fine to me now. Thanks.
>>
>> Anton.
>>
>> On 20/05/15 17:12, Semyon Sadetsky wrote:
>>> Hi Anton,
>>>
>>> http://cr.openjdk.java.net/~ssadetsky/8003399/webrev.02/
>>> I have added the macro you requested.
>>>
>>> --Semyon
>>>
>>> On 5/20/2015 3:34 PM, Anton V. Tarasov wrote:
>>>> Hi Semyon,
>>>>
>>>> I'm fine with it, but don't you want to define a simple macro for this:
>>>>
>>>> +    jfieldID field_guid = env->GetFieldID(cl, "guid", "Ljava/lang/String;");
>>>> +    DASSERT(field_guid != NULL);
>>>> +    CHECK_NULL_RETURN(field_guid, NULL);
>>>>
>>>> To call it like:
>>>>
>>>> DEFINE_FIELD_ID(field_guid, cl, "guid", "Ljava/lang/String;");
>>>>
>>>> You would reduce the code a lot and make it more readable.
>>>>
>>>> Regards,
>>>> Anton.
>>>>
>>>> On 19.05.2015 18:45, Semyon Sadetsky wrote:
>>>>> Hi Anton,
>>>>>
>>>>> here is an updated version: 
>>>>> http://cr.openjdk.java.net/~ssadetsky/8003399/webrev.01/
>>>>>
>>>>> --Semyon
>>>>>
>>>>> On 5/8/2015 5:01 PM, Semyon Sadetsky wrote:
>>>>>>
>>>>>> On 5/8/2015 3:45 PM, Sergey Bylokhov wrote:
>>>>>>> On 07.05.15 15:29, Semyon Sadetsky wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> Yes, after the fix filedialog produces usual filesystem paths 
>>>>>>>> for libraries which are readable for java.io.
>>>>>>> Just to clarify: after the fix, both Open and Save dialog works?
>>>>>> Open file in library was not a problem, because an exicting file 
>>>>>> has real FS path already.
>>>>>>
>>>>>>>> But there are no possibility to reference files in libraries 
>>>>>>>> directly using new File(<library link>).
>>>>>>>>
>>>>>>>> --Semyon
>>>>>>>>
>>>>>>>> On 5/7/2015 11:26 AM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Semyon.
>>>>>>>>> Can you please raise the supportness of this in the java.io on 
>>>>>>>>> the core-libs alias.
>>>>>>>>> Does the open filedialog will work after the fix?
>>>>>>>>>
>>>>>>>>> On 07.05.15 11:14, Semyon Sadetsky wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> Please review fix for JDK9.
>>>>>>>>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8003399/webrev.00/
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8003399
>>>>>>>>>>
>>>>>>>>>> ***THE ROOT CAUSE
>>>>>>>>>> JDK uses legacy WINAPI special folders calls while MS 
>>>>>>>>>> introduced a new interfaces IKnownFolder and IShellLibrary to 
>>>>>>>>>> manage special folder locations and the new Libraries 
>>>>>>>>>> functionality in Windows 7 is not backward compatible with 
>>>>>>>>>> old special folders CSIDL.
>>>>>>>>>>
>>>>>>>>>> ***SOLUTION
>>>>>>>>>> Since it is too expensive to migrate AWT shell to the new 
>>>>>>>>>> interfaces and still they are not supported in java.io the 
>>>>>>>>>> solution is to map virtual folder PIDL to the Known Folder 
>>>>>>>>>> GUID and replace libraries links with the default library 
>>>>>>>>>> save location. Thus the File save dialog will be able to work 
>>>>>>>>>> with any Libraries registered in the system (Windows 
>>>>>>>>>> Libraries concept assumes that Libraries can be added 
>>>>>>>>>> arbitrary).
>>>>>>>>>> The resulting code should be compatible with older Windows 
>>>>>>>>>> versions because the new COM interfaces are called only if 
>>>>>>>>>> they are available and a Libraries link has been actually 
>>>>>>>>>> requested.
>>>>>>>>>>
>>>>>>>>>> ***TESTING
>>>>>>>>>> A test scenario is added to check that all available 
>>>>>>>>>> Libraries links are converted into filesystem paths.
>>>>>>>>>>
>>>>>>>>>> --Semyon
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
> -- 
> Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20150522/f509c2e8/attachment-0001.html>


More information about the awt-dev mailing list