<Swing Dev> [10] RFR JDK-8183529: FilleChooser in "Detail view" does not change the Language of the column headings

Thu Jul 13 14:52:10 UTC 2017

Looks good.

--Semyon

On 07/12/2017 10:12 PM, Prasanta Sadhukhan wrote:
>
>
>
> On 7/13/2017 6:58 AM, Semyon Sadetsky wrote:
>> On 7/11/2017 10:54 PM, Prasanta Sadhukhan wrote:
>>>
>>> Thanks Semyon for the pointer. Calling NewGlobalRef makes these 
>>> fields a global ref which can then be accessed from different 
>>> thread, as doGetColumnInfo() is called from COM invoker thread and 
>>> initIDs() from main thread.
>>>
>> It is unrelated to mutithreading. You received a local reference 
>> which is disposed when the method exits.
>>>
>>> Modified webrev:
>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.07/
>> Why do you call GetStaticObjectField() on ShellFolderColumnInfo while 
>> the fields belong to Win32ShellFolder2?
> It was carried forward from prev. webrev. Modified webrev
> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.08/
>
> Regards
> Prasanta
>>
>> --Semyon
>>>
>>> Regards
>>> Prasanta
>>> On 7/12/2017 12:21 AM, Semyon Sadetsky wrote:
>>>>
>>>>
>>>> On 7/10/2017 8:57 AM, Prasanta Sadhukhan wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 7/10/2017 8:37 PM, Semyon Sadetsky wrote:
>>>>>>
>>>>>> On 07/10/2017 08:01 AM, Prasanta Sadhukhan wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 7/10/2017 8:29 PM, Semyon Sadetsky wrote:
>>>>>>>>
>>>>>>>> Hi Prasanta,
>>>>>>>>
>>>>>>>> Please, add static modifier to those java constants.
>>>>>>>>
>>>>>>> It causes "NoSuchFieldError" if we access static field defined 
>>>>>>> in java from jni.
>>>>>> I did not get this. It is impossible to read static java fields 
>>>>>> in JNI, is that what you mean?
>>>>>>>>
>>>>>>>> Also, in ShellFolder2.cpp, could you move the initialization 
>>>>>>>> lines from _doGetColumnInfo() to _initIDs()?
>>>>>>>>
>>>>>>> GetObjectField() needs jobject parameter which is not available 
>>>>>>> in initIDs()
>>>>>> Did you try GetStaticObjectField()?
>>>>>>
>>>>> Moving to initIDs() causes the same problem I faced earlier
>>>>> /    #  Internal Error 
>>>>> (d:\jdk10\client\hotspot\src\share\vm\runtime/jniHandles.hpp:223), 
>>>>> pid=16860, tid=21856//
>>>>> //    #  assert(value != (cast_to_oop(::badJNIHandleVal))) failed: 
>>>>> Pointing to zapped jni handle area//
>>>>> /
>>>> This is because you need to keep reference to use the object 
>>>> between calls.
>>>> Wrap the objects by NewGlobalRef() and the crash will have gone.
>>>> Also, please, add exception check after getting each filed.
>>>>
>>>> --Semyon
>>>>> //so I kept in doGetColumnInfo()
>>>>> Modified webrev:
>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.06/
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>>> --Semyon
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>>>
>>>>>>>> --Semyon
>>>>>>>>
>>>>>>>> On 07/07/2017 10:49 PM, Prasanta Sadhukhan wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 7/8/2017 2:22 AM, Semyon Sadetsky wrote:
>>>>>>>>>>
>>>>>>>>>> Why you declared fields instead of constants? Are they 
>>>>>>>>>> supposed to be changed?
>>>>>>>>>>
>>>>>>>>>> Also, most of them are already declared in the superclass:
>>>>>>>>>>
>>>>>>>>>>     private static final String COLUMN_NAME = 
>>>>>>>>>> "FileChooser.fileNameHeaderText";
>>>>>>>>>>     private static final String COLUMN_SIZE = 
>>>>>>>>>> "FileChooser.fileSizeHeaderText";
>>>>>>>>>>     private static final String COLUMN_DATE = 
>>>>>>>>>> "FileChooser.fileDateHeaderText";
>>>>>>>>>>
>>>>>>>>> Those had private access and I did not want to change 
>>>>>>>>> superclass but anyways, if you insist here's the modified webrev
>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.05/
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Prasanta
>>>>>>>>>>
>>>>>>>>>> --Semyon
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 07/07/2017 09:26 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>
>>>>>>>>>>> Modified webrev to not create new string objects every time 
>>>>>>>>>>> doGetColumnInfo() is called, rather it "gets" the  string 
>>>>>>>>>>> objects.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.04/
>>>>>>>>>>>
>>>>>>>>>>> Regards
>>>>>>>>>>> Prasanta
>>>>>>>>>>> On 7/7/2017 8:51 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> I may be wrong, but the cause may be that you did not keep 
>>>>>>>>>>>> references to those objects and they were garbage collected.
>>>>>>>>>>>>
>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 07/07/2017 08:13 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> I tried that putting the initialization in initIDs() which 
>>>>>>>>>>>>> will be called once only but am getting jni crash citing " 
>>>>>>>>>>>>> Pointing to zapped jni handle area". Only in 
>>>>>>>>>>>>> doGetColumnInfo(), it's working.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards
>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>> On 7/7/2017 8:38 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> That's better. But still each time when 
>>>>>>>>>>>>>> getFolderColumns() is called the title keys are initialized.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> That will be more optimal to initialize them once only 
>>>>>>>>>>>>>> and reuse them in consequent calls, won't it?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 07/06/2017 11:26 PM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Modified webrev after removal of intermediate variable 
>>>>>>>>>>>>>>> temp and reusing strings
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.03/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>> On 7/6/2017 9:52 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Why do you need intermediate variable temp to convert C 
>>>>>>>>>>>>>>>> string to java string?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Also could the strings be created only once and reused?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 07/06/2017 09:12 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Semyon,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I missed that. I see now, the page mentions that "The 
>>>>>>>>>>>>>>>>> first four fields are standard for all file system 
>>>>>>>>>>>>>>>>> folders"
>>>>>>>>>>>>>>>>> Column index
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 	Column title
>>>>>>>>>>>>>>>>> 0 	Name
>>>>>>>>>>>>>>>>> 1 	Size
>>>>>>>>>>>>>>>>> 2 	Type
>>>>>>>>>>>>>>>>> 3 	Date Modified
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> so I modified webrev to rely on column index rather 
>>>>>>>>>>>>>>>>> than string.
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.02/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>> On 7/6/2017 9:01 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Prasanta,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> See what MSDN says [1] about the column titles 
>>>>>>>>>>>>>>>>>> obtained by IShellFolder2::GetDetailsOf:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> ... Bear in mind that these titles can be localized 
>>>>>>>>>>>>>>>>>> and might not be the same for all locales.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> [1] 
>>>>>>>>>>>>>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/bb775053(v=vs.85).aspx
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 07/06/2017 01:13 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>>>>>>> Thanks Semyon for spotting this. Since this bug is 
>>>>>>>>>>>>>>>>>>> for windows, I concentrated on windows only.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> But it seems, for non-windows platform, ShellFolder 
>>>>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>>>>> COLUMN_NAME = "FileChooser.fileNameHeaderText";
>>>>>>>>>>>>>>>>>>> COLUMN_SIZE = "FileChooser.fileSizeHeaderText";
>>>>>>>>>>>>>>>>>>> COLUMN_DATE = "FileChooser.fileDateHeaderText";
>>>>>>>>>>>>>>>>>>> string which is locale-sensitive.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Only for windows, it uses Win32ShellFolder which 
>>>>>>>>>>>>>>>>>>> calls IShellDetails::GetDetailsOf() to get columns 
>>>>>>>>>>>>>>>>>>> details.
>>>>>>>>>>>>>>>>>>> Modified webrev applicable for only windows to 
>>>>>>>>>>>>>>>>>>> convert this windows specific names to 
>>>>>>>>>>>>>>>>>>> locale-sensitive names.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.01/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>>> On 7/5/2017 8:40 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hi Prasanta,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Haven't you tested how the details header 
>>>>>>>>>>>>>>>>>>>> localization works after your fix with other L&Fs 
>>>>>>>>>>>>>>>>>>>> and platforms?
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 07/04/2017 11:42 PM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Please review a fix for a locale issue where it is 
>>>>>>>>>>>>>>>>>>>>> seem FileChooser dialog is not showing the column 
>>>>>>>>>>>>>>>>>>>>> heading
>>>>>>>>>>>>>>>>>>>>> in selected locale in "Detail view mode".
>>>>>>>>>>>>>>>>>>>>> This was because, even though the locale strings 
>>>>>>>>>>>>>>>>>>>>> are present in properties resource file,
>>>>>>>>>>>>>>>>>>>>> /share/classes/com/sun/java/swing/plaf/windows/resources/windows.properties//
>>>>>>>>>>>>>>>>>>>>> //FileChooser.fileNameHeader.textAndMnemonic=Name//
>>>>>>>>>>>>>>>>>>>>> //FileChooser.fileSizeHeader.textAndMnemonic=Size//
>>>>>>>>>>>>>>>>>>>>> /the check done is wrong.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Proposed fix is to check and get locale string 
>>>>>>>>>>>>>>>>>>>>> resources correctly.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8183529
>>>>>>>>>>>>>>>>>>>>> webrev: 
>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.00/
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Regards
>>>>>>>>>>>>>>>>>>>>> Prasanta
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

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