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

Semyon Sadetsky semyon.sadetsky at oracle.com
Tue Jul 11 18:51:17 UTC 2017 


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

More information about the swing-dev mailing list