Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv

### Added

- We removed a little snap that used to happen with the side panel when one opened JabRef after changing the side panel's default position [#15394](https://github.com/JabRef/jabref/issues/15394)
Comment thread
pgrigolli marked this conversation as resolved.
Outdated
- We added a related work text extractor, which finds and inserts the related work text into bib entries from references in the texts. [#9840](https://github.com/JabRef/jabref/issues/9840)
- We added a hover button on group rows to quickly add a new group or subgroup. [#12289](https://github.com/JabRef/jabref/issues/12289)
- We added a shorthand for protecting terms in the fields: user can now select a text and type a opening curling brace to quickly wrap the selection in braces. [#15442](https://github.com/JabRef/jabref/pull/15442)
Expand Down
12 changes: 11 additions & 1 deletion jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,12 @@ private void updateSidePane() {
horizontalSplit.getItems().remove(sidePane);
} else {
if (!horizontalSplit.getItems().contains(sidePane)) {
horizontalSplit.setVisible(false);
horizontalSplit.getItems().addFirst(sidePane);
updateHorizontalDividerPosition();
Platform.runLater(() -> {
updateHorizontalDividerPosition();
horizontalSplit.setVisible(true);
});
}
}
}
Expand Down Expand Up @@ -407,6 +411,12 @@ private void initKeyBindings() {
private void initBindings() {
BindingsHelper.bindContentFiltered(tabbedPane.getTabs(), stateManager.getOpenDatabases(), LibraryTab.class::isInstance);

mainStage.showingProperty().addListener((obs, wasShowing, isShowing) -> {
if (isShowing) {
updateHorizontalDividerPosition();
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Leaked divider subscriptions 🐞 Bug ☼ Reliability

initBindings() adds a mainStage.showingProperty listener that calls
updateHorizontalDividerPosition(), but JabRefGUI.onShowing() already calls it; because
updateHorizontalDividerPosition() overwrites horizontalDividerSubscription without unsubscribing
the previous subscription, repeated window show cycles will leak listeners and duplicate preference
updates.
Agent Prompt
### Issue description
`updateHorizontalDividerPosition()` installs an EasyBind listener each time it runs, but it does not unsubscribe any prior listener. This PR adds an additional call site (`mainStage.showingProperty` listener), while `JabRefGUI.onShowing()` already calls the method, so subscriptions can stack up across show/hide cycles.

### Issue Context
- `JabRefGUI.onShowing()` already calls `mainFrame.updateHorizontalDividerPosition()`.
- `updateHorizontalDividerPosition()` assigns `horizontalDividerSubscription = ...listenToValues(...)` without cleanup.

### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java[311-320]
- jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java[411-418]
- jabgui/src/main/java/org/jabref/gui/JabRefGUI.java[357-361]

### Suggested fix approach
- Prefer a single invocation path (remove the newly-added `showingProperty` listener, since `JabRefGUI.onShowing()` already handles it), **or**
- Make `updateHorizontalDividerPosition()` idempotent by unsubscribing an existing `horizontalDividerSubscription` before creating a new one (and consider doing the same for vertical for consistency).

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgrigolli needs to be addressed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, turns out it was just dead code that I forgot to remove from one try to another. Removed on the last commit!


// the binding for stateManager.activeDatabaseProperty() is at org.jabref.gui.LibraryTab.onDatabaseLoadingSucceed

// Subscribe to the search
Expand Down
Loading