Context menu appearing on empty library table body

[faneeshh]

Related issues and pull requests

Closes #15384

PR Description

When the library is empty, right clicking the table body was triggering the column chooser context menu. This was happening because the placeholder VBox has no context menu handler, so the ContextMenuEvent bubbled up to the MainTable level
handler which showed the menu unconditionally for any non StackPane target.

Steps to test

1. Open an empty library
2. Try right clicking on the table body, the context menu shouldn't appear on it anymore.
3. Now try right clicking the header column menu and the context menu should open up and clicking on any other part except the table body should close it.

Checklist

• 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 (if change is visible to the user)
• [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
• 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]

Review Summary by Qodo

Fix context menu appearing on empty library table body

🐞 Bug fix

Walkthroughs

Description

• Fix context menu appearing on empty library table body
• Replace StackPane target check with proper column header detection
• Add recursive parent traversal to identify TableColumnHeader nodes
• Prevent context menu from showing on non-header table areas

Diagram

flowchart LR
  A["Context Menu Event"] --> B["Check Event Target"]
  B --> C["isColumnHeaderTarget Method"]
  C --> D["Traverse Node Parents"]
  D --> E{"TableColumnHeader Found?"}
  E -->|Yes| F["Show Context Menu"]
  E -->|No| G["Hide Context Menu"]

Loading

File Changes

1. jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java 🐞 Bug fix +16/-2

Replace StackPane check with column header detection

• Replaced StackPane instanceof check with new isColumnHeaderTarget() method
• Added import for Node and TableColumnHeader classes
• Removed import for StackPane class
• Implemented recursive parent traversal to properly identify column header targets
• Updated context menu display logic to use the new validation method

jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java

---

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. JavaFX skin API dependency ☑ 🐞 Bug ⚙ Maintainability

Description

MainTableHeaderContextMenu now relies on javafx.scene.control.skin.TableColumnHeader to detect
header clicks, coupling behavior to JavaFX implementation details that are not stable API. This
increases the risk of compile/runtime breakage during JavaFX upgrades and makes the code harder to
maintain.

Code

jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java[R59-69]

+    private static boolean isColumnHeaderTarget(Object eventTarget) {
+        if (!(eventTarget instanceof Node node)) {
+            return false;
+        }
+        while (node != null) {
+            if (node instanceof TableColumnHeader) {
+                return true;
+            }
+            node = node.getParent();
+        }
+        return false;

Evidence

The PR introduces an explicit dependency on javafx.scene.control.skin.TableColumnHeader and uses
it in runtime type checks. This is the only use of TableColumnHeader in the repository, and the
build logic explicitly treats javafx.scene.control.skin as a package that needs opening (a sign
it’s considered special/internal), reinforcing that this is not a typical public-API dependency to
build UI logic on.

jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java[6-15]
jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java[59-70]
build-logic/src/main/kotlin/org.jabref.gradle.base.dependency-rules.gradle.kts[487-495]
jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[205-229]

Agent prompt

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

## Issue description
`MainTableHeaderContextMenu` uses `javafx.scene.control.skin.TableColumnHeader` to decide whether to show the menu. Skin classes are implementation details and can change across JavaFX versions, making this logic brittle.
## Issue Context
The original bug happens because the empty-library placeholder (`VBox`) does not consume `ContextMenuEvent`, so the event bubbles to `MainTable`'s handler.
## Fix Focus Areas
- Prefer fixing the bubbling at the source (placeholder) instead of introspecting the skin tree.
- Keep the header menu trigger based on stable/public signals where possible.
### Suggested implementation direction
1. In `MainTable`, when creating `placeholderBox`, add a context-menu handler/filter that consumes the event (e.g., `placeholderBox.setOnContextMenuRequested(Event::consume)` or an event filter for `ContextMenuEvent.CONTEXT_MENU_REQUESTED`). This prevents bubbling to the table-level handler when the library is empty.
2. Remove the `javafx.scene.control.skin.TableColumnHeader` dependency and revert the header detection to a non-skin approach (or keep current logic but using CSS style classes / lookup if absolutely needed).
## Fix Focus Areas (exact locations)
- jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[205-229]
- jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java[6-15]
- jabgui/src/main/java/org/jabref/gui/maintable/MainTableHeaderContextMenu.java[46-70]
- build-logic/src/main/kotlin/org.jabref.gradle.base.dependency-rules.gradle.kts[487-495]

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

---

ⓘ The new review experience is currently in Beta. Learn more

[faneeshh]

So after these changes, the context menu doesn't close after clicking on the table body. If you click anywhere else except the table body it does close. Do you think it is needed to make the Table body area clickable to close the context menu?

[calixtus]

why a loop?

[calixtus]

This change is nonsense.
instanceof was just fine

C'mon, use your brain, not an LLM.

[faneeshh]

My bad was in a rush so opted for a quick fix... I should have reviewed it. I'll revert to the instanceof and make the requested changes when I get back home.

[Siedlerchr]

JabRef is not about getting things done fast. There is no "race" or competition.

[faneeshh]

I totally understand that and I promise I would never blindly make AI generated changes. It's just that I was going on a hike for 2-3 days and wouldn't have had access to work on the issue until the weekend. I'm really sorry about it. I always usually take my time reviewing everything precisely, this was a one time mistake and won't happen again.

[calixtus]

That is totally ok, quick note about a few days not available, we put the pr on waiting an you have a good time hiking. Just don't make bs commits in a hurry. That's just a waste of your and our time.
As soon as you are back we can finish the pr. Have a good time.

[faneeshh]

Thank you so much for understanding... I wouldn't wanna waste your guys time on AI generated PRs either. I'm sorry once again. I'll be back in 2 days and get back on it.

[github-actions]

Your pull request conflicts with the target branch.

Please merge 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.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 0969675
▶️ Tests: 10203 executed
⚪️ Checks: 50/50 completed

---

Learn more about TestLens at testlens.app.

[faneeshh]

So after these changes, the context menu doesn't close after clicking on the table body. If you click anywhere else except the table body it does close. Do you think it is needed to make the Table body area clickable to close the context menu?

@calixtus I've updated the method to use instanceof and it checks the target and its immediate parent instead of looping now too. Should I also add this functionality that I discussed above?