<Swing Dev> [10] RFR JDK-8183529: FilleChooser in "Detail view" does not change the Language of the column headings
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Wed Jul 12 05:54:19 UTC 2017
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.
Modified webrev:
http://cr.openjdk.java.net/~psadhukhan/8183529/webrev.07/
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/20170712/9c25a67f/attachment.html>
More information about the swing-dev
mailing list