8356652: Input field ignores custom input source characters #1805
Conversation
|
👋 Welcome back mfox! A progress list of the required criteria for merging this PR into |
|
@beldenfox This change is no longer ready for integration - check the PR body for details. |
Webrevs
|
|
/reviewers 2 |
|
@andy-goryachev-oracle |
|
What's interesting, not all Hebrew keys produce Latin text. For example, 'f' key does generate // Longer strings are sent out above as commits.? With the fix, both keyman and standard macOS text input work (including Japanese IME popup). |
|
Noticed a bit of a problem. To reproduce, install Himyarit Musnad keyboard https://keyman.com/keyboards/himyarit_musnad typing 'h' causes this character to appear: 𐩱
(I am using the monkey tester, but it will work with any application that uses a TextArea) |
The same happens without the fix. For some reason events for the keys not mapped on the active keyman keyboard still pass to the component - at least partially. That affects the character composition. |
|
You are right, @azuev-java ! Looks like this is a separate bug, where the same text involving surrogate pairs is rendered differently by the TextArea:
|
|
The N key in the Himyarit Musnad layout shouldn't generate a character (just a beep). Under the hood glass is seeing a keyDown: event for the N key (same as it would for a U.S. English layout) so it generates a KeyEvent. I can't figure out a good way of avoiding this, any fix I can think of would also affect keys like Tab where we have to generate a KeyEvent. I'll continue to think about this but for now this PR gets Keyman working at some minimal level and I have my doubts that we can do much better. |
|
Created https://bugs.openjdk.org/browse/JDK-8357070 for the rendering bug. |
Do you think it's a bug in keyman or our code? I head the beep (correctly) but then n appears, despite the keyman onscreen keyboard showing an empty key. If it is our code, should there be some additional logic? |
|
FYI @sgschantz 👆 |
Keyman is definitely sending us a keyDown: for the N key. I'm still trying to figure out how to ignore it without also dropping other key strokes that we need. |
|
@beldenfox i mentioned the mac lead who should be able to comment. Does the Devanagari case in https://bugs.openjdk.org/browse/JDK-8195675?focusedId=14148577&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14148577 work ? That's the one I was testing for the corresponding jdk issue |
|
@srl295 |
|
It’s still there if you click through.
https://keyman.com/go/package/download/isis_devanagari?version=2.1.1&tier=stable
|
|
OK, here is the test result with keyman ISIS Devanagari looks like: with the fix - Typing "j" displays ज without the fix - Typing "j" displays j |
|
A second shot at getting the Keyman input method working. In this version we detect that Keyman is active at the very beginning of key down handling. If Keyman is active we drop any key event that doesn’t lead to a call to one of the NSTextInputClient methods. Character-generating keys are caught inside Under the hood Keyman is using some Roman layout (for me it’s US English) and all keyboard shortcuts will be based on that layout and not the Keyman layout. So if you test with Dvorak shortcuts like Cmd+X require you to hit X in the US English (QWERTY) location, not the Dvorak one. This is a general problem with Keyman that affects all applications. A keystroke that generate more than one UTF-16 code unit will be processed as a JavaFX InputMethodEvent rather than a KeyEvent. This is old logic that’s been in JavaFX for a long time that I didn’t want to re-visit. You’ll see this with the Himyarit Musnad layout. I tried to isolate the Keyman logic as much as possible. I’ve done some testing with Himyarit Musnad, Hebrew, and ISIS-Devanagari. It won’t work the newer Guatami Devanagari since it keeps sending backspaces to erase previous characters. That layout doesn’t seem to work well anywhere. |
I do know that keyboards in general need to be able to interact with the existing context, that is, to convert h + i into ひ erasing the "h". I'm not sure of the correct mechanism here but just wanted to note that. |
|
As far as I can tell, this version works as expected. I found one irregularity with the ISIS Devanagari keyboard: unmapped 'w' produces w symbol, unmapped punctuation keys produce corresponding symbols, but unmapped 'x' produces no input. This could be the keyman problem though. It is weird that we have to add third-party specific code to JavaFX as a workaround - why doesn't macOS provide a general purpose API for this? |
| @@ -777,6 +803,9 @@ - (void)doCommandBySelector:(SEL)aSelector | |||
| // According to Apple an NSResponder will send this up the responder chain | |||
| // but a text input client should not. So we ignore this which avoids an | |||
| // annoying beep. | |||
| if (keymanActive) { | |||
There was a problem hiding this comment.
I noticed the latest code does not beep. Were the beeps produced earlier expected for unmapped keys?
There was a problem hiding this comment.
I'm not sure where the beeps are coming from. I still hear a beep when I press N on the Himyarit Musnad layout but I don't hear a beep when I press X on the ISIS-Devanagari layout. Both are unused keys but one produces a beep and the other doesn't.
Normally an input method is designed for a specific language and is bundled with a collection of keyboard layouts. The input method is an active blob of code but the keyboard layouts are basically big passive tables mapping hardware key positions to characters. It appears that Keyman is trying to use one input method that works with all of its layouts. It's not producing the corresponding data tables; the OS just sees the same Roman table when it generates events. We're basically working around that mis-match. To really do this right Keyman would either have to create new layout tables on-the-fly (assuming they can, I've never tried it) or use a low-level event tap to re-write the keyboard events (assuming they can, I've never tried it). I suspect the API's are available but are probably a devil to get right. |
|
Requesting review from @sgschantz (Keyman lead for macOS) |
Under the hood the Keyman input method appears as a US English keyboard layout. The characters attached to an NSEvent are always US English Roman even if the selected Keyman layout is, say, Hebrew or Dvorak. Keyman sends the correct Hebrew or Dvorak character to insertText:replacementRange: instead.
This PR special-cases the Keyman layout, detecting it using the same method that AWT does. When Keyman is active Glass records the insertText: character and uses that when sending out KeyEvents.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1805/head:pull/1805$ git checkout pull/1805Update a local copy of the PR:
$ git checkout pull/1805$ git pull https://git.openjdk.org/jfx.git pull/1805/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1805View PR using the GUI difftool:
$ git pr show -t 1805Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1805.diff
Using Webrev
Link to Webrev Comment