8274854: Mnemonics for menu containing numeric text not working

Reviewed-by: aghaisas, kcr

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextBinding.java

@@ -183,25 +183,33 @@ private void parseAndSplit(String s) {
         }
 
         StringBuilder builder = new StringBuilder(s.length());
+        int i = 0;
 
-        for (int i = 0, length = s.length(); i < length; ++i) {
+        // Parse the input string and stop after the first mnemonic.
+        for (int length = s.length(); i < length; ++i) {
             if (isEscapedMnemonicSymbol(s, i)) {
                 builder.append(s.charAt(i++));
+            } else if (isExtendedMnemonic(s, i)) {
+                mnemonic = String.valueOf(s.charAt(i + 2));
+                mnemonicIndex = i;
+                extendedMnemonicText = s.substring(i + 1, i + 4);
+                i += 4;
+                break;
             } else if (isSimpleMnemonic(s, i)) {
                 char c = s.charAt(i + 1);
-                builder.append(c);
                 mnemonic = String.valueOf(c);
                 mnemonicIndex = i;
                 i += 1;
-            } else if (isExtendedMnemonic(s, i)) {
-                mnemonic = String.valueOf(s.charAt(i + 2));
-                extendedMnemonicText = s.substring(i + 1, i + 4);
-                i += 3;
+                break;
             } else {
                 builder.append(s.charAt(i));
             }
         }
 
+        if (s.length() > i) {
+            builder.append(s.substring(i));
+        }
+
         text = builder.toString();
     }
 
@@ -216,21 +224,26 @@ private boolean isEscapedMnemonicSymbol(String s, int position) {
 
     /**
      * Determines whether the string contains a simple mnemonic at the specified position.
+     * A simple mnemonic is any two-character string similar to "_x", where x is not an
+     * underscore or a whitespace character.
      */
     private boolean isSimpleMnemonic(String s, int position) {
         return s.length() > position + 1
             && s.charAt(position) == MNEMONIC_SYMBOL
-            && Character.isAlphabetic(s.charAt(position + 1));
+            && s.charAt(position + 1) != MNEMONIC_SYMBOL
+            && !Character.isWhitespace(s.charAt(position + 1));
     }
 
     /**
      * Determines whether the string contains an extended mnemonic at the specified position.
+     * An extended mnemonic is any four-character string similar to "_(x)", where x is any
+     * character except whitespace.
      */
     private boolean isExtendedMnemonic(String s, int position) {
         return s.length() > position + 3
             && s.charAt(position) == MNEMONIC_SYMBOL
             && s.charAt(position + 1) == '('
-            && Character.isAlphabetic(s.charAt(position + 2))
+            && !Character.isWhitespace(s.charAt(position + 2))
             && s.charAt(position + 3) == ')';
     }
 

modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/behavior/TextBindingTest.java

@@ -0,0 +1,145 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package test.com.sun.javafx.scene.control.behavior;
+
+import com.sun.javafx.scene.control.behavior.TextBinding;
+import javafx.scene.input.KeyCombination;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class TextBindingTest {
+
+    private static void assertKeyCombination(String expected, KeyCombination actual) {
+        if (com.sun.javafx.PlatformUtil.isMac()) {
+            assertSame(KeyCombination.ModifierValue.DOWN, actual.getMeta());
+        } else {
+            assertSame(KeyCombination.ModifierValue.DOWN, actual.getAlt());
+        }
+
+        assertEquals(expected, ((TextBinding.MnemonicKeyCombination)actual).getCharacter());
+    }
+
+    @Test
+    public void testSimpleMnemonicLetter() {
+        var binding = new TextBinding("foo _bar");
+        assertEquals("foo bar", binding.getText());
+        assertEquals("b", binding.getMnemonic());
+        assertKeyCombination("b", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testSimpleMnemonicDigit() {
+        var binding = new TextBinding("foo _1 bar");
+        assertEquals("foo 1 bar", binding.getText());
+        assertEquals("1", binding.getMnemonic());
+        assertKeyCombination("1", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testExtendedMnemonicLetter() {
+        var binding = new TextBinding("foo _(x)bar");
+        assertEquals("foo bar", binding.getText());
+        assertEquals("x", binding.getMnemonic());
+        assertKeyCombination("x", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testExtendedMnemonicUnderscore() {
+        var binding = new TextBinding("foo _(_)bar");
+        assertEquals("foo bar", binding.getText());
+        assertEquals("_", binding.getMnemonic());
+        assertKeyCombination("_", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testExtendedMnemonicClosingBrace() {
+        var binding = new TextBinding("foo _())bar");
+        assertEquals("foo bar", binding.getText());
+        assertEquals(")", binding.getMnemonic());
+        assertKeyCombination(")", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testEscapedMnemonicSymbol() {
+        var binding = new TextBinding("foo __bar");
+        assertEquals("foo _bar", binding.getText());
+        assertNull(binding.getMnemonic());
+        assertNull(binding.getMnemonicKeyCombination());
+        assertEquals(-1, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testWhitespaceIsNotProcessedAsExtendedMnemonic() {
+        var binding = new TextBinding("foo _( ) bar");
+        assertEquals("foo ( ) bar", binding.getText());
+        assertEquals("(", binding.getMnemonic());
+        assertKeyCombination("(", binding.getMnemonicKeyCombination());
+        assertEquals(4, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testUnderscoreNotFollowedByAlphabeticCharIsNotAMnemonic() {
+        var binding = new TextBinding("foo_ bar");
+        assertEquals("foo_ bar", binding.getText());
+        assertNull(binding.getMnemonic());
+        assertNull(binding.getMnemonicKeyCombination());
+        assertEquals(-1, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testUnderscoreAtEndOfTextIsNotAMnemonic() {
+        var binding = new TextBinding("foo_");
+        assertEquals("foo_", binding.getText());
+        assertNull(binding.getMnemonic());
+        assertNull(binding.getMnemonicKeyCombination());
+        assertEquals(-1, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testMnemonicParsingStopsAfterFirstSimpleMnemonic() {
+        var binding = new TextBinding("_foo _bar _qux");
+        assertEquals("foo _bar _qux", binding.getText());
+        assertEquals("f", binding.getMnemonic());
+        assertKeyCombination("f", binding.getMnemonicKeyCombination());
+        assertEquals(0, binding.getMnemonicIndex());
+    }
+
+    @Test
+    public void testMnemonicParsingStopsAfterFirstExtendedMnemonic() {
+        var binding = new TextBinding("_(x)foo _bar _qux");
+        assertEquals("foo _bar _qux", binding.getText());
+        assertEquals("x", binding.getMnemonic());
+        assertKeyCombination("x", binding.getMnemonicKeyCombination());
+        assertEquals(0, binding.getMnemonicIndex());
+    }
+
+}

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java

@@ -2064,7 +2064,7 @@ public class LabelSkinTest {
      ***************************************************************************/
 
     @Test
-    public void mnemonicSymbolIsRemovedFromDisplayedText() {
+    public void testMnemonicIsProcessedWhenParsingIsEnabled() {
         label.setMnemonicParsing(true);
         label.setText("foo _bar");
         label.autosize();
@@ -2073,7 +2073,7 @@ public void mnemonicSymbolIsRemovedFromDisplayedText() {
     }
 
     @Test
-    public void extendedMnemonicIsRemovedFromDisplayedText() {
+    public void testExtendedMnemonicIsProcessedWhenParsingIsEnabled() {
         label.setMnemonicParsing(true);
         label.setText("foo _(x)bar");
         label.autosize();
@@ -2082,41 +2082,23 @@ public void extendedMnemonicIsRemovedFromDisplayedText() {
     }
 
     @Test
-    public void escapedMnemonicSymbolIsRetainedInDisplayedText() {
-        label.setMnemonicParsing(true);
-        label.setText("foo __bar");
+    public void testMnemonicsAreNotProcessedWhenParsingIsDisabled() {
+        label.setMnemonicParsing(false);
+        label.setText("foo _bar");
         label.autosize();
         skin.updateDisplayedText();
         assertEquals("foo _bar", LabelSkinBaseShim.getText(label).getText());
     }
 
     @Test
-    public void escapedMnemonicSymbolIsNotProcessedWhenMnemonicParsingIsDisabled() {
+    public void testEscapedMnemonicSymbolIsNotProcessedWhenParsingIsDisabled() {
         label.setMnemonicParsing(false);
         label.setText("foo __bar");
         label.autosize();
         skin.updateDisplayedText();
         assertEquals("foo __bar", LabelSkinBaseShim.getText(label).getText());
     }
 
-    @Test
-    public void underscoreNotFollowedByAlphabeticCharIsNotAMnemonic() {
-        label.setMnemonicParsing(true);
-        label.setText("foo_ bar");
-        label.autosize();
-        skin.updateDisplayedText();
-        assertEquals("foo_ bar", LabelSkinBaseShim.getText(label).getText());
-    }
-
-    @Test
-    public void underscoreAtEndOfTextIsNotAMnemonic() {
-        label.setMnemonicParsing(true);
-        label.setText("foo_");
-        label.autosize();
-        skin.updateDisplayedText();
-        assertEquals("foo_", LabelSkinBaseShim.getText(label).getText());
-    }
-
     public static final class LabelSkinMock extends LabelSkin {
         boolean propertyChanged = false;
         int propertyChangeCount = 0;