On Wed, 15 Dec 2021 21:30:41 GMT, Martin Fox <d...@openjdk.org> wrote:
>> The algorithm in `KeyCharacterCombination.match` relies on the call >> `Toolkit.getKeyCodeForChar` which is difficult to implement correctly. It >> defies the way most keyboard API’s work and no platform has got it right >> yet. In particular the Mac and Linux implementations have to resort to a >> brute-force approach which monitors keystrokes to learn the relationship >> between keys and characters. >> >> This PR introduces an alternative mechanism which directly asks the platform >> whether a given key can generate a specific character. It also allows the >> platform to attach identifying key information to each KeyEvent to make it >> easier to answer the question (much, much easier). >> >> This is mostly dumb plumbing. On the front-end there’s a new call >> `View.notifyKeyEx` that takes an additional platform-specific `hardwareCode` >> parameter. It also returns a boolean indicating whether the event was >> consumed or not so I can fix JDK-8087863. If you want to follow the path >> visit the files in this order: >> >> View.java >> GlassViewEventHandler.java >> TKSceneListener.java >> Scene.java >> >> The `KeyEvent` class has been expanded with an additional `hardwareCode` >> member that can only be accessed internally. See KeyEvent.java and >> KeyEventHelper.java. >> >> On the back-end `KeyCharacterCombination.match` calls a new routine >> `Toolkit.getKeyCanGenerateCharacter` which unpacks the `KeyEvent` >> information and sends it on to the Application. The default implementation >> falls back to the old `getKeyCodeForChar` call but platform specific >> Applications can send it on to the native glass code. >> >> KeyCharacterCombination.java >> Toolkit.java >> QuantumToolkit.java >> Application.java >> GtkApplication.java >> >> The glass code can use the `hardwareCode` to answer the question directly. >> It also has enough information to fall back on the old `getKeyCodeForChar` >> logic while also enabling the keypad (a common complaint is that Ctrl+’+’ >> only works on the main keyboard and not the keypad, see JDK-8090275). >> >> This PR improves the situation for key events generated by keystrokes. >> Manually constructed key events won’t work any better or worse than they >> already do. Based on the bug database I don't think this is an issue. > > I've decided that if this PR goes forward we should abandon #672. The changes > in #672 would be bypassed for KeyEvents generated by physical keystrokes. For > a while I thought it would be useful for handling KeyEvents created via the > API but have decided against it. > > I've been going back and forth on how to handle synthetic events created > using the KeyEvent constructor i.e. events that don't have a hardwareCode > attached. One option is to just route them through the old > `getKeyCodeForChar` machinery to maintain the current level of support. > Unfortunately this varies widely, from mostly working on Windows to almost > never working on Mac. If we just maintain what we've got I would ditch #672 > since it would be fine-tuning a feature on Windows that's largely busted on > other platforms. > > The other option is to dive in and make synthetic events work as well as > possible on every platform. In this PR `getKeyCanGenerateCharacter` has a > KeyCode in hand and can use the existing Robot code to map that to a physical > key. It wouldn't be entirely reliable (particularly on Mac and Linux) but it > would be easy and improve support for synthetic events enough to forestall > bugs like [JDK-8087486](https://bugs.openjdk.java.net/browse/JDK-8087486). > And it would be behavior I could write automated and manual tests for. > > I don't think synthetic events are all that useful for triggering > KeyCharacterCombinations (you have to know the current keyboard layout to > accurately target punctuation or symbols) but they exist and it's probably > best to get them to a testable state. This means fixing the Robot code which > has bugs on every platform but that was already on my to-do list and I've > prototyped most of the code. I'll get the Robot bugs entered in the next few > days. @beldenfox would it be possible to make the same change to the .classpath file (though it might create a merge conflict later)? <?xml version="1.0" encoding="UTF-8"?> <classpath> <classpathentry combineaccessrules="false" kind="src" path="/base"> <attributes> <attribute name="module" value="true"/> </attributes> </classpathentry> <classpathentry combineaccessrules="false" kind="src" path="/controls"> <attributes> <attribute name="module" value="true"/> </attributes> </classpathentry> <classpathentry combineaccessrules="false" kind="src" path="/graphics"> <attributes> <attribute name="module" value="true"/> </attributes> </classpathentry> <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"> <attributes> <attribute name="module" value="true"/> </attributes> </classpathentry> <classpathentry excluding=".classpath|.project|.settings" kind="src" output="bin" path=""/> <classpathentry kind="output" path="bin"/> </classpath> ------------- PR Comment: https://git.openjdk.org/jfx/pull/694#issuecomment-1518206571
