Fix closing of combo box popup on Escape key

[Devansh-18155]

PR Description

This pull request fixes an issue where pressing the Escape key while a combo box or drop-down list popup (such as a CheckComboBox) is open within a dialog causes the entire dialog (e.g. JabRef Preferences) to close immediately instead of closing only the drop-down menu. We updated event filtering and key handlers in BaseDialog, FXDialog, and PreferencesDialogView to detect active popups and consume the Escape key event when a drop-down menu is showing.

Related issues and pull requests

Closes #16596

Steps to test

1. Launch JabRef (./gradlew :jabgui:run).
2. Go to Options -> Preferences -> Entry types.
3. Under "Required and optional fields", click on the Field property drop-down menu (CheckComboBox) so the list expands.
4. Press Escape.
5. Observe that only the drop-down popup list closes, while the Preferences dialog stays open.
6. Press Escape a second time to verify that the Preferences dialog closes as expected.

Screenshots

After Escape

AI usage

• AI tool: Google Antigravity
• Model ID: Gemini 3.6 Flash
• Disclosure: AI was used for pair-programming assistance to analyze JavaFX key event propagation and refactor the popup state logic across dialogs. I have reviewed, understood, and take full ownership of all AI-assisted code changes.

Checklist

• I own the copyright of the code submitted and I license it under the MIT license
• If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
• I manually tested my changes in running JabRef (always required)
• I added screenshots in the PR description (if change is visible to the user)
• [/] I added JUnit tests for changes (if applicable)
• I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
• I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Prevent dialogs from closing on Escape when a combo box popup is open

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

AI Description

• Detect active JavaFX popups when Escape is pressed in dialogs.
• Consume Escape to close only the dropdown popup, not the dialog.
• Document the UX requirement and add a changelog entry for #16596.

Diagram

graph TD
  U([User presses Esc]) --> KB["KeyBindingRepository: CLOSE"] --> D["Dialog handlers (BaseDialog/FXDialog)"] --> PD{"�Popup showing?"}
  PD -->|Yes| C["Consume Esc; let popup close"]
  PD -->|No| X["Close dialog"]
  PD --> P[("PopupWindow(s)")]

  subgraph Legend
    direction LR
    _user([User action]) ~~~ _svc["Handler/logic"] ~~~ _dec{"Decision"} ~~~ _db[(Window/Popup)]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Handle Escape in ComboBox/CheckComboBox popup skin

• ➕ Fix is localized to the control that owns the popup
• ➕ Avoids global Window scanning and dialog-level coupling
• ➖ Harder to apply consistently across different control types (ChoiceBox, custom controls)
• ➖ Skin/behavior hooks can be brittle across JavaFX versions

2. Use a global Scene-level key event dispatcher with popup priority

• ➕ Single place to define Escape precedence (popup first, then dialog)
• ➕ Potentially consistent across all windows/dialogs
• ➖ More invasive and higher risk of unintended side-effects
• ➖ Still needs reliable popup detection and careful event consumption rules

Recommendation: The chosen approach (dialog-level detection + consuming Escape when a popup is showing) is a pragmatic, cross-control fix with limited blast radius. The main risk is correctness of popup detection and event timing; the added KEY_PRESSED filter that records popup state before the close handler runs is a reasonable mitigation.

Files changed (5) +58 / -3

Bug fix (3) +48 / -3

FXDialog.javaSuppress dialog close on Escape when a popup was open +16/-1

Suppress dialog close on Escape when a popup was open

• Tracks whether a popup was showing at the moment Escape (CLOSE keybinding) was pressed. If so, consumes the key event instead of closing the JavaFX Alert window.

jabgui/src/main/java/org/jabref/gui/FXDialog.java

PreferencesDialogView.javaAvoid closing Preferences dialog when a dropdown popup is active +6/-1

Avoid closing Preferences dialog when a dropdown popup is active

• Updates Preferences dialog key handlers to skip closing on the CLOSE keybinding if a popup is currently showing. Keeps existing special handling for list/table/tree focused targets.

jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java

BaseDialog.javaAdd popup detection and consume Escape to prevent premature dialog close +26/-1

Add popup detection and consume Escape to prevent premature dialog close

• Adds a static helper to detect showing PopupWindow instances (excluding Tooltip). Records popup state on KEY_PRESSED and consumes the CLOSE event if a popup was open, preventing the dialog from closing.

jabgui/src/main/java/org/jabref/gui/util/BaseDialog.java

Documentation (2) +10 / -0

CHANGELOG.mdDocument Escape-with-open-dropdown dialog-close fix +1/-0

Document Escape-with-open-dropdown dialog-close fix

• Adds a changelog entry describing that Escape now closes only the dropdown popup rather than the entire dialog. References issue #16596.

CHANGELOG.md

ux.mdAdd UX requirement for Escape behavior with combo box popups +9/-0

Add UX requirement for Escape behavior with combo box popups

• Introduces a new UX requirement stating that Escape should close an open combo box/drop-down popup without closing the enclosing dialog. Marks the requirement as needing implementation.

docs/requirements/ux.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Popup check on release ✓ Resolved 🐞 Bug ≡ Correctness

Description

PreferencesDialogView guards its ListView Escape-close path by checking isPopupShowing() on
KEY_RELEASED, which can become false after the popup closes during KEY_PRESSED handling. In that
case, closeDialog() can still run even though a popup was open when Escape was pressed.

Code

jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java[R83-86]

if (preferences.getKeyBindingRepository().checkKeyCombinationEquality(KeyBinding.CLOSE, key)) {
-                this.closeDialog();
+                if (!BaseDialog.isPopupShowing()) {
+                    this.closeDialog();
+                }

Evidence

The ListView close logic runs on KEY_RELEASED and checks popup state at that time, while the PR’s
core fix elsewhere explicitly snapshots popup visibility on KEY_PRESSED to avoid state changes
during the Escape dispatch. This inconsistency means the ListView path can still close after a popup
has been dismissed by the Escape press.

jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java[81-88]
jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java[100-110]
jabgui/src/main/java/org/jabref/gui/util/BaseDialog.java[30-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PreferencesDialogView` has an Escape close path on `preferenceTabList.setOnKeyReleased(...)` that checks popup visibility too late (on release). If a popup closes during key-press processing, the release-time check can miss it and still close the dialog.
### Issue Context
This PR already uses a press-time snapshot pattern (`popupWasShowingOnKeyPress`) in `BaseDialog`/`FXDialog` to handle popup state changes during the Escape event dispatch.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java[81-88]
- jabgui/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java[100-110]
- jabgui/src/main/java/org/jabref/gui/util/BaseDialog.java[30-47]
### Suggested fix
Implement the same press-time snapshot logic for the `preferenceTabList` path:
- Add a boolean field in `PreferencesDialogView` (e.g., `popupWasShowingOnKeyPressForTabList`).
- In a `KEY_PRESSED` filter/handler for `preferenceTabList` (or dialog pane), when CLOSE matches, set the flag to `BaseDialog.isPopupShowing(...)` (scoped if you apply finding #1).
- In the existing `setOnKeyReleased` handler, if the flag was true, reset it and consume/return (do not close). Otherwise close as before.
Alternative: switch the ListView handler to `setOnKeyPressed` and use the press-time snapshot + consume pattern consistently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[InAnYan]

There should be impl link

[Devansh-18155]

Done! Added the impl requirement tag to FXDialog above dialogWindow.getScene().setOnKeyPressed(..)

[InAnYan]

Same remarque as here: https://github.com/JabRef/jabref/pull/16620/changes#r3816213614

[Devansh-18155]

Moved the impl tag in FXDialog.java above if (popupWasShowingOnKeyPress) as well.

[InAnYan]

Here also should be impl link

[Devansh-18155]

Yeah i have put the impl link above getDialogPane().getScene().setOnKeyPressed() in previous change.

[InAnYan]

I would actually put it above the if (popupWasShowingOnKeyPress) {, because this listener handles some other stuff too unrelated to the requirement

[Devansh-18155]

Yes it should be, So as per your request i have put it above (popupWasShowingOnKeyPress) { ,

[InAnYan]

Custom entry types.

That's WRONG name. The preferences section is named Entry types 😅

[InAnYan]

Sorry, Windows 11, doesn't work with the steps to test that you provided in the PR description

[Devansh-18155]

Custom entry types.

That's WRONG name. The preferences section is named Entry types 😅

Ohh my bad it is Entry types😅, I have updated in pr description

[Devansh-18155]

Sorry, Windows 11, doesn't work with the steps to test that you provided in the PR description

Try testing with these updated steps:

1. Open JabRef.
2. Go to Options -> Preferences
3. Select Entry types from the left navigation panel.
4. Under the fields table, click the Field property drop-down (CheckComboBox) so the drop-down list opens.
5. Press Esc.

Expected behavior:

• First Esc press: Only the drop-down list closes; the Preferences dialog remains open.
• Second Esc press: The Preferences dialog closes.

Please let me know if it works for you now on Windows 11!

[subhramit]

please fix the merge conflicts

[github-actions]

Do not request reviews if changes are required.
Address the changes first.

[github-actions]

Your pull request conflicts with the target branch.

Please merge upstream/main with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line.