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

Thu Jul 13 05:12:06 UTC 2017


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/d053a7b5/attachment.html>