diff --git a/modules/controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java b/modules/controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java --- a/modules/controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java +++ b/modules/controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java @@ -29,6 +29,7 @@ import com.sun.javafx.scene.control.ControlAcceleratorSupport; import com.sun.javafx.scene.control.LabeledImpl; import com.sun.javafx.scene.control.skin.Utils; +import javafx.application.Platform; import javafx.collections.ListChangeListener; import javafx.event.ActionEvent; import javafx.scene.control.ContextMenu; @@ -186,9 +187,15 @@ if (popup.isShowing()) { Utils.addMnemonics(popup, getSkinnable().getScene(), getSkinnable().impl_isShowMnemonics()); - } - else { - Utils.removeMnemonics(popup, getSkinnable().getScene()); + } else { + // we wrap this in a runLater so that mnemonics are not removed + // before all key events are fired (because KEY_PRESSED might have + // been used to hide the menu, but KEY_TYPED and KEY_RELEASED + // events are still to be fired, and shouldn't miss out on going + // through the mnemonics code (especially in case they should be + // consumed to prevent them being used elsewhere). + // See JBS-8090026 for more detail. + Platform.runLater(() -> Utils.removeMnemonics(popup, getSkinnable().getScene())); } }); } diff --git a/modules/graphics/src/main/java/com/sun/javafx/scene/KeyboardShortcutsHandler.java b/modules/graphics/src/main/java/com/sun/javafx/scene/KeyboardShortcutsHandler.java --- a/modules/graphics/src/main/java/com/sun/javafx/scene/KeyboardShortcutsHandler.java +++ b/modules/graphics/src/main/java/com/sun/javafx/scene/KeyboardShortcutsHandler.java @@ -38,6 +38,7 @@ import javafx.collections.ObservableMap; import javafx.event.Event; import javafx.scene.Node; +import javafx.scene.input.KeyCode; import javafx.scene.input.KeyCombination; import javafx.scene.input.KeyEvent; import javafx.scene.input.Mnemonic; @@ -54,24 +55,25 @@ private ObservableMap> mnemonics; public void addMnemonic(Mnemonic m) { - ObservableList mnemonicsList = (ObservableList)getMnemonics().get(m.getKeyCombination()); + ObservableList mnemonicsList = getMnemonics().get(m.getKeyCombination()); if (mnemonicsList == null) { - mnemonicsList = new ObservableListWrapper(new ArrayList()); + mnemonicsList = new ObservableListWrapper<>(new ArrayList<>()); getMnemonics().put(m.getKeyCombination(), mnemonicsList); } boolean foundMnemonic = false; - for (int i = 0 ; i < mnemonicsList.size() ; i++) { - if (mnemonicsList.get(i) == m) { + for (Mnemonic mnemonic : mnemonicsList) { + if (mnemonic == m) { foundMnemonic = true; + break; } } - if (foundMnemonic == false) { + if (!foundMnemonic) { mnemonicsList.add(m); } } public void removeMnemonic(Mnemonic m) { - ObservableList mnemonicsList = (ObservableList)getMnemonics().get(m.getKeyCombination()); + ObservableList mnemonicsList = getMnemonics().get(m.getKeyCombination()); if (mnemonicsList != null) { for (int i = 0 ; i < mnemonicsList.size() ; i++) { if (mnemonicsList.get(i).getNode() == m.getNode()) { @@ -83,7 +85,7 @@ public ObservableMap> getMnemonics() { if (mnemonics == null) { - mnemonics = new ObservableMapWrapper>(new HashMap>()); + mnemonics = new ObservableMapWrapper<>(new HashMap<>()); } return mnemonics; } @@ -138,7 +140,7 @@ } @Override - public Event dispatchBubblingEvent(Event event) { + public Event dispatchCapturingEvent(Event event) { /* ** If the key event hasn't been consumed then ** we will process global events in the order : @@ -168,6 +170,18 @@ } } + if (event.getEventType() == KeyEvent.KEY_TYPED) { + if (PlatformUtil.isMac()) { + if (((KeyEvent) event).isMetaDown()) { + processMnemonics((KeyEvent) event); + } + } else { + if (((KeyEvent) event).isAltDown() || isMnemonicsDisplayEnabled()) { + processMnemonics((KeyEvent) event); + } + } + } + /* ** if we're not on mac, and nobody consumed the event, then we should ** check to see if we should highlight the mnemonics on the scene @@ -199,132 +213,141 @@ return event; } - private void processMnemonics(KeyEvent event) { - if (mnemonics != null) { + private void processMnemonics(final KeyEvent event) { + if (mnemonics == null) return; - ObservableList mnemonicsList = null; + // we are going to create a lookup event that is a copy of this event + // except replacing KEY_TYPED with KEY_PRESSED. If we find a mnemonic + // with this lookup event, we will consume the event so that + // KEY_TYPED events are not fired after a mnemonic consumed the + // KEY_PRESSED event. + // We pass in isMnemonicDisplayEnabled() for the altDown test, as if + // mnemonic display has been enabled, we can act as if the alt key is + // being held down. + KeyEvent lookupEvent; + if (event.getEventType() == KeyEvent.KEY_TYPED) { + lookupEvent = new KeyEvent(null, event.getTarget(), KeyEvent.KEY_PRESSED, + " ", + event.getCharacter(), + KeyCode.getKeyCode(event.getCharacter()), + event.isShiftDown(), + event.isControlDown(), + isMnemonicsDisplayEnabled(), + event.isMetaDown()); + } else { + lookupEvent = new KeyEvent(null, event.getTarget(), KeyEvent.KEY_PRESSED, + event.getCharacter(), + event.getText(), + event.getCode(), + event.isShiftDown(), + event.isControlDown(), + isMnemonicsDisplayEnabled(), + event.isMetaDown()); + } - for (Map.Entry> - mnemonic: mnemonics.entrySet()) { - if (!isMnemonicsDisplayEnabled()) { - if (mnemonic.getKey().match(event)) { - mnemonicsList = (ObservableList) mnemonic.getValue(); - break; - } - } - else { + ObservableList mnemonicsList = null; + + for (Map.Entry> mnemonic: mnemonics.entrySet()) { + if (mnemonic.getKey().match(isMnemonicsDisplayEnabled() ? lookupEvent : event)) { + mnemonicsList = mnemonic.getValue(); + break; + } + } + + if (mnemonicsList == null) return; + + /* + ** for mnemonics we need to check if visible and reachable.... + ** if a single Mnemonic on the keyCombo we + ** fire the runnable in Mnemoninic, and transfer focus + ** if there is more than one then we just + ** transfer the focus + ** + */ + boolean multipleNodes = false; + Node firstNode = null; + Mnemonic firstMnemonics = null; + int focusedIndex = -1; + int nextFocusable = -1; + + /* + ** find first focusable node + */ + for (int i = 0 ; i < mnemonicsList.size() ; i++) { + Mnemonic mnemonic = mnemonicsList.get(i); + Node currentNode = mnemonic.getNode(); + + if (firstMnemonics == null && (currentNode.impl_isTreeVisible() && !currentNode.isDisabled())) { + firstMnemonics = mnemonic; + } + + if (currentNode.impl_isTreeVisible() && (currentNode.isFocusTraversable() && !currentNode.isDisabled())) { + if (firstNode == null) { + firstNode = currentNode; + } else { /* - ** Mnemonics display has been enabled, which means - ** we act as is the alt key is being held down. + ** there is more than one node on this keyCombo */ - - KeyEvent fakeEvent = new KeyEvent(null, event.getTarget(), KeyEvent.KEY_PRESSED, - event.getCharacter(), - event.getText(), - event.getCode(), - event.isShiftDown(), - event.isControlDown(), - true, - event.isMetaDown()); - - - if (mnemonic.getKey().match(fakeEvent)) { - mnemonicsList = (ObservableList) mnemonic.getValue(); - break; + multipleNodes = true; + if (focusedIndex != -1) { + if (nextFocusable == -1) { + nextFocusable = i; + } } } } - if (mnemonicsList != null) { + /* + ** one of our targets has the focus already + */ + if (currentNode.isFocused()) { + focusedIndex = i; + } + } + + if (firstNode != null) { + if (!multipleNodes == true) { /* - ** for mnemonics we need to check if visible and reachable.... - ** if a single Mnemonic on the keyCombo we - ** fire the runnable in Mnemoninic, and transfer focus - ** if there is more than one then we just - ** transfer the focus - ** + ** just one target */ - boolean multipleNodes = false; - Node firstNode = null; - Mnemonic firstMnemonics = null; - int focusedIndex = -1; - int nextFocusable = -1; - + firstNode.requestFocus(); + event.consume(); + } + else { /* - ** find first focusable node + ** we have multiple nodes using the same mnemonic. + ** this is allowed for nmemonics, and we simple + ** focus traverse between them */ - for (int i = 0 ; i < mnemonicsList.size() ; i++) { - if (mnemonicsList.get(i) instanceof Mnemonic) { - Node currentNode = (Node)mnemonicsList.get(i).getNode(); - - if (firstMnemonics == null && (currentNode.impl_isTreeVisible() && !currentNode.isDisabled())) { - firstMnemonics = mnemonicsList.get(i); - } - - if (currentNode.impl_isTreeVisible() && (currentNode.isFocusTraversable() && !currentNode.isDisabled())) { - if (firstNode == null) { - firstNode = currentNode; - } - else { - /* - ** there is more than one node on this keyCombo - */ - multipleNodes = true; - if (focusedIndex != -1) { - if (nextFocusable == -1) { - nextFocusable = i; - } - } - } - } - /* - ** one of our targets has the focus already - */ - if (currentNode.isFocused()) { - focusedIndex = i; - } - } + if (focusedIndex == -1) { + firstNode.requestFocus(); + event.consume(); } - - if (firstNode != null) { - if (!multipleNodes == true) { - /* - ** just one target - */ + else { + if (focusedIndex >= mnemonicsList.size()) { firstNode.requestFocus(); event.consume(); } else { - /* - ** we have multiple nodes using the same mnemonic. - ** this is allowed for nmemonics, and we simple - ** focus traverse between them - */ - if (focusedIndex == -1) { - firstNode.requestFocus(); - event.consume(); + if (nextFocusable != -1) { + ((Node)mnemonicsList.get(nextFocusable).getNode()).requestFocus(); } else { - if (focusedIndex >= mnemonicsList.size()) { - firstNode.requestFocus(); - event.consume(); - } - else { - if (nextFocusable != -1) { - ((Node)mnemonicsList.get(nextFocusable).getNode()).requestFocus(); - } - else { - firstNode.requestFocus(); - } - event.consume(); - } + firstNode.requestFocus(); } + event.consume(); } } - if (!multipleNodes && firstMnemonics != null) { - firstMnemonics.fire(); - } + } + } + + if (!multipleNodes && firstMnemonics != null) { + if (event.getEventType() == KeyEvent.KEY_TYPED) { + event.consume(); + } else { + firstMnemonics.fire(); + event.consume(); } } }