<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
Thu Jul 13 01:28:21 UTC 2017
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?
--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/20170712/19fa1846/attachment.html>
More information about the swing-dev
mailing list