Review request for 7093156: NLS: Please change the mnemonic assignment system to avoid translation issue (Swing files) (original) (raw)

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Wed Apr 4 10:12:55 UTC 2012

webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.03/ test: http://cr.openjdk.java.net/~alexsch/7093156/test/02/

Now all properties are generated with the following suffixes: xxx.textAndMnemonic and xxx.titleAndMnemonic

After that the extended hashmap check a key suffix and generate a compositeKey which allows to get a property in a new format and extract a text, mnemonic or mnemonic index from it.

The test gets a path to the old properties and L&F class. If L&F is not null than it is set. After that the test gets all properties and it's values and check that values are equal to the values which are got from the UIManager.get(key, locale).

There are some exceptions which are included into the exclude list in the end of the test. For example a mnemonic can be set to zero or to a character instead of using the character integer key value. Or there can be defined just mnemonics without a text.

Thanks, Alexandr.

On 3/30/2012 12:56 PM, Alexander Scherbatiy wrote:

webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.01/ There is the updated webrev which is used the TextAndMnemonicPattern class and separate getTextProperty and getMnemonicProperty methods. The getMnemonicFromProperty returns null in case if a mnemonic has not been found. See other comments inline... On 3/29/2012 5:28 PM, Pavel Porvatov wrote: I'll try explain what I meant below...

Lets take the first loop: + for (int i = 0; i < TEXTSUFFIX.length; i++) { + String suffix = TEXTSUFFIX[i]; + if (stringKey.endsWith(suffix)) { + value = super.get(stringKey + ANDMNEMONIC); + if (value != null) { + return getText(value.toString()); + } + } + } and lets stringKey = BlaBlaBlaNameText. Then you'll invoke super.get() twice and the loop will have 4 iterations. So if you will use for the first loop only "Text", "Title" you will have only 2 loop iterations and no unnecessary super.get() invocations. I have added the TextAndMnemonicPattern class and updated the code to use the following patterns: ( texts: { "NameText", "Text", "Title"} , mnemonic: "Mnemonic") ( texts: { "nameText" } , mnemonic: "mnemonic") Now the "for each" loop is used and for each mnemonic there is only one iteration of the text.

About "They are (xxxNameText, xxxMnemonic), (xxx.nameText, xxx.mnemonic),(xxxText, xxxMnemonic) and (xxxTitle, xxxMnemonic)": can we don't use "Name" for pair (xxxNameText, xxxMnemonic), like xxxTextAndMnemonic? For consistency we could use for pair (xxx.nameText, xxx.mnemonic) xxx.textAndMnemonic... 1) There are real collisions if we will use the same suffix like xxx.textAndMnemonic for all properties. For example see the gtk.properties: FileChooser.renameFileErrorTitle=Error FileChooser.renameFileErrorText=Error renaming file "{0}" to "{1}" Changing it to the same suffix will lead to: FileChooser.renameFileError.textAndMnemonic=Error FileChooser.renameFileError.textAndMnemonic=Error renaming file "{0}" to "{1}" So one value will be lost. 2) We really need to check all these text suffixes. There are 3 examples from the swing property files: ColorChooser.hsvNameText=HSV ColorChooser.hsvMnemonic=72 FileChooser.helpButtonText=Help FileChooser.helpButtonMnemonic=72 GTKColorChooserPanel.nameText=GTK Color Chooser GTKColorChooserPanel.mnemonic=71 which are converted to the new format: FileChooser.helpButtonTextAndMnemonic=&Help ColorChooser.hsvNameTextAndMnemonic=&HSV GTKColorChooserPanel.nameTextAndMnemonic=&GTK Color Chooser Now on the left side there are requests and on the right side are our actions: ColorChooser.hsvNameText | check that it is request to the text (has NameText suffix) and convert to new format ColorChooser.hsvNameTextAndMnemonic (add AndMnemonic suffix) GTKColorChooserPanel.nameText | check that it is request to the text (has nameText) and convert to new format GTKColorChooserPanel.nameTextAndMnemonicadd AndMnemonic suffix) FileChooser.helpButtonMnemonic | check that it is a request to the mnemonic (has Mnemonic suffix) and convert to new format (remove Mnemonic suffix and add TextAndMnemonic suffix) GTKColorChooserPanel.mnemonic | check that it is a request to the mnemonic (has mnemonic suffix) and convert to new format (remove mnemonic suffix and add nameTextAndMnemonic suffix) The suffixes for the text can be Text and Title as well. So the "NameText", "Text", "Title" and "nameText" suffixes are already used in swing components and their unification requires to updating a lot of java code.

3. Could you pleas use "for each" loops (if possible), they are more compact and readable In the second loop the i index is used to add a TEXTSUFFIX after cutting a MNEMONICSUFFIX. So the first loop can use the "for each" loop. Yep! That's what I meant... May be it is better to have a TextAndMnemonic class that holds values for the corresponded text and mnemonic suffixes? 4. Integer.toString((int) Character.toUpperCase(c)) looks a little bit tricky. Can we use something like this: String.valueOf(Character.toUpperCase(c)) ? For example there is the property: ColorChooser.rgbRedTextAndMnemonic=Re&d The code that needs a mnemonic expect that it gets the integer value |68| in the string instead of char D. So the right mnemonic value should be ColorChooser.rgbRedMnemonic=68 Ok. BTW: I think "return null" will be more correct in the getMnemonic method Updated the method to return null if a mnemonic has not been found. Thanks, Alexandr. 5. What is the reason to have the second loop? The first loop checks is it a request to a text value. The second loop checks is it a request to a mnemonic value. Can we just check stringKey.endsWith("mnemonic") || stringKey.endsWith("Mnemonic")? There might be collisions. It is better to explicitly check that a text suffix from a pattern have the certain suffix from the mnemonic pattern. Regards, Pavel Thanks, Alexandr. Regards, Pavel bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7093156 webrev: http://cr.openjdk.java.net/~alexsch/7093156/webrev.00/ The properties in the swing resources files are changed from the xxxText=ABC xxxMnemonic=B to xxxTextAndMnemonic=A&BC where Text suffix can be one of the following: "NameText", "nameText", "Text" and "Title"

The hashmap in UIDefaults class is extended to return the correct values for the keys xxxText and xxxMnemonic from the stored xxxTextAndMnemonic string. There is no html text in the swing resource files so this case is not considered. The fix covers only the swing resource files in the JDK: src/share/classes/com/sun/swing/internal/plaf src/share/classes/com/sun/java/swing/plaf The demo resources will be fixed in the separated issue. Thanks, Alexandr.

More information about the swing-dev mailing list