RFR: 8278924: [Linux] Robot key test can fail if multiple keyboard layouts are installed
The Robot implementation on Linux did not consult the current layout when mapping from a KeyCode to a hardware code. Internally it retrieved results for all the layouts but just picked the first one it saw leading to random effects. Though not part of the original bug report, the code also ignored the shift level when choosing which result to pick. On a French layout the dollar sign is on two keys (AltGr 4 is the second one) and the code could choose either one. Same is true for pound. This PR consults the current layout and only on shift level 0 which is the same level used in get_glass_key to figure out which KeyCode to assign when generating a KeyEvent. - Commit messages: - Robot no longer gets confused by multiple layouts Changes: https://git.openjdk.java.net/jfx/pull/718/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=718&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278924 Stats: 86 lines in 3 files changed: 73 ins; 8 del; 5 mod Patch: https://git.openjdk.java.net/jfx/pull/718.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/718/head:pull/718 PR: https://git.openjdk.java.net/jfx/pull/718
Re: RFR: 8150709: Mac OSX and German Keyboard Layout (Y/Z) [v6]
On Tue, 21 Dec 2021 16:42:40 GMT, Martin Fox wrote: >> This PR adds code to ensure that KeyCodeCombinations match KeyEvents as >> expected by more accurately mapping from a Mac key code to a Java key code >> based on the user’s active keyboard layout (the existing code assumes a US >> QWERTY layout). The new code first identifies a set of Mac keys which can >> produce different characters based on the user’s keyboard layout. A Mac key >> code outside that area is processed exactly as before. For a key inside the >> layout-sensitive area the code calls UCKeyTranslate to translate the key to >> an unshifted ASCII character based on the active keyboard and uses that to >> determine the Java key code. >> >> When performing the reverse mapping for the Robot the code first uses the >> old QWERTY mapping to find a candidate key. If it lies in the >> layout-sensitive area the code then scans the entire area calling >> UCKeyTranslate until it finds a match. If the key lies outside the >> layout-sensitive area it’s processed exactly as before. >> >> There are multiple duplicates of these bugs logged against Mac applications >> built with JavaFX. >> >> https://bugs.openjdk.java.net/browse/JDK-8090257 Mac: Inconsistent KeyEvents >> with alternative keyboard layouts >> https://bugs.openjdk.java.net/browse/JDK-8088120 [Accelerator, Mac] CMD + Z >> accelerator is not working with French keyboard >> https://bugs.openjdk.java.net/browse/JDK-8087915 Mac: accelerator doesn't >> take into account azerty keyboard layout >> https://bugs.openjdk.java.net/browse/JDK-8150709 Mac OSX and German Keyboard >> Layout (Y/Z) > > Martin Fox has updated the pull request incrementally with one additional > commit since the last revision: > > Bug fixes in Robot. Ensure symbols uncovered using Option are ignored. I've been working on automated tests for keyboard handling but am thinking of dropping them. IMHO the tests need to cover punctuation and symbols (that's where the bugs are) but testing those keys on a random keyboard layout is complicated. I'm running into bugs on Mac and Linux that I can fix but that are probably not worth the trouble. For example, with this PR dead keys are miscoded (what should be DEAD_CIRCUMFLEX is CIRCUMFLEX) and that can lead an automated test to accidentally trigger a dead key state (this can also happen on Windows though PR #702 should fix that). The fixes to get the codes right on Mac aren't deep but I don't think they would be of any benefit to JavaFX developers so I would be asking for review cycles just to appease the automated tests. The manual test attached to this thread is more likely to find relevant bugs. - PR: https://git.openjdk.java.net/jfx/pull/425
Re: RFR: 8244234: MenuButton: NPE on removing from scene with open popup [v2]
> The NPE occurs when the skinnable is removed from the scene while the popup > is showing. > The MenuButtonSkinBase, when popup becomes hidden, tries to remove Mnemonics > from the scene and runs into NPE. > To avoid NPE a null-check is added to the 'showing' listener. > > Since the mnemonics cannot be removed from the scene in standard way, when > popup becomes hidden. > They are now also removed from the scene, when the skinnable is removed from > it. eduardsdv has updated the pull request incrementally with one additional commit since the last revision: 8244234: Add changes suggested in review and add file header to the test - Changes: - all: https://git.openjdk.java.net/jfx/pull/713/files - new: https://git.openjdk.java.net/jfx/pull/713/files/0c598695..6749816f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=713&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=713&range=00-01 Stats: 36 lines in 2 files changed: 25 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jfx/pull/713.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/713/head:pull/713 PR: https://git.openjdk.java.net/jfx/pull/713
Re: RFR: 8244234: MenuButton: NPE on removing from scene with open popup
On Tue, 11 Jan 2022 14:42:19 GMT, eduardsdv wrote: > The NPE occurs when the skinnable is removed from the scene while the popup > is showing. > The MenuButtonSkinBase, when popup becomes hidden, tries to remove Mnemonics > from the scene and runs into NPE. > To avoid NPE a null-check is added to the 'showing' listener. > > Since the mnemonics cannot be removed from the scene in standard way, when > popup becomes hidden. > They are now also removed from the scene, when the skinnable is removed from > it. I added changes suggested in the review and added the file header to the test class. Please review. - PR: https://git.openjdk.java.net/jfx/pull/713
Re: RFR: 8244234: MenuButton: NPE on removing from scene with open popup
On Tue, 11 Jan 2022 14:42:19 GMT, eduardsdv wrote:
> The NPE occurs when the skinnable is removed from the scene while the popup
> is showing.
> The MenuButtonSkinBase, when popup becomes hidden, tries to remove Mnemonics
> from the scene and runs into NPE.
> To avoid NPE a null-check is added to the 'showing' listener.
>
> Since the mnemonics cannot be removed from the scene in standard way, when
> popup becomes hidden.
> They are now also removed from the scene, when the skinnable is removed from
> it.
The fix looks OK, but, a few cleanups are required.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
line 157:
> 155:
> 156: // We only need to remove the mnemonics from the old
> scene,
> 157: // they will be added to the new one as soon as the pop
> becomes visible again.
typo : pop --> popup
modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java
line 250:
> 248: @Override protected double computeMinWidth(double height, double
> topInset, double rightInset, double bottomInset, double leftInset) {
> 249: return leftInset
> 250:+ label.minWidth(height)
Unintended spacing changes are there in computePrefHeight, computePrefWidth,
computeMinHeight and computeMinWidth methods. Please revert these unrelated
spacing changes.
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/MenuButtonSkinBaseTest.java
line 1:
> 1: package test.javafx.scene.control.skin;
Please add the missing copyright header.
-
PR: https://git.openjdk.java.net/jfx/pull/713
