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

Tue Jul 11 18:47:18 UTC 2017

I have no any additional comments, I think you will need an approval from Semyon. 

----- prasanta.sadhukhan at 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.sadetsky at 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.sadhukhan at 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 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > 
> 
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170711/043a15e3/attachment.html>