Fix delete entry keybindings bug (#1815)

* Fix delete entry keybindings bug

* Fix comments and unnecessary class variables

CHANGELOG.md

@@ -50,12 +50,13 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - Selecting an entry in the search result Window will now select the correct entry in the bib file
 - Entries in the SearchResultDialog are now converted to Unicode
 - Suggestions in the autocomplete (search) are now in Unicode
-- Fixed NullPointerException when opening search result window for an untitled database 
+- Fixed NullPointerException when opening search result window for an untitled database
 - Fixed entry table traversal with Tab (no column traversal thus no double jump)
 - Fixed [#1757](https://github.com/JabRef/jabref/issues/1757): Crash after saving illegal argument in entry editor
 - Fixed [#1663](https://github.com/JabRef/jabref/issues/1663): Better multi-monitor support
 - Fixed [#1882](https://github.com/JabRef/jabref/issues/1882): Crash after saving illegal bibtexkey in entry editor
 - Fixed field `location` containing only city is not exported correctly to MS-Office 2007 xml format
+- Fixed [#1235](https://github.com/JabRef/jabref/issues/1235): Modified Key bindings do not work correctly
 - Fixed field `key` field is not exported to MS-Office 2008 xml format
 - Fixed [#1181](https://github.com/JabRef/jabref/issues/1181) and [#1504](https://github.com/JabRef/jabref/issues/1504): Improved "Normalize to BibTeX name format": Support separated names with commas and colons. Considered name affixes such as "Jr".
 - Fixed download files failed silently when an invalid directory is selected

src/main/java/net/sf/jabref/gui/keyboard/KeyBindingsListener.java

@@ -2,6 +2,9 @@
 
 import java.awt.event.KeyAdapter;
 import java.awt.event.KeyEvent;
+import java.util.ArrayList;
+import java.util.Locale;
+import java.util.stream.Collectors;
 
 /**
  * respond to grabKey and display the key binding
@@ -16,51 +19,93 @@ public KeyBindingsListener(KeyBindingTable table) {
 
     @Override
     public void keyPressed(KeyEvent evt) {
-        // first check if anything is selected if not the return
+
+        boolean isFunctionKey = false;
+        boolean isEscapeKey = false;
+        boolean isDeleteKey = false;
+
+        // first check if anything is selected if not then return
         final int selRow = table.getSelectedRow();
         boolean isAnyRowSelected = selRow >= 0;
         if (!isAnyRowSelected) {
             return;
         }
 
-        final String modifier = KeyEvent.getKeyModifiersText(evt.getModifiers());
+        final String modifier = getModifierText(evt);
 
         // VALIDATE code and modifier
         // all key bindings must have a modifier: ctrl alt etc
         if ("".equals(modifier)) {
             int kc = evt.getKeyCode();
-            boolean isFunctionKey = (kc >= KeyEvent.VK_F1) && (kc <= KeyEvent.VK_F12);
-            boolean isEscapeKey = kc == KeyEvent.VK_ESCAPE;
-            boolean isDeleteKey = kc == KeyEvent.VK_DELETE;
+            isFunctionKey = (kc >= KeyEvent.VK_F1) && (kc <= KeyEvent.VK_F12);
+            isEscapeKey = kc == KeyEvent.VK_ESCAPE;
+            isDeleteKey = kc == KeyEvent.VK_DELETE;
             if (!(isFunctionKey || isEscapeKey || isDeleteKey)) {
-                return; // need a modifier except for function, escape and delete keys
+                // need a modifier except for function, escape and delete keys
+                return;
             }
         }
 
-        final String code = KeyEvent.getKeyText(evt.getKeyCode());
-        // second key cannot be a modifiers
-        if ("Tab".equals(code)
-                || "Backspace".equals(code)
-                || "Enter".equals(code)
-                || "Space".equals(code)
-                || "Ctrl".equals(code)
-                || "Shift".equals(code)
-                || "Alt".equals(code)) {
+        int code = evt.getKeyCode();
+        String newKey;
+        //skip the event triggered only by a modifier, tab, backspace or enter because these normally have preset
+        // functionality if they alone are pressed
+        if (code == KeyEvent.VK_ALT ||
+                code == KeyEvent.VK_TAB ||
+                code == KeyEvent.VK_BACK_SPACE ||
+                code == KeyEvent.VK_ENTER ||
+                code == KeyEvent.VK_SPACE ||
+                code == KeyEvent.VK_CONTROL ||
+                code == KeyEvent.VK_SHIFT ||
+                code == KeyEvent.VK_META) {
             return;
         }
-
-        // COMPUTE new key binding
-        String newKey;
         if ("".equals(modifier)) {
-            newKey = code;
+            if (isFunctionKey) {
+                newKey = KeyEvent.getKeyText(code);
+            } else if (isEscapeKey) {
+                newKey = "ESCAPE";
+            } else if (isDeleteKey) {
+                newKey = "DELETE";
+            } else {
+                return;
+            }
         } else {
-            newKey = modifier.toLowerCase().replace("+", " ") + " " + code;
+            newKey = modifier.toLowerCase(Locale.ENGLISH) + " " + KeyEvent.getKeyText(code);
         }
-
-        // SHOW new key binding
+        //SHOW new key binding
         //find which key is selected and set its value
         table.setValueAt(newKey, selRow, 1);
         table.revalidate();
         table.repaint();
     }
+
+    /**
+     * Collects th English translations of all modifiers and returns them separated by a space
+     *
+     * @param evt the KeyEvent that was triggered to set the KeyBindings
+     * @return a String containing the modifier keys text
+     */
+    private String getModifierText(KeyEvent evt) {
+        ArrayList<String> modifiersList = new ArrayList<>();
+
+        if (evt.isControlDown()) {
+            modifiersList.add("ctrl");
+        }
+        if (evt.isAltDown()) {
+            modifiersList.add("alt");
+        }
+        if (evt.isShiftDown()) {
+            modifiersList.add("shift");
+        }
+        if (evt.isAltGraphDown()) {
+            modifiersList.add("alt gr");
+        }
+        if (evt.isMetaDown()) {
+            modifiersList.add("meta");
+        }
+        //Now build the String with all the modifier texts
+        String modifiersAsString = modifiersList.stream().collect(Collectors.joining(" "));
+        return modifiersAsString;
+    }
 }