Bug ID: JDK-8093578 Menubar menu item accelerators work only upon second pressing

JDK 8
8u20Fixed

I don't see any harm in registering the key from this method. +1
28-05-2014
Webrev: http://cr.openjdk.java.net/~anthony/g-36-menuAccel-RT-37026.0/ I've tested this with a test application and ensured that w/o the fix the Cmd+[ only works when I press it for the second time, while with the fix the accelerator starts working immediately after starting the application.
28-05-2014
diff -r c470fa477657 modules/graphics/src/main/native-glass/mac/GlassView3D.m --- a/modules/graphics/src/main/native-glass/mac/GlassView3D.m +++ b/modules/graphics/src/main/native-glass/mac/GlassView3D.m @@ -453,6 +453,8 @@ - (BOOL)performKeyEquivalent:(NSEvent *)theEvent { KEYLOG("performKeyEquivalent"); + [GlassApplication registerKeyEvent:theEvent]; + // Crash if the FS window is released while performing a key equivalent // Local copy of the id keeps the retain/release calls balanced. id fsWindow = [self->_delegate->fullscreenWindow retain];
28-05-2014
1. I still want to call registerKeyEvent from performKeyEquivalent in Glass though. The performKeyEquivalent is basically an equivalent for the keyDown events. The former are used for special key combinations (like Cmd+a), while the latter - for regular characters (like typing the plain letter 'a'). So registering the key equivalent events with the keyCode<->character map in Glass looks reasonable.
28-05-2014
2. As for this issue, I see that the SG code tries to translate accelerator when processing every key event. Naturally, pressing modifier keys also generates key events, hence the calls to getKeyCodeForCharacter(). I think this behavior is correct so I won't be filing an issue to fix it.
28-05-2014
3. (continued...) I've filed RT-37307 to evaluate the issue #3.
28-05-2014
3. (continued...) The brackets are listed in the KeyCode class with names "Open Bracket" and "Close Bracket" correspondingly. So, if I use this name when installing an accelerator as follows: menu11.setAccelerator(KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+Open Bracket")); then everything starts working as expected w/o any changes to the FX code. While this does look like a workaround, using "Open Bracket" instead of a plain character "[" seems very odd.
28-05-2014
3. The difference between the cases is that Cmd+[ uses a KeyCharacterCombination, while Cmd+a uses a KeyCodeCombination instance as a key in the accelerators table. The latter doesn't call getKeyCodeForChar(), but instead compares the event's key code with a key code previously stored in the KeyCombination instance itself. It is the KeyCombination.valueOf() method that decides which descendant of the KeyCombiantion class to use depending on whether the passed in shortcut string can be translated to a key code directly. The translation is performed by the KeyCode class, and it does seem to list the OPEN_ and CLOSE_BRACKET characters with their key codes, however, the translation fails for these types of shortcuts. I'll file a separate bug against Scenegraph to investigate this issue.
28-05-2014
From the log above I see the following issues: 1. The performKeyEquivalent doesn't register the key event with the dynamic lookup table, and hence the event handler in FX code fails to invoke the menu item action for the first time. Later the keyDown event is registered and the key code can be looked up in the table. So when I invoke the accelerator for the second time, it works fine. Adding a registerKeyEvent call to the performKeyEquivalent handler seems to resolve the issue. 2. SG and/or Controls code call getKeyCodeForChar() when I only press the modifier key alone. This doesn't seem to make sense and will need to be investigated (as a P4 issue, I suppose). 3. It seems that processing of both Cmd+a and Cmd+[ results in the same behavior wrt. the lookup table. Yet, the Cmd+a accelerator fires the event from the start, i.e. it doesn't require the mapping between the key code and the character to be present in the table. This looks odd.
28-05-2014
Here's what's happening from Glass perspective: ======== Cmd + a ========= // Pressing and holding Cmd alone: 1. getKeyCodeForChar('[' (== "[")) --> 0x0 (val = 0) // Pressing and releasing 'a': -- performKeyEquivalent: 'a' 2. getKeyCodeForChar('[' (== "[")) --> 0x0 (val = 0) Copy - action called -- keyDown: 'a' 3. registerKeyEvent: keyCode=0 char='a' (the "Copy..." string indicates that the menu item action listener has been invoked). ======== Cmd + [ ========= // Pressing and holding Cmd for the first time: 1. getKeyCodeForChar('[' (== "[")) --> 0x0 (val = 0) // Pressing and releasing '[': -- performKeyEquivalent: '[' 2. getKeyCodeForChar('[' (== "[")) --> 0x0 (val = 0) -- keyDown: '[' 3. registerKeyEvent: keyCode=33 char='[' // Pressing and holding Cmd for the second time: 4. getKeyCodeForChar('[' (== "[")) --> 0x2187 (val = 33) -> javaKeyCode = 33 // Pressing and releasing '[' again: -- performKeyEquivalent: '[' 5. getKeyCodeForChar('[' (== "[")) --> 0x2187 (val = 33) -> javaKeyCode = 33 New - action called -- keyDown: '[' 6. registerKeyEvent: keyCode=33 char='['
28-05-2014
Thanks for testing this, Paul. All these keys ([]-='" etc.) are so-called OEM keys, so the events generated by the native system for these keys may be slightly different from normal keys. I'll investigate this issue. Note that given that this didn't work in 2.x, I think this issue cannot be considered a regression since we're talking about a new behavior implemented for FX 8 only.
21-05-2014
@Anthony: I just ran it using JDK 7 java -version: 1.7.0_55 javafx.runtime.version: 2.2.55-b13 Neither shortcut+] nor shortcut+[ works _at all_ (as in never). Following Jonathan's lead, I tried another set of keys (right and left arrows) with the shortcut modifier. They work as expected; they work the first time pressed, and each subsequent time. I just now tried shortcut+= (next) and shortcut+- (previous). They do not work _at all_. So, you can add those two key codes to the list of those which have problems.
20-05-2014
I can't tell. The Application.getKeyCodeForChar() was implemented on Mac only in FX 8, and this code hasn't changed since. So both FX 8 and 8u20 should fail the same way. But I'm not sure if this works with FX 2.x. Paul, could you please try running your app with the latest JDK 7u/FX 2.x and see if the problem is reproducible?
20-05-2014
Is this a regression?
20-05-2014
The getKeyCodeForChar() on Mac is implemented using a table that is built dynamically. We add the mapping to the table on each keyDown: call before sending any events to FX, so in theory the getKeyCodeForChar() should return the right thing from a key event handler that processes the accelerator. We need to instrument the code in GlassApplication.m +registerKeyEvent and +getKeyCodeForChar to understand what is going wrong here.
20-05-2014
Anthony is the guy you want here. There were some changes to handle international keyboards but I believe these were on Windows. In any case, we need to determine whether this is a regression and increase the priority appropriately.
20-05-2014
I'll add that if I test with other keys, (e.g. KeyCombination.keyCombination("shortcut+A")) that the accelerator works as expected the first time, so there is something special about the square brackets (and possibly other symbols).
20-05-2014
I booted up my mac to look into this issue, and I can reproduce it. It seems to me that the following is happening: 1) The accelerators are being properly installed into the Scene accelerators list. 2) The events are being properly delivered to the Scene when you push cmd+[ or cmd+] keyboard combinations. 3) When the events are received, they go to KeyboardShortcutsHandler#processAccelerators(KeyEvent). 4) Inside this method, we call 'accelerator.getKey().match(event)'. 5) Inside the match method of KeyCharacterCombination, we compare the code of the event to the toolkit code for the character stored in the KeyCharacterCombination. 6) To me, it seems as if the call to Toolkit.getToolkit().getKeyCodeForChar(..) fails the first time it is called for each character (at least for the [ and ] characters). The first time cmd+[ or cmd+] are hit, the toolkit returns 0 for the character value. If I dig down into this getKeyCodeForChar method, I eventually end up in GlassKey.m, and unfortunately at this point I become unstuck, as I'm not familiar with Objective-C. Hopefully Steve, Kevin, or Martin might be able to take a look from here, as it seems glass-related, rather than controls-related?
20-05-2014
Regardless of which approach I use -- the static constant (1, KeyCombination.SHORTCUT_DOWN) or "shortcut" (2, "shortcut+]") -- I get the same result on the Mac: first press, nothing; subsequent presses work as expected. As I see it (see logging results below), my verbose approach using the constant (eliminates typos, compiler checking) (1) and your more direct string approach (2, typo prone) are the same. Just a note: Using slf4j (log4j) for logging ... log.debug("KeyCombination.SHORTCUT_DOWN: {}", KeyCombination.SHORTCUT_DOWN); prints KeyCombination.SHORTCUT_DOWN: Shortcut and log.debug("KeyCombination.SHORTCUT_DOWN + \"+]\": {}", KeyCombination.SHORTCUT_DOWN + "+]"); prints KeyCombination.SHORTCUT_DOWN + "+]": Shortcut+] and finally log.debug("KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + \"+]\"): {}", KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+]")); prints KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+]"): Shortcut+']' In the last one, the key code "]" is wrapped in single quotes. Seems odd, but I can't step through the javafx-rt.jar source code (the library is not built with debug lines, source, var on?) to see where this is happening.
12-05-2014
I tested this on my Windows machine and cannot reproduce the issue. I note two things that may be of interest to you: 1) There seems to be a typo in your KeyCombination code for the accelerators: KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+]") Note the additional '+' character inside the string - this seems unnecessary........UNLESS..... 2) You rewrite the code a little bit simpler as the following: KeyCombination.keyCombination("shortcut+]") Are you able to give these a try, and if the issue persists let me know and I'll attempt to reproduce the issue on a Mac? My gut feeling is that this issue is real on Mac, but may be resolved if you follow either 1) or 2) above. This doesn't explain why it didn't work the first time and does subsequently however.
11-05-2014
file: /org/pf/app/AcceleratorTestApp.java package org.pf.app; import javafx.application.Application; import javafx.fxml.FXMLLoader; import javafx.scene.Scene; import javafx.scene.layout.Pane; import javafx.stage.Stage; public class AcceleratorTestApp extends Application{ static final String MAIN_SCENE_FILENAME = "/org/pf/view/AcceleratorTest.fxml"; public static void main(String[] args) { System.out.printf("java -version: %s", System.getProperty("java.version")); Application.launch(args); } @Override public void start(Stage stage) throws Exception { try { FXMLLoader loader = getLoader(); Pane parentNode = (Pane) loader.load(); Scene scene = new Scene(parentNode); stage.setScene(scene); stage.setResizable(true); stage.sizeToScene(); stage.show(); } catch (Exception ex) { System.out.println(ex); } } protected FXMLLoader getLoader() { return new FXMLLoader(this.getClass().getResource(MAIN_SCENE_FILENAME)); } } ** file: /org/pf/view/AcceleratorTestController.java package org.pf.view; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.event.ActionEvent; import javafx.fxml.FXML; import javafx.scene.control.ComboBox; import javafx.scene.control.MenuItem; import javafx.scene.control.SingleSelectionModel; import javafx.scene.input.KeyCombination; public class AcceleratorTestController { @FXML protected MenuItem nextEventMenuItem; @FXML protected MenuItem previousEventMenuItem; @FXML protected ComboBox<String> eventChooser; @FXML void initialize() throws Exception { configureAccelerators(); configureChooser(); } private void configureChooser() { ObservableList<String> items = FXCollections.observableArrayList(); for(int i = 1; i < 21; i++) { items.add("Event " + i); } eventChooser.setItems(items); eventChooser.getSelectionModel().select(0); } protected void configureAccelerators() { nextEventMenuItem.setAccelerator(KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+]")); previousEventMenuItem.setAccelerator(KeyCombination.keyCombination(KeyCombination.SHORTCUT_DOWN + "+[")); } @FXML void nextEvent(ActionEvent actionEvent) { SingleSelectionModel<String> selectionModel = eventChooser.getSelectionModel(); if (selectionModel.isSelected(eventChooser.getItems().size() - 1)) { selectionModel.selectFirst(); } else { selectionModel.selectNext(); } actionEvent.consume(); } @FXML void previousEvent(ActionEvent actionEvent) { SingleSelectionModel<String> selectionModel = eventChooser.getSelectionModel(); if (selectionModel.isSelected(0)) { selectionModel.selectLast(); } else { eventChooser.getSelectionModel().selectPrevious(); } actionEvent.consume(); } } ** file: /org/pf/view/AcceleratorTest.fxml <?xml version="1.0" encoding="UTF-8"?> <?import java.lang.?> <?import javafx.geometry.?> <?import javafx.scene.control.?> <?import javafx.scene.layout.?> <AnchorPane id="AnchorPane" maxHeight="1.7976931348623157E308" maxWidth="1.7976931348623157E308" minHeight="-Infinity" minWidth="-Infinity" prefHeight="221.0" prefWidth="487.0" style=" -fx-border-color: rgba(232,232,232,0.5); -fx-border-width: 1px; -fx-border-style: solid;" xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.pf.view.AcceleratorTestController"> <children> <VBox id="VBox" spacing="0.0" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> <children> <MenuBar AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0"> <menus> <Menu mnemonicParsing="false" text="Navigate"> <items> <MenuItem fx:id="nextEventMenuItem" mnemonicParsing="false" onAction="#nextEvent" text="Next event" /> <MenuItem fx:id="previousEventMenuItem" mnemonicParsing="false" onAction="#previousEvent" text="Previous event" /> </items> </Menu> </menus> </MenuBar> <VBox id="VBox" alignment="CENTER" prefWidth="-1.0" style="-fx-background-color: rgba(232,232,232,0.5); " VBox.vgrow="NEVER"> <children> <HBox maxWidth="1.7976931348623157E308" prefHeight="-1.0" VBox.vgrow="NEVER"> <children> <Label alignment="CENTER" maxWidth="1.7976931348623157E308" style="-fx-font-size: 16px; " text="Events" HBox.hgrow="ALWAYS"> <HBox.margin> <Insets bottom="8.0" top="4.0" /> </HBox.margin> </Label> </children> </HBox> <HBox alignment="CENTER" VBox.vgrow="NEVER"> <children> <ComboBox fx:id="eventChooser" maxWidth="1.7976931348623157E308" minWidth="-Infinity" prefWidth="-1.0" HBox.hgrow="NEVER" /> </children> </HBox> </children> <padding> <Insets bottom="12.0" /> </padding> </VBox> </children> </VBox> </children> </AnchorPane>
09-05-2014
Attachments have been disabled. Could you paste the source code in a comment?
08-05-2014