8216971: [macosx swing] For JCheckBoxMenuItem actionPerformed() is called twice, when apple.laf.useScreenMenuBar=true and modifier is InputEvent.META_DOWN_MASK

Reviewed-by: psadhukhan, kaddepalli

src/java.desktop/macosx/native/libawt_lwawt/awt/CMenuItem.m

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2019, 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
@@ -70,7 +70,7 @@
     AWT_ASSERT_APPKIT_THREAD;
     JNIEnv *env = [ThreadUtilities getJNIEnv];
     JNF_COCOA_ENTER(env);
-    
+
     // If we are called as a result of user pressing a shortcut, do nothing,
     // because AVTView has already sent corresponding key event to the Java
     // layer from performKeyEquivalent.
@@ -81,45 +81,26 @@
     // from this "frameless" menu, because there are no active windows. This
     // means we have to handle it here.
     NSEvent *currEvent = [[NSApplication sharedApplication] currentEvent];
+
+    if ([currEvent type] == NSKeyDown) {
+        // The action event can be ignored only if the key window is an AWT window.
+        // Otherwise, the action event is the only notification and must be processed.
+        NSWindow *keyWindow = [NSApp keyWindow];
+        if (keyWindow != nil && [AWTWindow isAWTWindow: keyWindow]) {
+            return;
+        }
+    }
+
     if (fIsCheckbox) {
         static JNF_CLASS_CACHE(jc_CCheckboxMenuItem, "sun/lwawt/macosx/CCheckboxMenuItem");
         static JNF_MEMBER_CACHE(jm_ckHandleAction, jc_CCheckboxMenuItem, "handleAction", "(Z)V");
-        
+
         // Send the opposite of what's currently checked -- the action
         // indicates what state we're going to.
         NSInteger state = [sender state];
         jboolean newState = (state == NSOnState ? JNI_FALSE : JNI_TRUE);
         JNFCallVoidMethod(env, fPeer, jm_ckHandleAction, newState);
-    }
+    } else {
-    else {
-        if ([currEvent type] == NSKeyDown) {
-            // Event available through sender variable hence NSApplication
-            // not needed for checking the keyboard input sans the modifier keys
-            // Also, the method used to fetch eventKey earlier would be locale dependent
-            // With earlier implementation, if MenuKey: e EventKey: ा ; if input method
-            // is not U.S. (Devanagari in this case)
-            // With current implementation, EventKey = MenuKey = e irrespective of
-            // input method
-            NSString *eventKey = [sender keyEquivalent];
-            // Apple uses characters from private Unicode range for some of the
-            // keys, so we need to do the same translation here that we do
-            // for the regular key down events
-            if ([eventKey length] == 1) {
-                unichar origChar = [eventKey characterAtIndex:0];
-                unichar newChar =  NsCharToJavaChar(origChar, 0);
-                if (newChar == java_awt_event_KeyEvent_CHAR_UNDEFINED) {
-                    newChar = origChar;
-                }
-                eventKey = [NSString stringWithCharacters: &newChar length: 1];
-            }
-            // The action event can be ignored only if the key window is an AWT window.
-            // Otherwise, the action event is the only notification and must be processed.
-            NSWindow *keyWindow = [NSApp keyWindow];
-            if (keyWindow != nil && [AWTWindow isAWTWindow: keyWindow]) {
-                return;
-            }
-        }
-        
         static JNF_CLASS_CACHE(jc_CMenuItem, "sun/lwawt/macosx/CMenuItem");
         static JNF_MEMBER_CACHE(jm_handleAction, jc_CMenuItem, "handleAction", "(JI)V"); // AWT_THREADING Safe (event)
 
@@ -129,7 +110,6 @@
         JNFCallVoidMethod(env, fPeer, jm_handleAction, UTC(currEvent), javaModifiers); // AWT_THREADING Safe (event)
     }
     JNF_COCOA_EXIT(env);
-    
 }
 
 - (void) setJavaLabel:(NSString *)theLabel shortcut:(NSString *)theKeyEquivalent modifierMask:(jint)modifiers {

test/jdk/javax/swing/JMenuItem/8216971/DoubleActionTest.java

@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2019, 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.
+ *
+ * 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.
+ */
+
+/**
+ * @test
+ * @key headful
+ * @bug 8216971
+ * @summary For JCheckBoxMenuItem actionPerformed() is called twice, when
+ *  apple.laf.useScreenMenuBar=true and modifier is InputEvent.META_DOWN_MASK
+ * @library /test/lib
+ * @run main DoubleActionTest
+ */
+
+import javax.swing.JFrame;
+import javax.swing.JMenu;
+import javax.swing.JMenuBar;
+import javax.swing.JCheckBoxMenuItem;
+import javax.swing.SwingUtilities;
+import javax.swing.AbstractAction;
+import javax.swing.Action;
+import javax.swing.KeyStroke;
+import java.awt.event.ActionEvent;
+import java.awt.event.InputEvent;
+import java.awt.event.KeyEvent;
+import java.awt.Robot;
+import jdk.test.lib.Platform;
+
+public class DoubleActionTest {
+
+    private static int metaDownCount = 0;
+    private static JFrame frame;
+    private static final int ORIGIN_X = 200;
+    private static final int ORIGIN_Y = 200;
+
+    public static void main(String[] args) throws Exception {
+        if (!System.getProperty("os.name").startsWith("Mac")) {
+            System.out.println("This test is only for Mac OS, passed " +
+            "automatically on other platforms.");
+            return;
+        }
+
+        try {
+            System.setProperty("apple.laf.useScreenMenuBar", "true");
+
+            SwingUtilities.invokeAndWait(DoubleActionTest::createAndShowGUI);
+
+            Robot robot = new Robot();
+            robot.setAutoDelay(100);
+            testKeyPress(robot);
+            robot.delay(1000);
+
+        } finally {
+            SwingUtilities.invokeAndWait(()->frame.dispose());
+            if (metaDownCount != 1) {
+                throw new RuntimeException("Test Failed: actionPerformed is called twice");
+            }
+        }
+    }
+
+    private static void createAndShowGUI() {
+        frame = new JFrame();
+        final JMenuBar menubar = new JMenuBar();
+        final JMenu fileMenu = new JMenu("OPEN ME");
+        final MyAction myAction = new MyAction();
+        final JCheckBoxMenuItem menuItem = new JCheckBoxMenuItem(myAction);
+
+        fileMenu.add(menuItem);
+        menubar.add(fileMenu);
+        frame.setJMenuBar(menubar);
+        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
+        frame.setBounds(ORIGIN_X, ORIGIN_X, 200, 200);
+        frame.setVisible(true);
+    }
+
+    private static class MyAction extends AbstractAction {
+
+        MyAction() {
+            putValue(Action.NAME, "HIT MY ACCELERATOR KEY");
+            putValue(Action.ACCELERATOR_KEY, KeyStroke.getKeyStroke(KeyEvent.VK_E, InputEvent.META_DOWN_MASK));
+        }
+
+        @Override
+        public void actionPerformed(final ActionEvent e) {
+            System.out.println("Action! called with modifiers: " + e.getModifiers() + "\n" + e);
+            metaDownCount++;
+        }
+    }
+
+    private static void testKeyPress(Robot robot) throws Exception {
+        robot.mouseMove(ORIGIN_X + 50, ORIGIN_Y + 50);
+        robot.waitForIdle();
+        robot.keyPress(KeyEvent.VK_META);
+        robot.keyPress(KeyEvent.VK_E);
+        robot.keyRelease(KeyEvent.VK_E);
+        robot.keyRelease(KeyEvent.VK_META);
+        robot.waitForIdle();
+    }
+}