Fix #15397 invisible delete area for internal CSL styles by Francis-Lee-210 · Pull Request #15399 · JabRef/jabref

Francis-Lee-210 · 2026-03-24T07:06:34Z

Related issues and pull requests

PR Description

This fixes the invisible delete area shown for internal CSL styles in the OpenOffice/LibreOffice style selection dialog.

The delete cell is now explicitly cleared for empty rows and internal styles, so internal CSL styles no longer show a visible or invisible delete area. External CSL styles still show the delete button and remain deletable.

Steps to test

Open JabRef and connect to an OpenOffice/LibreOffice document.
Open the OpenOffice/LibreOffice panel and click Select style.
In CSL Styles, verify that internal CSL styles no longer show a visible or invisible delete area.
Import an external .csl style.
Verify that the external style still shows the delete button and can still be deleted.

Comparison screenshots

Before:

After - Internal CSL styles:

After - External CSL styles:

Library screenshot:

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

github-actions · 2026-03-24T07:06:46Z

Hey @Francis-Lee-210! 👋

Thank you for contributing to JabRef!

We have automated checks in place, based on which you will soon get feedback if any of them are failing. We also use Qodo for review assistance. It will update your pull request description with a review help and offer suggestions to improve the pull request.

After all automated checks pass, a maintainer will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. You can read about the whole pull request process in our contribution guide.

Please ensure that your pull request is in line with our AI Usage Policy and make necessary disclosures.

qodo-free-for-open-source-projects · 2026-03-24T07:06:57Z

Review Summary by Qodo

Fix invisible delete area for internal CSL styles

🐞 Bug fix

Walkthroughs

Description

• Fix invisible delete button area for internal CSL styles
• Replace ValueTableCellFactory with custom TableCell implementation
• Explicitly clear cell content for internal and empty rows
• Preserve delete functionality for external CSL styles

Diagram

flowchart LR
  A["CSL Styles Table"] -->|"Custom TableCell"| B["Check if Internal Style"]
  B -->|"Internal or Empty"| C["Clear Cell Content"]
  B -->|"External Style"| D["Show Delete Button"]
  D --> E["User Can Delete"]
  C --> F["No Delete Area"]

File Changes

1. jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java 🐞 Bug fix +21/-10

Replace factory with custom TableCell for delete button

• Added imports for TableCell and Tooltip classes
• Replaced ValueTableCellFactory with custom TableCell implementation
• Implemented updateItem() method to conditionally render delete button
• Delete button only appears for external CSL styles, hidden for internal styles
• Explicitly clears cell graphics, text, tooltip and mouse handlers for empty/internal rows

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java

2. CHANGELOG.md 📝 Documentation +1/-0

Document CSL delete button fix

• Added entry documenting the fix for invisible delete button area
• References issue #15397 and clarifies external styles remain deletable

CHANGELOG.md

qodo-free-for-open-source-projects · 2026-03-24T07:06:58Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Right-click triggers delete 🐞 Bug ✓ Correctness

Description

The CSL delete cell binds deletion to any mouse click (including secondary/middle clicks), which can
pop a delete confirmation unexpectedly and potentially delete a style on right-click. This is
particularly risky now that the handler deletes the row’s style directly rather than the
currently-selected style.

Code

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[R160-164]

+                CSLStyleSelectViewModel style = getTableRow().getItem();
+                setGraphic(IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode());
+                setTooltip(new Tooltip(Localization.lang("Remove style")));
+                setOnMouseClicked(_ -> viewModel.deleteCslStyle(style.getLayout().citationStyle()));
+            }

Evidence
The new cell factory assigns setOnMouseClicked without checking event.getButton(), so any mouse
click on the cell will call deleteCslStyle(...) and show a confirmation dialog. The new handler
targets getTableRow().getItem() (row-under-pointer), whereas the previous code used the selection
model; this makes accidental non-primary clicks act on the clicked row instead of a selected one.
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[141-165]
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogViewModel.java[237-243]
jabgui/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java[124-133]

Agent prompt

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

### Issue description
The delete icon cell triggers deletion for any mouse button, which can open delete confirmation on right-click/middle-click.

### Issue context
The delete cell is implemented as a custom `TableCell` and uses `setOnMouseClicked` with an ignored event parameter.

### Fix focus areas
- Ensure delete triggers only on primary mouse button.
- (Optional) Trigger only on single-click to avoid odd multi-click behavior.

- jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[160-164]

### Suggested change (sketch)
Change the handler to accept the event and guard it:
```java
setOnMouseClicked(event -> {
   if (event.getButton() != MouseButton.PRIMARY) {
       return;
   }
   // optionally: if (event.getClickCount() != 1) return;
   viewModel.deleteCslStyle(style.getLayout().citationStyle());
   event.consume();
});
```

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

2. Delete click bubbles to row 🐞 Bug ✓ Correctness

Description

The delete cell’s click handler does not consume the mouse event, so a double-click on the delete
column can also trigger the row’s double-click handler (which closes the dialog). This can cause
unexpected dialog closing while the user is interacting with the delete control.

Code

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[R163-168]

+                setOnMouseClicked(_ -> viewModel.deleteCslStyle(style.getLayout().citationStyle()));
+            }
+        });

        new ViewModelTableRowFactory<CSLStyleSelectViewModel>()
                .withOnMouseClickedEvent((item, event) -> {

Evidence
The delete cell installs an onMouseClicked handler but never consumes the event; the row factory
separately listens for clicks and closes the dialog on clickCount == 2. Without consuming in the
cell, the click can propagate/bubble to the row handler.
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[163-176]
jabgui/src/main/java/org/jabref/gui/util/ViewModelTableRowFactory.java[137-143]

Agent prompt

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

### Issue description
Click events on the delete cell can propagate to the table row handler. A double-click on the delete area can therefore trigger the row’s double-click behavior (select + close dialog).

### Issue context
The CSL styles table uses `ViewModelTableRowFactory.withOnMouseClickedEvent` to close the dialog on double-click, and the delete cell currently does not consume events.

### Fix focus areas
- Consume the mouse event in the delete cell handler to prevent bubbling to the row handler.
- Optionally ignore double-clicks for delete.

- jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[163-175]
- jabgui/src/main/java/org/jabref/gui/util/ViewModelTableRowFactory.java[137-143]

### Suggested change (sketch)
```java
setOnMouseClicked(event -> {
   if (event.getButton() != MouseButton.PRIMARY || event.getClickCount() != 1) {
       return;
   }
   viewModel.deleteCslStyle(style.getLayout().citationStyle());
   event.consume();
});
```

ⓘ 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

calixtus · 2026-03-24T11:17:21Z

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java

+        cslDeleteColumn.setCellFactory(_ -> new TableCell<>() {
+            @Override
+            protected void updateItem(Boolean internalStyle, boolean empty) {
+                super.updateItem(internalStyle, empty);
+
+                setText(null);
+                setGraphic(null);
+                setTooltip(null);
+                setOnMouseClicked(null);
+
+                if (empty || (internalStyle == null) || (getTableRow() == null) || (getTableRow().getItem() == null) || internalStyle) {
+                    return;
+                }

-        new ValueTableCellFactory<CSLStyleSelectViewModel, Boolean>()
-                .withGraphic(internalStyle -> internalStyle ? null : IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
-                .withOnMouseClickedEvent(_ -> _ -> {
-                    CSLStyleSelectViewModel selectedStyle = cslStylesTable.getSelectionModel().getSelectedItem();
-                    if (selectedStyle != null) {
-                        viewModel.deleteCslStyle(selectedStyle.getLayout().citationStyle());
-                    }
-                })
-                .withTooltip(_ -> Localization.lang("Remove style"))
-                .install(cslDeleteColumn);
+                CSLStyleSelectViewModel style = getTableRow().getItem();
+                setGraphic(IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode());
+                setTooltip(new Tooltip(Localization.lang("Remove style")));
+                setOnMouseClicked(_ -> viewModel.deleteCslStyle(style.getLayout().citationStyle()));
+            }
+        });


Please use our ValueTableCellFactory.

I updated the implementation to use ValueTableCellFactory, re-tested it locally, and pushed the change.

I also noticed that the Validate requirement coverage check keeps getting cancelled/timing out. Do I need to do anything on my side for that, or is it a CI/workflow issue?

github-actions · 2026-03-27T04:01:54Z

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 · 2026-03-27T20:21:22Z

✅ All tests passed ✅

🏷️ Commit: e5b1a59
▶️ Tests: 10203 executed
⚪️ Checks: 51/51 completed

Learn more about TestLens at testlens.app.

qodo-free-for-open-source-projects · 2026-03-27T20:21:43Z

Review Summary by Qodo

Fix invisible delete area for internal CSL styles

🐞 Bug fix

Walkthroughs

Description

• Fix invisible delete button area for internal CSL styles
• Prevent delete action execution on internal styles
• Maintain delete functionality for external CSL styles
• Update ValueTableCellFactory usage with correct parameter handling

Diagram

flowchart LR
  A["CSL Style Delete Cell"] --> B{"Is Internal Style?"}
  B -->|Yes| C["Hide Delete Button"]
  B -->|Yes| D["Disable Delete Action"]
  B -->|No| E["Show Delete Button"]
  B -->|No| F["Enable Delete Action"]
  C --> G["No Delete Area Visible"]
  D --> G
  E --> H["Delete Button Visible"]
  F --> H

File Changes

1. jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java 🐞 Bug fix +8/-6

Fix CSL delete button visibility and action handling

• Updated ValueTableCellFactory lambda expressions to accept both style and internal flag
 parameters
• Modified graphic factory to conditionally hide delete icon for internal styles
• Enhanced mouse click handler to return empty action for internal styles, preventing deletion
• Updated tooltip factory to hide tooltip text for internal styles

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java

2. CHANGELOG.md 📝 Documentation +1/-0

Document internal CSL delete button fix

• Added entry documenting the fix for invisible delete button area on internal CSL styles
• Clarified that external CSL styles remain deletable
• Referenced issue #15397

CHANGELOG.md

qodo-free-for-open-source-projects · 2026-03-27T20:21:43Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Stale tooltip not cleared 🐞 Bug ✓ Correctness ⭐ New

Description

Internal CSL style rows can still display the "Remove style" tooltip after scrolling because the
cell factory only sets a tooltip when the tooltip text is non-blank and never clears a previously
set tooltip when the new tooltip text is null/blank. This undermines the PR’s goal of removing the
(visible/invisible) delete affordance for internal styles.

Code

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[155]
+                .withTooltip((_, internalStyle) -> internalStyle ? null : Localization.lang("Remove style"))

Evidence
The PR makes internal styles return a null tooltip; however, ValueTableCellFactory.updateItem() only
calls setTooltip(...) when the tooltip text is not blank and does not call setTooltip(null)
otherwise. Since JavaFX virtualizes/reuses TableCell instances, a cell previously used for an
external style (tooltip set) can be reused for an internal style and keep the old tooltip.
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[145-156]
jabgui/src/main/java/org/jabref/gui/util/ValueTableCellFactory.java[108-138]

Agent prompt

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

### Issue description
`StyleSelectDialogView` now returns `null` tooltip text for internal CSL styles, but `ValueTableCellFactory` does not clear an existing tooltip when the tooltip text is null/blank. Because JavaFX reuses cells, internal rows can incorrectly retain and show the previous row’s tooltip (e.g., “Remove style”).

### Issue Context
This PR intentionally removes delete affordances for internal CSL styles by setting tooltip to `null`.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/util/ValueTableCellFactory.java[108-138]
- jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[145-156]

### Suggested fix
In `ValueTableCellFactory.updateItem`, when `toTooltip != null`, explicitly clear the tooltip when the computed tooltip text is blank/null, e.g.:
- compute `tooltipText`
- if blank: `setTooltip(null)`
- else: set the newly created Tooltip

This makes the new `internalStyle ? null : ...` behavior in `StyleSelectDialogView` work reliably under cell reuse.

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

2. Right-click triggers delete 🐞 Bug ✓ Correctness

Description

The CSL delete cell binds deletion to any mouse click (including secondary/middle clicks), which can
pop a delete confirmation unexpectedly and potentially delete a style on right-click. This is
particularly risky now that the handler deletes the row’s style directly rather than the
currently-selected style.

Code

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[R160-164]

+                CSLStyleSelectViewModel style = getTableRow().getItem();
+                setGraphic(IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode());
+                setTooltip(new Tooltip(Localization.lang("Remove style")));
+                setOnMouseClicked(_ -> viewModel.deleteCslStyle(style.getLayout().citationStyle()));
+            }

Evidence
The new cell factory assigns setOnMouseClicked without checking event.getButton(), so any mouse
click on the cell will call deleteCslStyle(...) and show a confirmation dialog. The new handler
targets getTableRow().getItem() (row-under-pointer), whereas the previous code used the selection
model; this makes accidental non-primary clicks act on the clicked row instead of a selected one.
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[141-165]
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogViewModel.java[237-243]
jabgui/src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java[124-133]

Agent prompt

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

## Issue description
The delete icon cell triggers deletion for any mouse button, which can open delete confirmation on right-click/middle-click.
### Issue context
The delete cell is implemented as a custom `TableCell` and uses `setOnMouseClicked` with an ignored event parameter.
### Fix focus areas
- Ensure delete triggers only on primary mouse button.
- (Optional) Trigger only on single-click to avoid odd multi-click behavior.
- jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[160-164]
### Suggested change (sketch)
Change the handler to accept the event and guard it:

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

3. Delete click bubbles to row 🐞 Bug ✓ Correctness

Description

The delete cell’s click handler does not consume the mouse event, so a double-click on the delete
column can also trigger the row’s double-click handler (which closes the dialog). This can cause
unexpected dialog closing while the user is interacting with the delete control.

Code

jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[R163-168]

+                setOnMouseClicked(_ -> viewModel.deleteCslStyle(style.getLayout().citationStyle()));
+            }
+        });

       new ViewModelTableRowFactory<CSLStyleSelectViewModel>()
               .withOnMouseClickedEvent((item, event) -> {

Evidence
The delete cell installs an onMouseClicked handler but never consumes the event; the row factory
separately listens for clicks and closes the dialog on clickCount == 2. Without consuming in the
cell, the click can propagate/bubble to the row handler.
jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[163-176]
jabgui/src/main/java/org/jabref/gui/util/ViewModelTableRowFactory.java[137-143]

Agent prompt

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

## Issue description
Click events on the delete cell can propagate to the table row handler. A double-click on the delete area can therefore trigger the row’s double-click behavior (select + close dialog).
### Issue context
The CSL styles table uses `ViewModelTableRowFactory.withOnMouseClickedEvent` to close the dialog on double-click, and the delete cell currently does not consume events.
### Fix focus areas
- Consume the mouse event in the delete cell handler to prevent bubbling to the row handler.
- Optionally ignore double-clicks for delete.
- jabgui/src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java[163-175]
- jabgui/src/main/java/org/jabref/gui/util/ViewModelTableRowFactory.java[137-143]
### Suggested change (sketch)

ⓘ 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

subhramit · 2026-03-27T20:21:47Z

@Francis-Lee-210 please update the screenshots after your most recent change

pluto-han · 2026-03-28T13:30:42Z

Just tried out this PR, found one problem:

Add .csl file
"Remove style" does not remove the file from the UI immediately, unless users reopen the dialog

PR.test.mp4

pluto-han · 2026-03-28T13:48:14Z

Just tried out this PR, found one problem:

But the problem seems not related to this change.

subhramit · 2026-03-28T16:20:44Z

Just tried out this PR, found one problem:

Add .csl file

"Remove style" does not remove the file from the UI immediately, unless users reopen the dialog

Is the problem there on the main branch as well? Or just this PR?

pluto-han · 2026-03-28T16:26:16Z

Is the problem there on the main branch as well? Or just this PR?

It exists in the main branch as well. An error shows up when removing the "removed" csl file again.

subhramit · 2026-03-28T16:30:11Z

Is the problem there on the main branch as well? Or just this PR?

It exists in the main branch as well. An error shows up when removing the "removed" csl file again.

Good finding - create an issue

Francis-Lee-210 added 2 commits March 24, 2026 18:59

Fix invisible delete button for internal CSL styles

2898a4a

Add changelog entry for internal CSL delete fix

5f23cc4

github-actions bot added the first contrib label Mar 24, 2026

github-actions bot added good first issue An issue intended for project-newcomers. Varies in difficulty. component: libre-office status: changes-required Pull requests that are not yet complete labels Mar 24, 2026

This comment has been minimized.

Sign in to view

github-actions bot mentioned this pull request Mar 24, 2026

Remove invisible delete button for internal CSL styles #15397

Open

2 tasks

Fix import order in StyleSelectDialogView

55e1d1f

This comment has been minimized.

Sign in to view

github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 24, 2026

calixtus requested changes Mar 24, 2026

View reviewed changes

github-actions bot added status: changes-required Pull requests that are not yet complete and removed status: no-bot-comments labels Mar 24, 2026

calixtus marked this pull request as draft March 25, 2026 07:27

Use ValueTableCellFactory for CSL delete cell

affabb0

This comment has been minimized.

Sign in to view

github-actions bot added status: no-bot-comments status: changes-required Pull requests that are not yet complete and removed status: changes-required Pull requests that are not yet complete status: no-bot-comments labels Mar 25, 2026

Merge branch 'main' into fix-15397-internal-csl-delete

693b03a

This comment has been minimized.

Sign in to view

github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 27, 2026

Merge branch 'main' into fix-15397-internal-csl-delete

e5b1a59

subhramit marked this pull request as ready for review March 27, 2026 20:21

Uh oh!

Conversation

Francis-Lee-210 commented Mar 24, 2026

Related issues and pull requests

PR Description

Steps to test

Comparison screenshots

Library screenshot:

Checklist

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 24, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

This comment has been minimized.

This comment has been minimized.

calixtus Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Francis-Lee-210 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

github-actions bot commented Mar 27, 2026

Uh oh!

This comment has been minimized.

testlens-app bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All tests passed ✅

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 27, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

subhramit commented Mar 27, 2026

Uh oh!

pluto-han commented Mar 28, 2026

Uh oh!

pluto-han commented Mar 28, 2026

Uh oh!

subhramit commented Mar 28, 2026

Uh oh!

pluto-han commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

subhramit commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qodo-free-for-open-source-projects bot commented Mar 24, 2026 •

edited

Loading

testlens-app bot commented Mar 27, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Mar 27, 2026 •

edited

Loading

pluto-han commented Mar 28, 2026 •

edited

Loading