Created attachment 7157 [details]
Suggested patch

Here are comments how to use the above given patch:

In Bundle_ru.properties one has to specify line:

MNEMONIC_\u5224=C

which would mean that the when a unicode letter 5224 is to be a
mnemonic 'C' should be used instead of it. I have tested that on

MNEMONIC_F=L 

which succeffully changed the mnemonic of &File to be L. 

One could also specify

MNEMONIC_F=5345

where 5345 is value of a constant from java.awt.event.KeyEvent.VK_***
so one could specify that a mnemonic is key 1, 2, 3 or even Home, etc.


Of course the patch works only for menu items that use services of
org.openide.awt.Actions, but I am not aware of anyone not using this
method.

Jaroslav, Great Work!
And wind-fast!

But as I've already written in nbdev, 
> SWING is not very smart about underlining:
> It will not underline the Russian symbol.

Do you know any way to solve this?

in javadoc for AbstractButton:
> If the corresponding character is not contained within 
> the button's label, then it will be displayed near the 
> label in a look and feel dependent manner (commonly to 
> the right, surrounded by parenthesis). 

But it's UGLY and still there's no (&<latin>) near The 
file menu,
so no symbol is underlined, but Alt+A (the mnemonic) works.

JDK/Swing bug?

and if the letter is not underlined how the user finds out what the
mnemonic he's looking for is?

Not a bug but I would like to see it fixed for 4.0

"It will not underline the Russian symbol" - really? I am not sure
why. I have checked the sources of jdk1.4 and there does not seem to
be such restriction. MetalButtonUI seems to work with any character.
I was able to set 'i' with carka above it to be mnemonic for my JButton.


Moreover I have found that since JDK1.4 there is a method
AbstractButton.setMnemonicIndex. So the actual mnemonic and the
underlined character can be different. This can be used to assign
mnemonic 'X' and select a russian letter.


Maxim, if you want to experiment with it, try to modify the patch to
use setMnemonicIndex, maybe it will work.

AbstractButton.setDisplayedMnemonicIndex to be exact ;)

> "It will not underline the Russian symbol" - really?
Yes, it DOESN'T (I work with localized IDE for more than 
half a year).
It either: (when I specify &<RU>, without the patch) - 
underlines, but Alt-combinations do not work,
Or (with the patch) - Alt-combination perfectly works, but 
the Symbol doesn't get underlined.

Created attachment 7171 [details]
Another (changed patch)

> Maxim, if you want to experiment with it, try to modify
> the patch to use setMnemonicIndex, maybe it will work.

It Works!!!
Please integrate the proposed change in trunk,
and maybe it's not too late for release34?

"Tor Norbye" <Torbjorn.Norbye@Sun.COM> wrote in the news: 
> Maxym Mykhalchuk wrote:
> MM > There're actually so many setMnemonic's around ALL 
Netbeans
> MM > code
<skipped>
> 
> You've noticed 
>   org.openide.awt.Actions's
> setMenuText(AbstractButton item, String text, boolean 
useMnemonic)
> method. This is used for a lot of actions, but as you can
> see, it only applies to buttons.
> 
> In NetBeans we have to set mnemonics on lots of labels 
too
> (for accessibility reasons; e.g. when a label is 
associated
> with a focusable component, you use the mnemonic on the 
label
> to jump to the focusable component.)
> 
> Unfortunately there's no corresponding method which 
works on
> labels - which is why you see all those setMnemonic calls
> (and lots of mnemonic entries in the bundle files.)
> 
> My opinion is that we should add a corresponding method 
to
> org.openide.awt.Actions which works for labels, and 
convert
> code elsewhere to use it. Then these two methods can 
share
> some common code, and you can hack on it to make it work
> better for the Russian locale.    This will mean smaller
> bundle files too.

code attached

Created attachment 7172 [details]
Handy solution for JLabel's (unverified)

We are very close to 3.4 release, this patch was just developed, and
it does not sound like it solves a critical bug relative to other bugs
we have been fixing - rather a situation of poor usability and/or
ugliness, and only affecting a small percentage of users. I  would
suggest we try to push it into 3.4.1 if there is such a release (mark
with keyword: 3.4.1_CANDIDATE).

Possible counterargument: the patch is small and in an
English-language build, the behavior will be the same, except for an
additionally thrown MissingResourceException (hopefully not a
performance problem), so it would probably be harmless. Would need to
be tested some to confirm it is safe and works as documented, in
various locales (English, Russian, Japanese).

Of course the patch can be applied to the trunk right away if it
breaks nothing.

Is this related to 24521 and/or does it not change
the 2 ways of specifying mnemonics:

using the character after the &
(and being sure to put the & there and not relying on
default of letter after it since localization center needs that)

or

having 2 keys per mnemonic - one for mnemonic letter itself
and one for the label of the mnemonic

ken.frank@sun.com
08-20-2002

to Ken.Frank: no it does not, it still uses "&".

The problem that patch2 solves is mostly for the Languages 
like Russian & Greek, which have different from Latin, but 
still a small number of symbols, so it's possible to fit 
them all into the keyboard.
Hence, user should be able to use Alt+<These keys>,
but Swing doesn't directly allow this, so Netbeans 
needs "to reinvent the bicycle", hacking around the Swing.

This is a patch, and not the Action system redesign ;(

2 Jesse:
> We are very close to 3.4 release, this patch was just 
> developed, and
> it does not sound like it solves a critical bug 

For Russian distribution - it's a very critical bug,
not for English distribution, of course.

> I  would suggest we try to push it into 
> 3.4.1 if there is such a release 

Agreed. Is there any way to make it come into Russian 
distribution of Netbeans 3.4, while not coming into 
English distrib?

"Is there any way to make it come into Russian distribution of
Netbeans 3.4, while not coming into English distrib?" - not really.

If you're serious about trying to test this and get it into 3.4,
please double-check

http://www.netbeans.org/devhome/docs/releases/34/high-resistance.html

and specifically post to nbreviewers@netbeans.org ASAP. Since we are
so close to the release date, I think it would be wise to have at
least one code review (besides Yarda), some basic check for possible
performance impact from the MREs, some confirmation that it really
works as stated in Russian builds, and some assurance from a QA person
that no harmful effects are visible. All of this ought to be attached
to the bug report here.

BTW I am talking about Maxym's 8/21 revised patch, not the additional
JLabel-related patch which will need more work and should be for 4.0 IMHO.

Mailed to reviewers@netbeans.org

> some confirmation that it really
> works as stated in Russian builds

I can confirm, maybe someone else may help?

To confirm, one must:
1. Checkout the fresh translatedfiles_nowww, 
2. apply the patch#2:
http://www.netbeans.org/issues/showattachment.cgi?
attach_id=7171&file=D:\Actions.diff
3. Build Netbeans with translatedfiles in it,
4. Start Netbeans with "-locale ru" passed to runide

He sees:
File menu starts from "underlined Greek F (Phi)",
on the Russian keyboard it corresponds to Latin A,
so Alt+A should work to open File menu.
(See attach#4 - the picture).

Created attachment 7178 [details]
Picture of Russian File menu (Alt+A must work)

OK from performance.
It adds about 10-15 MREs which will fit well under 1ms.
I may be better to add a mnemonic cahhing map to the Actions though.

My review:

The performance of the patch is not problem. The method is invoked
just ten (or so) times during startup and ten times when opening a
menu. Throwing such low number of exceptions should not be problem. 

We cannot integrate
http://www.netbeans.org/issues/showattachment.cgi?attach_id=7171&file=D:\Actions.diff
patch without modification. It uses method from JDK1.4 and NetBeans
are supposed to run on JDK1.3. We need to use reflection or something
like that.

To Maxim: If you think this is a bug and not enhancement than change
it to bug.

Ad. 3.4.1: Russian version is distributed as NBM on autoupdate. I
believe we might  as well distribute update of openide there too. Thus
we will not prevent release 3.4 from finishing and we allow russian
localization to work (even by increasing the download size).

I'm sorry, but I don't think this fix should go into 3.4 at this
point. Hacking around Swing at this level is risky and may cause
undesirable side-effects. I'm afraid there's not enough time to test
it thoroughly unless we want to postpone the release for another week
or two.

IMO, fixing it in trunk and marking it a candidate for 3.4.1 would be
more appropriate.

OK, it's siply too late ;(

Please do not commit into main trunk, it will not compile 
under 1.3.
I will prepare a better patch: cached & JDK1.3 compatible.

Created attachment 7180 [details]
Patch that works on jdk1.3 (does not underline RU characters) and also on jdk1.4 (does underline)

Created attachment 7182 [details]
Patch that works on jdk1.3 & jdk1.4 !!!

Created attachment 7183 [details]
Patch that works on jdk1.3 & jdk1.4 !!!

The last proposed patch (sorry for double-posting)
works BOTH on jdk1.3 & jdk1.4

It's achieved by:
jdk1.3: adding " (<latin char>)" to the text of 
AbstractButton (JMenuItem), and setting this <latin char> 
to be the mnemonics.
jdk1.4: setting <latin char> as mnemonics, and calling 
setDisplayedMnemonicIndex to underline <locale character>.

Please verify and commit into the main trunk.

Do not commit, please,
On JDK1.4 it doesn't work as expected, sorry for 
inconveniece, please

Created attachment 7184 [details]
Under jdk1.3

Created attachment 7185 [details]
Under jdk1.4

Created attachment 7187 [details]
VERIFIED Patch that works both for jdk1.3 & jdk1.4

The latest three attachments:
1. http://www.netbeans.org/issues/showattachment.cgi?
attach_id=7184&file=E:\picture_jdk13.jpg
2. http://www.netbeans.org/issues/showattachment.cgi?
attach_id=7185&file=E:\picture_jdk14.jpg

3. http://www.netbeans.org/issues/showattachment.cgi?
attach_id=7187&file=E:\Actions.verified.diff

Are about the VERIFIED on jdk1.3.1, jdk1.4.0 and 
jdk1.4.1beta patch.
The 3rd is a patch itself.

If you are not bothered with me for today ;), please 
review, check and integrate into the main trunk.

The patch is optimized for speed for English locales:
 it doesn't even try to lookup the bundle if the letter is 
a latin symbol, so no Exceptions and bundle loading are 
needed.

following up on comment below
"Is there any way to make it come into Russian distribution of
     Netbeans 3.4, while not coming into English distrib?" - not
really.

Yes, please make sure that this or any other localized files
do not come into the English builds of Netbeans.

This could also have effects on the Netbeans that is used
by FFJ also - the assumption is that no localized files
are part of a basic build.

Thanks - Ken

I'd add a little optimalization for russian version as well:
use static instance of the method or null, you'll get only
one NSME on 1.3 and will have to lookup the method only once
on 1.4.

To Ken:
> Yes, please make sure that this or any other 
> localized files
> do not come into the English builds of Netbeans.

Localized files (except for Japanese) reside in 
translatedfiles module.

> This could also have effects on the Netbeans that is 
> used by FFJ also - the assumption is that no localized
> files are part of a basic build.

I'm willing to have the Programmatic support integrated 
into Netbeans, not the _ru bundles.

To Petr:

> use static instance of the method or null, you'll get
> only one NSME on 1.3 and will have to lookup the method
> only once on 1.4.

Not sure it's good solution. If a subclass of 
AbstractButton overrides this method, will the reflect 
call the correct method of a subclass?

To All: 
Who will integrate this patch into the trunk? 
I do not have access to openide's CVS.

To Petr:

> If the underlying method is an instance method, it is
> invoked using dynamic method lookup as documented in 
> The Java Language Specification, Second Edition, section
> 15.12.4.4; in particular, overriding based on the
> runtime type of the target object will occur. 

Sorry for not carefully reading JavaDoc on Method.invoke,
but I had to read the above paragraph 3 or 4 times to 
understand, that static Method will be OK.
Thanks for the proposal. 

There're some other perfomance issues on Russian version, 
but this method is called rarely, only when menus are 
created, so throwing such low number of exceptions should 
not be a problem.

Misc. comments:

Rather than using reflection this way, it is now customary to put the
1.4-specific code into a separate class named *14.java, which may use
1.4 APIs. From the main class, try to load and use this class,
catching ClassNotFoundException and LinkageError which should revert
to the 1.3 impl.

Re. integrator: it is assigned to Dafe, so he is responsible unless
someone else accepts it.

BTW Maxym don't use JPEGs for screenshots - use GIF or PNG, which are
non-lossy.

The 1.4 screenshot looks nice!

Has anyone checked whether the non-ASCII mnemonics (Alt-<<keycode>>)
actually work on non-Windows systems? Linux? Solaris? Seems like a
potential trouble area for users of different kinds of keyboards, so
it would be good to check this as soon as possible.

I don't have Linux/Solaris,
but the patch seems to be OS/independent (I think so).

The layout of Russian keyboards is standardized, and 
there's a strong correspondence between Ciryllic & Latin 
letters.

About *14.java : should I change the patch? I didn't know 
about this Netbeans code convention.

It would be nice to use the *14.java style, but it's definitely not
very important. Mainly a style convention, so we don't have to read
dozens of lines of java.lang.reflect.* code every time someone wants
to use a 1.4 API.

To Integrator:
I'll create a new patch which uses *14-style

To Jaroslav Tulach:
I cannot apply the patch you suggested:
Exception: Missing comma.

Any ideas?

BTW:
in Actions.java on line 195:

//Ampersand is last characted, use first character as 
shortcut
...
    item.setMnemonic (0);
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and not
    item.setMnemonic (text.charAt(0));

It's a bug, or comment is outdated?

Created attachment 7199 [details]
FINAL patch, working both for JDK1.3 & JDK1.4

Created attachment 7200 [details]
FINAL file Actions.java - Provided by Jaroslav Tulach

Latest two marked FINAL are really final.

Sorry for providing two files (not one), but I cannot create a single
patch using Netbeans CVS / GUI Style

To integrator:
Please verify that CVS client you use integrates the patch correctly,
because internal Netbeans Patcher produces bad results ;(
I'll file an issue.

This use of reflection is unsafe. The VM could throw a
NoSuchMethodError when the method is *called*, rather than when the
class is *loaded*. This is because of lazy bytecode compilation by
HotSpot when verification is disabled (as it is in release builds).

Use e.g.:

if (actions14 instanceof Map.Entry) {
    actions14.setValue(...);
}
if (actions14 == null) {
    try {
        Class c = Class.forName(...);
        Map.Entry e = (Map.Entry)c.newInstance();
        e.setValue(...);
        actions14 = e;
    } catch (Exception e) {
        actions14 = new Object();
    } catch (LinkageError e) {
        actions14 = new Object();
    }
}
if (!(actions14 instanceof Map.Entry)) {
    // JDK 1.3 style...
}

or similar.

*** Issue 24521 has been marked as a duplicate of this issue. ***

I've started discussion on nbdiscuss on how to properly do 
all this stuff:
Localize mnemonics of Buttons, Labels, etc.

Sure, just nbui (for how it should appear to the user) or nbdev (for
how to implement it) please, rather than nbdiscuss.

Patch integrated into main trunk.
To Maxym: I used console based patch command to apply your patch and
everything should be ok, test build was also ok.

Thanks,

I will reopen 
http://www.netbeans.org/issues/show_bug.cgi?id=26640

and send there a new patch for setLabelText method with similiar way
of setting text,mnemonic & displayedMnemonicIndex

works, thanks, I do work using this fix since 23.08.2002

Checking in openide/src/org/openide/awt/Actions.java;
new revision: 1.64.14.1; previous revision: 1.64

Commited to release341 branch,

Also added a tag release341_base & branched to release341 the file
Actions14.java

Adding a tag is bad, thanks to Jesse for pointing that out, I followed
his recommendations:

Checking in openide/src/org/openide/awt/Actions14.java;
new revision: 1.1.38.1; previous revision: 1.1

Resolved for 3.4.x or earlier, no new info since then -> closing.