<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
Tue Jul 11 04:53:47 UTC 2017



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/524360ff/attachment.html>


More information about the swing-dev mailing list