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 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>

Reply via email to