Fix: Keyboard shortcuts cannot be edited in Preferences (closes #14237) by AmandaBalderas20 · Pull Request #14312 · JabRef/jabref

AmandaBalderas20 · 2025-11-14T01:26:07Z

I fixed an issue where keyboard shortcuts could not be edited in:
Preferences → Keyboard shortcuts.

The KeyEvent was being consumed by global application shortcuts before reaching
the shortcut editor in the KeyBindingsTab. As a result, pressing keys did not
update the selected shortcut, and changes were not persisted.

This PR adds:

A key event filter in KeyBindingsTab to ensure the TreeTableView receives key presses.
Logic to forward the event to the ViewModel (setNewBindingForCurrent).
Correct persistence of edited shortcuts by syncing the repository into GuiPreferences.
An updated JUnit test verifying the binding update behavior.

Steps to test

Open JabRef → Preferences → Keyboard shortcuts.
Select any shortcut entry (e.g., “Close library”).
Press a new key combination (e.g., Ctrl+Shift+L).
Click Save.
Restart JabRef.
Verify that:
- The new shortcut appears in the list.
- The shortcut works in the application (e.g., Ctrl+Shift+L closes the library).

Screenshots visible changes are not required since UI remains the same.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
I manually tested my changes in running JabRef (always required)
I added JUnit tests for changes (if applicable)
[/] I added screenshots in the PR description (not applicable — UI did not change)
I described the change in CHANGELOG.md
[/] I checked the user documentation: no updates required since UI workflow remains identical

github-actions · 2025-11-14T01:26:18Z

Hey @AmandaBalderas20!

Thank you for contributing to JabRef! Your help is truly appreciated ❤️.

We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful.

Please re-check our contribution guide in case of any other doubts related to our contribution workflow.

Siedlerchr · 2025-11-15T13:10:32Z

Thanks a lot for your PR, please check the failing tests in KeyBindingsTabModelTest. Maybe they need to be adapted?

AmandaBalderas20 · 2025-11-17T17:59:23Z

I validated the functionality by running the tests individually and as a group using the following commands:

./gradlew :jabgui:test --tests "org.jabref.gui.preferences.keybindings.KeyBindingViewModelTest"

./gradlew :jabgui:test --tests "org.jabref.gui.preferences.keybindings.KeyBindingViewModelTest.resetToDefault"

./gradlew :jabgui:test --tests "org.jabref.gui.preferences.keybindings.KeyBindingViewModelTest.verifyStoreSettingsWritesChanges"

All tests executed successfully. I am not sure if there are other test cases or validation steps that I should do for this area, but everything appears correct.

Siedlerchr · 2025-11-17T19:17:24Z

+
+        Optional<String> saved = prefsRepo.get(binding);
+        assertTrue(saved.isPresent());
+        assertTrue("ctrl+shift+L".equalsIgnoreCase(saved.get()));


Can you make this an assertEquals? This makes it easer to see the diff when tests fails

You can just do assertEquals(Optional.of("...."), saved)

Siedlerchr · 2025-11-17T20:00:24Z

Tess work for me on mac as well.

koppor · 2025-11-17T20:02:54Z

[/] I described the change in CHANGELOG.md (not applicable — minor behavior fix)

No - you fixed an issue. Please add a line - you can ref the issue there #14237

Siedlerchr · 2025-11-17T22:15:45Z

KeyBindingsTabModelTest fails. I cannot verfiy it because there is an assumeFalse(OSX) inside, so the tests do not run on mac

koppor · 2025-11-20T13:50:14Z

Locally

> Task :jabgui:test
org.jabref.gui.keyboard.KeyBindingsTabModelTest setSingleKeyBindingToDefault() FAILED

  org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
      at app/org.jabref/org.jabref.gui.keyboard.KeyBindingsTabModelTest.setSingleKeyBindingToDefault(KeyBindingsTabModelTest.java:173)

org.jabref.gui.keyboard.KeyBindingsTabModelTest setAllKeyBindingsToDefault() FAILED

  org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
      at app/org.jabref/org.jabref.gui.keyboard.KeyBindingsTabModelTest.setAllKeyBindingsToDefault(KeyBindingsTabModelTest.java:140)

Not sure how to proceed

A: Think why tests could fail and fix the tests/logic
B: Disable tests - and document why the test could fail
C: Just disable tests

AmandaBalderas20 · 2025-11-20T19:47:42Z

I reviewed it and I think the failing tests in KeyBindingsTabModelTest expect hasChanged to be false after resetting a key binding.
After fixing #14237, changing a binding and then resetting it still counts as a modification, so the correct final value is true.
I can update those two tests to assert true instead of false to match the new behavior. Is that okay?

Siedlerchr · 2025-11-20T20:03:40Z

It would be great if you add this as comment to the test somehow

AmandaBalderas20 · 2025-11-20T20:09:52Z

Okay, so I’ll update the tests and add a comment explaining why hasChanged() is now expected to be true after a reset (because of the fix for #14237)

…ion comment

koppor · 2025-11-21T21:14:37Z

-        assertTrue(saved.isPresent());
-        assertTrue("ctrl+shift+L".equalsIgnoreCase(saved.get()));
+        assertEquals(Optional.of("ctrl+shift+L"), saved);


Thank you for this, too!

Scroogeee · 2025-11-24T21:11:00Z

Does not seem to fix the issue on

JabRef 6.0-alpha.194--2025-11-24--5e08c82
Mac OS X 26.1 aarch64
Java 24.0.2
JavaFX 25.0.1+3

New keycombinations are ignored. How are they supposed to be entered?

Siedlerchr · 2025-11-28T19:10:36Z

@Scroogeee
Might be a bit unintuitive.
Click on the clear icon (the broom), then press your new key combination

Scroogeee · 2025-11-29T19:52:39Z

I did try doing it on JabRef 6.0-PullRequest14057.266--2025-11-28--a581582
Mac OS X 26.1 aarch64
Java 25.0.1
JavaFX 25.0.1+3;
and
JabRef 6.0-alpha.226--2025-11-29--9087e17
Mac OS X 26.1 x86_64
Java 24.0.2
JavaFX 25.0.1+3

the broom clears the combination, but the new key combination is ignored

Siedlerchr · 2025-12-03T18:39:50Z

@Scroogeee Sorry for the late reply. For me it worked. Just maybe a thing, if you want to have a shortcut using command key, you need to press ctrl key instead. It will be automatically mapped to command under the hood

koppor · 2025-12-03T21:47:17Z

@Scroogeee Sorry for the late reply. For me it worked. Just maybe a thing, if you want to have a shortcut using command key, you need to press ctrl key instead. It will be automatically mapped to command under the hood

Should we add this as hint at the dialog somehow? It is strange that keys are remapped somehow? I assume, this is only the case at JabRef - or is this a general issue happening in all apps?

Siedlerchr · 2025-12-03T22:27:50Z

Yeah somehow at the dialog this would be helpful.
The key remapping internally is natural. As a Mac user, you don't want to press ctrl + c you press cmd + c but as jabref keybindings are designed for every OS

jabref/jabgui/src/main/java/org/jabref/gui/keyboard/KeyBindingRepository.java

Lines 154 to 163 in 518f61f

    
           public Optional<KeyCombination> getKeyCombination(KeyBinding bindName) { 
        
               String binding = get(bindName.getConstant()); 
        
               if (binding.isEmpty()) { 
        
                   return Optional.empty(); 
        
               } 
        
               if (OS.OS_X) { 
        
                   binding = binding.replace("ctrl", "meta"); 
        
               } 
        
               return Optional.of(KeyCombination.valueOf(binding)); 
        
           }

But it seems like javafx supports shortcut key for such cases

The shortcut modifier is used to represent the modifier key which is used commonly in keyboard shortcuts on the host platform. This is for example control on Windows and meta (command key) on Mac. By using shortcut key modifier developers can create platform independent shortcuts. So the "Shortcut+C" key combination is handled internally as "Ctrl+C" on Windows and "Meta+C" on Mac.

https://openjfx.io/javadoc/24/javafx.graphics/javafx/scene/input/KeyCombination.html

Siedlerchr · 2025-12-21T22:56:54Z

Key remapping is no longer need thanks to javafx shortcuts, folllow up in #14684

Fix: editing keyboard shortcuts in Preferences (closes JabRef#14237)

70219ab

github-actions Bot added the first contrib label Nov 14, 2025

AmandaBalderas20 and others added 3 commits November 13, 2025 20:41

chore: apply OpenRewrite automatic fixes

1f176de

chore: restore unrelated files after rewriteRun cleanup

19d5a4e

Merge branch 'main' into fix-14237-keyboard-shortcuts

327a8ff

Siedlerchr added the status: changes-required Pull requests that are not yet complete label Nov 15, 2025

Merge branch 'main' into fix-14237-keyboard-shortcuts

0df1a2a

github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Nov 17, 2025

Siedlerchr reviewed Nov 17, 2025

View reviewed changes

InAnYan added 5 commits November 17, 2025 21:08

chore(changelog): add changelog entry

149ecbf

fix(tests): disable test on CI

80866f5

chore: fix submodules

5b10586

chore: fix tests

ee7fffb

chore: fix submodules

25863bf

Siedlerchr previously approved these changes Nov 17, 2025

View reviewed changes

Siedlerchr enabled auto-merge November 17, 2025 20:39

koppor added the status: awaiting-second-review For non-trivial changes label Nov 20, 2025

test: update keybinding tests after fix JabRef#14237 and add explanat…

8eb5456

…ion comment

auto-merge was automatically disabled November 20, 2025 21:00
Head branch was pushed to by a user without write access

AmandaBalderas20 dismissed Siedlerchr’s stale review via 8eb5456 November 20, 2025 21:00

Merge branch 'main' into fix-14237-keyboard-shortcuts

5484974

koppor reviewed Nov 21, 2025

View reviewed changes

koppor approved these changes Nov 21, 2025

View reviewed changes

koppor merged commit aae5b96 into JabRef:main Nov 21, 2025
50 of 51 checks passed

github-actions Bot mentioned this pull request Nov 21, 2025

Unable to change any keyboard shortcut #14237

Closed

2 tasks

koppor mentioned this pull request Nov 27, 2025

"Keyboard shortcuts changed" appears at all preference changes #14453

Closed

Siedlerchr mentioned this pull request Dec 23, 2025

Fix restart warning when nothing changes #14709

Merged

3 tasks

Uh oh!

Conversation

AmandaBalderas20 commented Nov 14, 2025 • edited by Siedlerchr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

Mandatory checks

Uh oh!

github-actions Bot commented Nov 14, 2025

Hey @AmandaBalderas20!

Uh oh!

Siedlerchr commented Nov 15, 2025

Uh oh!

AmandaBalderas20 commented Nov 17, 2025

Uh oh!

Siedlerchr Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Siedlerchr Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Nov 17, 2025

Uh oh!

koppor commented Nov 17, 2025

Uh oh!

Siedlerchr commented Nov 17, 2025

Uh oh!

koppor commented Nov 20, 2025

Uh oh!

AmandaBalderas20 commented Nov 20, 2025

Uh oh!

Siedlerchr commented Nov 20, 2025

Uh oh!

AmandaBalderas20 commented Nov 20, 2025

Uh oh!

koppor Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Scroogeee commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr commented Nov 28, 2025

Uh oh!

Scroogeee commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr commented Dec 3, 2025

Uh oh!

koppor commented Dec 3, 2025

Uh oh!

Siedlerchr commented Dec 3, 2025

Uh oh!

Siedlerchr commented Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AmandaBalderas20 commented Nov 14, 2025 •

edited by Siedlerchr

Loading

Scroogeee commented Nov 24, 2025 •

edited

Loading

Scroogeee commented Nov 29, 2025 •

edited

Loading