I have no any additional comments, I think you will need an approval from Semyon.
----- prasanta.sadhuk...@oracle.com wrote: > > > > On 7/11/2017 2:28 AM, Semyon Sadetsky wrote: > On 07/10/2017 01:48 PM, Sergey Bylokhov wrote: > > So just to confirm that in the case when the > "JComponent.setDefaultLocale(English)" will be called on non-English system > the whole chooser will use English for all parts. > I think so, keys should be used to get localized strings from the > corresponding resource bundle. > > Yes, since we are using resource string, it will pick up > FileChooser.fileNameHeader.textAndMnemonic > from java/swing/plaf/windows/resources/windows.properties (properties for > English locale) > and show "Name" instead of native locale name. > > Is webrev.06 ok for commit? > > Regards > Prasanta > --Semyon > > > ----- semyon.sadet...@oracle.com wrote: > > It seem this question was already discussed. Now in the fix the column numbers are used instead of the titles. > > --Semyon > > > On 07/10/2017 12:36 PM, Sergey Bylokhov wrote: > > > > Small question about current approach: > > will it be affected by the "JComponent.setDefaultLocale(someLocale)"? where > > some locale is English on non-English system. > > > > ----- prasanta.sadhuk...@oracle.com 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 > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
