Skip to content

Route dropped bibliography files to import dialog#15754

Open
JiahaoHe123 wants to merge 9 commits into
JabRef:mainfrom
JiahaoHe123:issue-dnd-library-import-clean
Open

Route dropped bibliography files to import dialog#15754
JiahaoHe123 wants to merge 9 commits into
JabRef:mainfrom
JiahaoHe123:issue-dnd-library-import-clean

Conversation

@JiahaoHe123
Copy link
Copy Markdown

Reviewed and exploratory tested with @kramosss

Related issues and pull requests

Closes #15391


PR Description

Fixes #15391

This PR updates drag-and-drop handling so that dropped importable bibliography files, such as .ris and .bib, open JabRef’s standard import dialog instead of being imported or linked immediately. This makes drag-and-drop consistent with the normal import workflow while keeping the existing behavior for PDFs and other non-importable files unchanged. This change does not affect invalidation behavior elsewhere, and no additional invalidation logic was introduced.


Steps to test

  1. Open JabRef
  2. Create a new example library, or open an existing library
  3. Drag and drop a .ris file containing multiple entries into the main table.
  4. Verify that the import dialog opens and shows the parsed entries instead of importing them directly.
  5. Drag and drop a .bib file and verify that it also opens the import dialog.
  6. Drag and drop a PDF file and verify that the existing PDF import behavior is unchanged.
  7. Drag and drop a non-importable file and verify that the existing non-bibliography file-drop behavior is unchanged.
  8. Drag and drop an invalid .ris file and verify that JabRef does not import it as a bibliography file.
  9. Drag and drop multiple bibliography files together, such as a .ris file and a .bib file, and verify that all parsed entries appear in the import dialog.
  10. Cancel the import dialog and verify that no entries are imported.

Screenshots
Screenshot 2026-05-18 at 1 41 48 AM
Screenshot 2026-05-18 at 1 44 03 AM
Screenshot 2026-05-18 at 1 41 48 AM
Screenshot 2026-05-18 at 1 49 31 AM
Screenshot 2026-05-18 at 2 03 13 AM

Example .ris file for testing:

TY  - JOUR
TI  - Test Article One
AU  - Smith, John
PY  - 2024
JO  - Test Journal
ER  -

TY  - JOUR
TI  - Test Article Two
AU  - Doe, Jane
PY  - 2025
JO  - Another Test Journal
ER  -

Example .bib file for testing:

@article{TestArticleOne,
  title   = {Test Article One},
  author  = {Smith, John},
  year    = {2024},
  journal = {Test Journal},
}

@article{TestArticleTwo,
  title   = {Test Article Two},
  author  = {Doe, Jane},
  year    = {2025},
  journal = {Another Test Journal},
}

Example .enw file for testing:

%0 Journal Article
%T EndNote Test Article One
%A Lee, Chris
%D 2024
%J EndNote Test Journal

%0 Journal Article
%T EndNote Test Article Two
%A Wang, Emily
%D 2025
%J Another EndNote Journal

Note
You can also use demo file Chocolate.bib from jabref-demonstration-libraries to test the drag-and-drop behavior.


AI usage

GitHub Copilot (GPT-5.4)
ChatGPT (GPT-5.5 Thinking)


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 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
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Route dropped bibliography files to import dialog

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Routes dropped bibliography files (RIS, BibTeX, etc.) to import dialog
• Separates importable files from PDFs and other non-bibliography files
• Maintains existing behavior for PDFs and non-importable files
• Adds comprehensive tests for dropped file classification logic
Diagram
flowchart LR
  A["Dropped Files"] --> B["planDroppedFiles"]
  B --> C{"isImportableBibliographyFile?"}
  C -->|Yes| D["Bibliography Files"]
  C -->|No| E["Remaining Files"]
  D --> F["importBibliographyFilesWithDialog"]
  E --> G["handleDropOfFiles"]
  F --> H["ImportEntriesDialog"]
  G --> I["Link to Entry or Import"]
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java ✨ Enhancement +21/-4

Integrate ImportHandler for entry editor drag-drop

• Imports ImportHandler class for handling dropped files
• Refactors drag-and-drop handler to use planDroppedFiles() method
• Routes bibliography files to import dialog via importBibliographyFilesWithDialog()
• Passes remaining non-bibliography files to existing DragDrop.handleDropOfFiles() logic

jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java


2. jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java ✨ Enhancement +88/-0

Add bibliography file detection and import dialog flow

• Adds ImportEntriesDialog and ImportResultsMerger imports for dialog-based imports
• Implements shouldShowImportDialog() method to check for importable bibliography files
• Adds planDroppedFiles() method that separates bibliography files from other files
• Implements importBibliographyFilesWithDialog() to show import dialog for bibliography files
• Adds isImportableBibliographyFile() method to detect importable bibliography formats
• Adds parseBibliographyFiles() method to parse multiple bibliography files with error handling
• Adds createImportFormatReader() helper method for consistent reader creation
• Introduces DroppedFileImportPlan record to encapsulate file classification results

jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java


3. jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java ✨ Enhancement +26/-8

Update main table drag-drop to use file classification

• Uses planDroppedFiles() to classify dropped files in table row handler
• Routes bibliography files to import dialog for TOP/BOTTOM drop locations
• Routes bibliography files to import dialog for CENTER drop location
• Passes remaining files to existing import/link handlers
• Applies consistent file classification logic across all drop scenarios

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


View more (2)
4. jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java 🧪 Tests +63/-0

Add tests for dropped file classification logic

• Adds imports for IOException, Files, Path, and test utilities
• Adds @TempDir annotation for temporary directory in tests
• Adds mock setup for ImporterPreferences and CitationKeyPatternPreferences
• Adds test shouldShowImportDialogForRisFile() to verify RIS file detection
• Adds test shouldNotShowImportDialogForPdfFile() to verify PDF exclusion
• Adds test shouldNotShowImportDialogForPlainTextFile() to verify text file exclusion
• Adds test planDroppedFilesSeparatesBibliographyFilesFromOtherFiles() to verify file
 classification

jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java


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

Document drag-and-drop import dialog fix

• Documents fix for drag-and-drop behavior of bibliography files
• Notes that RIS and BibTeX files now open import dialog instead of being linked
• References issue #15391

CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented May 18, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Misindented @test method ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
A newly added test method is indented inconsistently (@Test and the method body are
over-indented), creating avoidable style noise and risking formatting/spotless enforcement failures.
This violates the requirement to follow existing formatting in changed sections.
Code

jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java[R163-180]

+        @Test
+        void planDroppedFilesSeparatesBibliographyFilesFromOtherFiles() throws IOException {
+                Path risFile = tempDir.resolve("test.ris");
+                Files.writeString(risFile, """
+                                TY  - JOUR
+                                TI  - Drag and drop test
+                                AU  - Doe, Jane
+                                PY  - 2024
+                                ER  -
+                                """);
+                Path textFile = tempDir.resolve("notes.txt");
+                Files.writeString(textFile, "plain text");
+
+                ImportHandler.DroppedFileImportPlan droppedFileImportPlan = importHandler.planDroppedFiles(List.of(risFile, textFile));
+
+                assertEquals(List.of(risFile), droppedFileImportPlan.bibliographyFiles());
+                assertEquals(List.of(textFile), droppedFileImportPlan.remainingFiles());
+        }
Evidence
PR Compliance ID 6 requires preserving established formatting and avoiding unnecessary formatting
issues in modified sections. The new test method is visibly misindented relative to adjacent @Test
methods.

AGENTS.md
jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java[163-180]

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 added test `planDroppedFilesSeparatesBibliographyFilesFromOtherFiles` is incorrectly indented compared to surrounding tests, which can cause formatting-check failures and reduces readability.
## Issue Context
Other test methods in this file use consistent 4-space indentation at class scope.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java[163-180]

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


2. UI-thread file parsing 🐞 Bug ➹ Performance
Description
EntryEditor/MainTable drag-and-drop handlers call ImportHandler.planDroppedFiles on the JavaFX event
thread, but planDroppedFiles classifies files by running ImportFormatReader.importWithAutoDetection
(full file I/O + parsing) per file. This can freeze drag-and-drop responsiveness and also wastes
work by parsing twice (classification + dialog parsing).
Code

jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[R542-546]

                               .map(File::toPath)
                               .map(FileUtil::resolveIfShortcut)
                               .collect(Collectors.toList());
+            ImportHandler.DroppedFileImportPlan droppedFileImportPlan = importHandler.planDroppedFiles(files);
Evidence
The drag-and-drop handlers synchronously call planDroppedFiles, which classifies each dropped file
by fully parsing it with importWithAutoDetection (file I/O + importer iteration). That work
happens before any background task is scheduled, and then the dialog path parses the files again,
doubling the cost.

jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[537-600]
jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[214-248]
jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[123-136]
jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[263-307]
jablib/src/main/java/org/jabref/logic/importer/ImportFormatReader.java[65-108]
jablib/src/main/java/org/jabref/logic/importer/ImportFormatReader.java[150-164]
jablib/src/main/java/org/jabref/logic/importer/ImportFormatReader.java[224-255]

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

## Issue description
Drag-and-drop currently blocks the JavaFX event thread by calling `ImportHandler.planDroppedFiles(...)`, which synchronously parses each dropped file via `ImportFormatReader.importWithAutoDetection(...)` to decide whether it is a bibliography file. This can cause UI freezes for large files or multi-file drops, and it also causes double work because the same files are parsed again when the import dialog runs.
## Issue Context
- `planDroppedFiles` is invoked directly inside JavaFX drag-drop handlers (EntryEditor + MainTable).
- `planDroppedFiles` calls `isImportableBibliographyFile` for each file.
- `isImportableBibliographyFile` creates a new `ImportFormatReader` and runs `importWithAutoDetection`, which iterates through many importers and reads file contents.
- The import dialog later triggers parsing again (`parseBibliographyFiles`).
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[537-600]
- jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[214-248]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[119-143]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[123-136]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[263-315]
## Suggested implementation direction
- Make dropped-file classification non-blocking:
- Option A (preferred): classify by extension using available importers’ `FileType`/extensions (no file I/O) and let the import dialog/background task handle actual parsing + errors.
- Option B: run `planDroppedFiles` in a `BackgroundTask` executed via `taskExecutor`, then open the dialog on the FX thread once classification completes.
- Avoid repeated work:
- Reuse a single `ImportFormatReader` instance per drop operation (instead of constructing one per file).
- If you keep content-based detection, consider caching `ImportResult`/`ParserResult` from classification and passing it into the dialog to avoid re-parsing.

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



Remediation recommended

3. Use .toList() in EntryEditor 📘 Rule violation ⚙ Maintainability
Description
The new stream pipeline uses collect(Collectors.toList()) instead of the modern Stream.toList()
idiom expected for Java 25+ code. This introduces legacy style in newly modified code and may reduce
consistency with the rest of the codebase.
Code

jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[R228-230]

+                List<Path> files = event.getDragboard().getFiles().stream()
+                                        .map(File::toPath)
+                                        .collect(Collectors.toList());
Evidence
PR Compliance ID 4 requires using modern Java 25+ idioms where applicable. The added code in
EntryEditor collects a stream using Collectors.toList() instead of toList().

AGENTS.md
jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[228-230]

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

## Issue description
New code uses `collect(Collectors.toList())` where modern Java (25+) `Stream.toList()` should be preferred for consistency and readability.
## Issue Context
The project compliance rules prefer modern Java idioms in new/changed code.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[228-230]

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


Grey Divider

Qodo Logo

@jabref-machine
Copy link
Copy Markdown
Collaborator

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of - [x] (done), - [ ] (yet to be done) or - [/] (not applicable). Please adhere to our pull request template.

Comment thread jabgui/src/test/java/org/jabref/gui/externalfiles/ImportHandlerTest.java Outdated
Comment on lines 542 to 546
.map(File::toPath)
.map(FileUtil::resolveIfShortcut)
.collect(Collectors.toList());
ImportHandler.DroppedFileImportPlan droppedFileImportPlan = importHandler.planDroppedFiles(files);

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. Ui-thread file parsing 🐞 Bug ➹ Performance

EntryEditor/MainTable drag-and-drop handlers call ImportHandler.planDroppedFiles on the JavaFX event
thread, but planDroppedFiles classifies files by running ImportFormatReader.importWithAutoDetection
(full file I/O + parsing) per file. This can freeze drag-and-drop responsiveness and also wastes
work by parsing twice (classification + dialog parsing).
Agent Prompt
## Issue description
Drag-and-drop currently blocks the JavaFX event thread by calling `ImportHandler.planDroppedFiles(...)`, which synchronously parses each dropped file via `ImportFormatReader.importWithAutoDetection(...)` to decide whether it is a bibliography file. This can cause UI freezes for large files or multi-file drops, and it also causes double work because the same files are parsed again when the import dialog runs.

## Issue Context
- `planDroppedFiles` is invoked directly inside JavaFX drag-drop handlers (EntryEditor + MainTable).
- `planDroppedFiles` calls `isImportableBibliographyFile` for each file.
- `isImportableBibliographyFile` creates a new `ImportFormatReader` and runs `importWithAutoDetection`, which iterates through many importers and reads file contents.
- The import dialog later triggers parsing again (`parseBibliographyFiles`).

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[537-600]
- jabgui/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java[214-248]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[119-143]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[123-136]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[263-315]

## Suggested implementation direction
- Make dropped-file classification non-blocking:
  - Option A (preferred): classify by extension using available importers’ `FileType`/extensions (no file I/O) and let the import dialog/background task handle actual parsing + errors.
  - Option B: run `planDroppedFiles` in a `BackgroundTask` executed via `taskExecutor`, then open the dialog on the FX thread once classification completes.
- Avoid repeated work:
  - Reuse a single `ImportFormatReader` instance per drop operation (instead of constructing one per file).
  - If you keep content-based detection, consider caching `ImportResult`/`ParserResult` from classification and passing it into the dialog to avoid re-parsing.

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

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label May 18, 2026
@Siedlerchr
Copy link
Copy Markdown
Member

You do no tneed to close and recreate a new pr, just push to your existing branch

@JiahaoHe123
Copy link
Copy Markdown
Author

You do no tneed to close and recreate a new pr, just push to your existing branch

Thank you for the suggestion. I realized the previous PR included some unintended changes and was not as clean as it should be. In addition, I cannot fully understand those changes. Thus, to avoid confusion, I opened a new PR with a cleaner branch that only contains the focused fix for this issue.

@github-actions
Copy link
Copy Markdown
Contributor

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.

@koppor
Copy link
Copy Markdown
Member

koppor commented May 20, 2026

Thus, to avoid confusion, I opened a new PR with a cleaner branch that only contains the focused fix for this issue.

force push is allowed in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compliance violation status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When a user drag-n-drops a "library" file, then parse it, and add all its entries to the library

4 participants