Allow drag and drop of Windows shortcut (.lnk) files #15053
Open
faneeshh wants to merge 8 commits intoJabRef:mainfrom
Open
Allow drag and drop of Windows shortcut (.lnk) files #15053faneeshh wants to merge 8 commits intoJabRef:mainfrom
faneeshh wants to merge 8 commits intoJabRef:mainfrom
Conversation
Contributor
Review Summary by QodoSupport drag and drop of Windows shortcut (.lnk) files
WalkthroughsDescription• Add support for dragging and dropping Windows shortcut (.lnk) files • Resolve .lnk shortcuts to target .bib files using PowerShell • Update FrameDndHandler and MainTable to handle shortcut resolution • Add utility methods in FileUtil for shortcut detection and resolution Diagramflowchart LR
A["User drags .lnk file"] --> B["FrameDndHandler/MainTable receives file"]
B --> C["FileUtil.isShortcutFile checks extension"]
C --> D["FileUtil.resolveWindowsShortcut uses PowerShell"]
D --> E["Target .bib file path returned"]
E --> F["Library opens with entries"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java
|
github-actions
bot
added
the
good first issue
Contributor
Code Review by Qodo
1. .lnk accepted on non-Windows platforms
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
548
to
584
| try { | ||
| // Use PowerShell to resolve the shortcut target | ||
| ProcessBuilder pb = new ProcessBuilder( | ||
| "powershell", "-NoProfile", "-Command", | ||
| "$ws = New-Object -ComObject WScript.Shell; " + | ||
| "$shortcut = $ws.CreateShortcut('" + shortcutPath.toAbsolutePath().toString().replace("'", "''") + "'); " + | ||
| "Write-Output $shortcut.TargetPath" | ||
| ); | ||
|
|
||
| pb.redirectErrorStream(true); | ||
| Process process = pb.start(); | ||
|
|
||
| String targetPath; | ||
| try (java.io.BufferedReader reader = new java.io.BufferedReader( | ||
| new java.io.InputStreamReader(process.getInputStream()))) { | ||
| targetPath = reader.readLine(); | ||
| } | ||
|
|
||
| int exitCode = process.waitFor(); | ||
|
|
||
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { | ||
| LOGGER.warn("Shortcut target does not exist: {}", resolvedPath); | ||
| return Optional.empty(); | ||
| } | ||
| } else { | ||
| LOGGER.warn("Failed to resolve shortcut: {} (exit code: {})", shortcutPath, exitCode); | ||
| return Optional.empty(); | ||
| } | ||
| } catch (Exception e) { | ||
| LOGGER.warn("Exception while resolving shortcut: {}", shortcutPath, e); | ||
| return Optional.empty(); | ||
| } |
Contributor
There was a problem hiding this comment.
1. resolvewindowsshortcut catches exception 📘 Rule violation ⛯ Reliability
• resolveWindowsShortcut wraps the whole PowerShell invocation in a broad catch (Exception) and degrades by returning Optional.empty(), which can silently drop .lnk files during drag-and-drop without a clear recovery path. • The method blocks on process.waitFor() with no timeout and does not restore the interrupt flag on interruption, which risks hangs and makes failures harder to diagnose reliably. • This violates the requirement for specific, non-disruptive exception handling and robust edge-case management around external process execution.
Agent Prompt
## Issue description
`FileUtil.resolveWindowsShortcut` catches a broad `Exception` and blocks on `process.waitFor()` without a timeout. This can lead to silent failures (returning `Optional.empty()`), and it risks hanging the application thread.
## Issue Context
The shortcut resolution executes an external process (PowerShell) which is a high-failure-risk dependency (missing executable, permission issues, COM errors, long-running/hung process). Exception handling should be specific and interruptions should preserve the thread interrupt status.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[548-584]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
524
to
596
| /// Test if the file is a shortcut file by simply checking the extension to be ".lnk" | ||
| /// | ||
| /// @param file The file to check | ||
| /// @return True if file extension is ".lnk", false otherwise | ||
| public static boolean isShortcutFile(Path file) { | ||
| return getFileExtension(file).filter("lnk"::equals).isPresent(); | ||
| } | ||
|
|
||
| /// Resolves a Windows shortcut (.lnk) file to its target path. | ||
| /// Only works on Windows systems. On other systems or if resolution fails, returns empty Optional. | ||
| /// | ||
| /// @param shortcutPath The path to the .lnk file | ||
| /// @return Optional containing the target path, or empty if resolution fails | ||
| public static Optional<Path> resolveWindowsShortcut(Path shortcutPath) { | ||
| if (!isShortcutFile(shortcutPath)) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| // Only attempt on Windows | ||
| if (!OS.WINDOWS) { | ||
| LOGGER.debug("Shortcut resolution only supported on Windows"); | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| try { | ||
| // Use PowerShell to resolve the shortcut target | ||
| ProcessBuilder pb = new ProcessBuilder( | ||
| "powershell", "-NoProfile", "-Command", | ||
| "$ws = New-Object -ComObject WScript.Shell; " + | ||
| "$shortcut = $ws.CreateShortcut('" + shortcutPath.toAbsolutePath().toString().replace("'", "''") + "'); " + | ||
| "Write-Output $shortcut.TargetPath" | ||
| ); | ||
|
|
||
| pb.redirectErrorStream(true); | ||
| Process process = pb.start(); | ||
|
|
||
| String targetPath; | ||
| try (java.io.BufferedReader reader = new java.io.BufferedReader( | ||
| new java.io.InputStreamReader(process.getInputStream()))) { | ||
| targetPath = reader.readLine(); | ||
| } | ||
|
|
||
| int exitCode = process.waitFor(); | ||
|
|
||
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { | ||
| LOGGER.warn("Shortcut target does not exist: {}", resolvedPath); | ||
| return Optional.empty(); | ||
| } | ||
| } else { | ||
| LOGGER.warn("Failed to resolve shortcut: {} (exit code: {})", shortcutPath, exitCode); | ||
| return Optional.empty(); | ||
| } | ||
| } catch (Exception e) { | ||
| LOGGER.warn("Exception while resolving shortcut: {}", shortcutPath, e); | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
|
|
||
| public static List<Path> resolveWindowsShortcuts(List<Path> paths) { | ||
| return paths.stream() | ||
| .flatMap(path -> { | ||
| if (isShortcutFile(path)) { | ||
| return resolveWindowsShortcut(path).stream(); | ||
| } | ||
| return Stream.of(path); | ||
| }) | ||
| .toList(); | ||
| } |
Contributor
There was a problem hiding this comment.
2. Missing tests for .lnk 📘 Rule violation ✓ Correctness
• New behavior was introduced for resolving Windows shortcuts via FileUtil.resolveWindowsShortcut/resolveWindowsShortcuts and used in drag-and-drop flows, but no corresponding JUnit tests were added/updated to cover this behavior. • The existing FileUtilTest covers file extension handling, yet does not include .lnk cases, leaving key edge cases (non-Windows behavior, failure modes) unverified.
Agent Prompt
## Issue description
New Windows shortcut (`.lnk`) drag-and-drop support introduces new `FileUtil` behavior without corresponding test coverage.
## Issue Context
At minimum, tests should cover:
- `isShortcutFile` correctly detecting `.lnk` (case handling if intended)
- `resolveWindowsShortcuts` returning the original paths for non-shortcut inputs
- non-Windows behavior: `resolveWindowsShortcut` should return `Optional.empty()` without throwing
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[524-596]
- jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[179-210]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
524
to
530
| /// Test if the file is a shortcut file by simply checking the extension to be ".lnk" | ||
| /// | ||
| /// @param file The file to check | ||
| /// @return True if file extension is ".lnk", false otherwise | ||
| public static boolean isShortcutFile(Path file) { | ||
| return getFileExtension(file).filter("lnk"::equals).isPresent(); | ||
| } |
Contributor
There was a problem hiding this comment.
3. Case-sensitive .lnk check 🐞 Bug ✓ Correctness
• isShortcutFile uses exact string match ("lnk"::equals) but getFileExtension does not
actually lowercase the extension, despite its documentation.
• As a result, shortcuts like FILE.LNK may not be recognized, breaking the new drag-and-drop
behavior on Windows.
Agent Prompt
### Issue description
`.lnk` detection is effectively case-sensitive because `getFileExtension` returns the raw extension and `isShortcutFile` compares using `"lnk"::equals`.
### Issue Context
Windows file extensions are case-insensitive and `.LNK` is common. The current implementation can fail to detect valid shortcuts.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[74-94]
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[524-530]
### Suggested approach
- Update `getFileExtension` to return `extension.toLowerCase(Locale.ROOT)` (and adjust imports/tests accordingly), aligning with its Javadoc.
- Alternatively, change `isShortcutFile` to `filter(ext -> ext.equalsIgnoreCase("lnk"))` (but prefer central normalization in `getFileExtension`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
568
to
573
| if (exitCode == 0 && targetPath != null && !targetPath.trim().isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath.trim()); | ||
| if (Files.exists(resolvedPath)) { | ||
| LOGGER.debug("Resolved shortcut {} to {}", shortcutPath, resolvedPath); | ||
| return Optional.of(resolvedPath); | ||
| } else { |
Contributor
There was a problem hiding this comment.
4. Shortcut returns non-file 🐞 Bug ✓ Correctness
• resolveWindowsShortcut returns any existing target path, including directories, because it only checks Files.exists. • Downstream import/link logic in ImportHandler treats unknown paths by creating an empty entry with a file link, which can create incorrect entries if a shortcut points to a directory.
Agent Prompt
### Issue description
Shortcut resolution currently returns any existing path, including directories. This can leak directory targets into file-import/link flows, creating empty entries with folder links.
### Issue Context
`ImportHandler` treats unknown paths as generic files and creates an empty entry with a link, which is incorrect when the path is a folder.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[568-576]
- jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java[183-199]
### Suggested approach
- Replace `Files.exists(resolvedPath)` with `Files.isRegularFile(resolvedPath)` (or `Files.isRegularFile(resolvedPath, LinkOption.NOFOLLOW_LINKS)` if appropriate).
- Consider logging a clear warning when the shortcut target is not a regular file.
- Optionally: for callers that can accept directories, add an explicit parameter/alternate method rather than returning directories by default.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
github-actions
bot
added
the
status: changes-required
This comment has been minimized.
This comment has been minimized.
Member
|
There is another java library which apparently can parse this stuf https://github.com/DmitriiShamrikov/mslinks |
Member
|
Note that resolving works at JabRef (!) If I double click a .lnk file - JabRef opens. Thus "only" the drag and drop disallowance is wrong currently. |
Member
Who works on DmitriiShamrikov/mslinks#29 ? |
Member
|
We need the newver version, because of JDK11 compatibility - https://github.com/DmitriiShamrikov/mslinks/releases/tag/1.0.8 |
Contributor
Author
I tested it by only allowing the .lnk extension. While the file is accepted by the UI, it currently results in an empty table because the Drag and Drop path doesn't seem to trigger the same resolution logic as double-clicking or the file picker. I am currently looking into how CommandLineProcessor or the main entry point handles .lnk files to see if I can reuse that existing resolution logic here without adding new dependencies like mslinks. |
Contributor
Author
|
@koppor I was able to resolve it by synchronizing the two seperate drag and drop code paths by integrating Windows shortcut resolution into the MainTable handlers. Previously dropping a .lnk file directly onto the table was bypassing the resolution logic in OpenDatabaseAction which resulted in "misc" entries. I think applying FileUtil : : resolveIfShortcut within both handleOnDragDroppedTable view and handleOnDragDropped in MainTable.java, and refining the resolution logic in FileUtil.java using mslinks.ShellLink now correctly resolves shortcuts. Let me know what you think! |
Member
|
recent |
koppor
requested changes
Feb 11, 2026
| } | ||
|
|
||
| private boolean isAcceptedFile(Path path) { | ||
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); |
Member
There was a problem hiding this comment.
Condition wrong - i think, resolving should take place
Suggested change
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); | |
| return FileUtil.isBibFile(FileUtils.resolveIfShortcut(path)); |
| // Resolve any shortcuts to their targets | ||
| List<Path> resolvedFiles = filesToOpen.stream() | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .collect(Collectors.toList()); |
Member
There was a problem hiding this comment.
Add comment that list needs to be modifiable.
CHANGELOG.md
Outdated
|
|
||
| ### Fixed | ||
|
|
||
| - Allowed drag and drop of Windows shortcut (.lnk) files to open libraries [#15036] (https://github.com/JabRef/jabref/issues/15036) |
koppor
reviewed
Feb 11, 2026
| import org.jabref.model.entry.LinkedFile; | ||
| import org.jabref.model.entry.field.StandardField; | ||
|
|
||
| import mslinks.ShellLink; |
Member
There was a problem hiding this comment.
@Siedlerchr Some dependency seems to already include mslinks :)
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. |
Contributor
|
Do not mark a PR as ready-for-review while the CI is running. CI checks need to be completed and passed. |
Contributor
Review Summary by QodoSupport drag and drop of Windows shortcut (.lnk) files
WalkthroughsDescription• Added support for dragging and dropping Windows shortcut (.lnk) files • Implemented shortcut resolution using mslinks library for target path detection • Updated file acceptance logic in drag-and-drop handlers across UI components • Resolves shortcuts before opening libraries to prevent file errors Diagramflowchart LR
A["User drags .lnk file"] --> B["FrameDndHandler accepts .lnk files"]
B --> C["OpenDatabaseAction resolves shortcut"]
C --> D["FileUtil.resolveIfShortcut uses mslinks"]
D --> E["Target .bib file opens in JabRef"]
F["MainTable drag-drop also resolves shortcuts"] --> D
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java
|
Contributor
|
Do not mark a PR as ready-for-review if changes are required. |
Contributor
|
Persistent review updated to latest commit 510012a |
Comment on lines
242
to
244
| private boolean isAcceptedFile(Path path) { | ||
| return FileUtil.isBibFile(path) || FileUtil.isShortcutFile(path); | ||
| } |
Contributor
There was a problem hiding this comment.
1. .lnk accepted on non-windows platforms 🐞 Bug ✓ Correctness
isAcceptedFile() calls FileUtil.isShortcutFile() with no OS guard. On macOS/Linux a dropped .lnk file passes the filter, but resolveIfShortcut() is a no-op there and returns the original .lnk path, which is then fed to the BibTeX parser producing confusing parse errors.
Agent Prompt
## Issue description
`isAcceptedFile()` in `FrameDndHandler` accepts `.lnk` files on all operating systems because `FileUtil.isShortcutFile()` only checks the file extension. On non-Windows systems `resolveIfShortcut()` is a no-op and returns the original `.lnk` binary path, which is then parsed as BibTeX.
## Issue Context
- `FileUtil.isShortcutFile()` has no OS check (extension-only test).
- `FileUtil.resolveIfShortcut()` guards with `!OS.WINDOWS` and returns the original path immediately on non-Windows.
- `openTheFile()` only checks `Files.exists()`, so the `.lnk` binary reaches the parser.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java[242-244]
Change the method to:
```java
private boolean isAcceptedFile(Path path) {
return FileUtil.isBibFile(path) || (OS.WINDOWS && FileUtil.isShortcutFile(path));
}
```
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
168
to
171
| // Resolve any shortcuts to their targets | ||
| List<Path> resolvedFiles = filesToOpen.stream() | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .collect(Collectors.toList()); |
Contributor
There was a problem hiding this comment.
2. No .bib validation after shortcut resolution 🐞 Bug ✓ Correctness
After resolving a .lnk shortcut, the code never checks whether the resolved target is a .bib file. A shortcut pointing to a .pdf, .docx, or any other file type is silently forwarded to openTheFile() and parsed as a BibTeX database, producing confusing errors with no explanation.
Agent Prompt
## Issue description
After `resolveIfShortcut()` maps each path to its target, there is no check that the resolved target is actually a `.bib` file. A `.lnk` pointing to a `.pdf` or any other file type will be passed to `openTheFile()` and parsed as BibTeX.
## Issue Context
- The old code used `filter(FileUtil::isBibFile)` in `getBibFiles()` to guarantee only `.bib` files reached `openFiles()`.
- The new code removes that guarantee: `isAcceptedFile()` admits all `.lnk` files, and `openFiles()` does not re-validate after resolution.
- `openTheFile()` only checks `Files.exists()` (line 220), not the file extension.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java[168-171]
Add a `.bib` filter after resolution:
```java
List<Path> resolvedFiles = filesToOpen.stream()
.map(FileUtil::resolveIfShortcut)
.filter(FileUtil::isBibFile) // ← ensure resolved target is a .bib file
.collect(Collectors.toList());
```
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
556
to
561
| } catch (IOException | ShellLinkException e) { | ||
| LOGGER.warn("Could not resolve shortcut {}", path, e); | ||
| } | ||
|
|
||
| return path; | ||
| } |
Contributor
There was a problem hiding this comment.
3. Failed resolution falls back to parsing .lnk binary 🐞 Bug ⛯ Reliability
When resolveIfShortcut() fails (exception thrown, or the shortcut target does not exist on disk), it returns the original .lnk path. That path passes openTheFile()'s Files.exists() guard (the .lnk file itself exists), so the binary .lnk content is fed to the BibTeX parser instead of surfacing a clear "shortcut target not found" error. Broken/orphaned shortcuts are common in real-world Windows usage.
Agent Prompt
## Issue description
When `resolveIfShortcut()` cannot resolve a shortcut (broken link, missing target, or parse exception), it returns the original `.lnk` path. This path passes `openTheFile()`'s `Files.exists()` check and the binary `.lnk` content is parsed as BibTeX, producing confusing errors instead of a clear failure message.
## Issue Context
- `resolveIfShortcut()` returns the original path on exception (line 556-560) and when the resolved target does not exist (line 552-554).
- The `.lnk` file itself exists on disk, so `openTheFile()`'s `Files.exists()` guard (line 220) does not filter it out.
- Broken shortcuts (target deleted or moved) are common in real-world Windows usage.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[549-561]
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java[168-171]
Preferred fix — add `filter(FileUtil::isBibFile)` after resolution in `openFiles()` (covers both this issue and Finding 2):
```java
List<Path> resolvedFiles = filesToOpen.stream()
.map(FileUtil::resolveIfShortcut)
.filter(FileUtil::isBibFile)
.collect(Collectors.toList());
```
Alternatively, change `resolveIfShortcut()` to return `Optional<Path>` so callers can explicitly handle resolution failure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Member
|
Hi @faneeshh, can we get an update on this PR? There seems to be unresolved comments. |
This comment has been minimized.
This comment has been minimized.
Contributor
Author
Sorry I forgot I had this PR drafted. I've implemented all the changes suggested above and it should be fine now. Let me know if any further changes are needed. |
✅ All tests passed ✅🏷️ Commit: 138fee6 Learn more about TestLens at testlens.app. |
Contributor
Review Summary by QodoAdd drag and drop support for Windows shortcut files
WalkthroughsDescription• Added support for dragging and dropping Windows shortcut (.lnk) files • Resolves shortcuts to target .bib files before opening libraries • Integrated shortcut resolution in file handling and drag-drop operations • Uses mslinks library for Windows shortcut target resolution Diagramflowchart LR
A["User drags .lnk file"] --> B["FrameDndHandler/MainTable"]
B --> C["FileUtil.resolveIfShortcut"]
C --> D["ShellLink resolves target"]
D --> E["Opens target .bib file"]
File Changes1. jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java
|
Contributor
Code Review by Qodo
1. Path.of(targetPath) may crash
|
Member
|
Please fix the mandatory checks section of the PR. You can copy-paste from another PR or from our PULL_REQUEST_TEMPLATE |
Comment on lines
+549
to
+555
| if (targetPath != null && !targetPath.isEmpty()) { | ||
| Path resolvedPath = Path.of(targetPath); | ||
|
|
||
| if (Files.exists(resolvedPath) && !Files.isDirectory(resolvedPath)) { | ||
| return resolvedPath; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
1. path.of(targetpath) may crash 📘 Rule violation ⛯ Reliability
resolveIfShortcut calls Path.of(targetPath) without handling InvalidPathException, which can crash the app for malformed/unsupported shortcut targets. This violates the requirement to handle edge cases and missing/invalid external input safely.
Agent Prompt
## Issue description
`FileUtil.resolveIfShortcut` converts the shortcut target string to a `Path` using `Path.of(targetPath)` without handling `InvalidPathException`. If a `.lnk` points to an invalid/unsupported path string, this can throw an unchecked exception and crash the flow.
## Issue Context
The shortcut target is external/untrusted data coming from the `.lnk` file. The method currently only catches `IOException | ShellLinkException`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[545-561]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
541
to
+545
| if (event.getDragboard().hasFiles()) { | ||
| List<Path> files = event.getDragboard().getFiles().stream().map(File::toPath).collect(Collectors.toList()); | ||
| List<Path> files = event.getDragboard().getFiles().stream() | ||
| .map(File::toPath) | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .collect(Collectors.toList()); |
Contributor
There was a problem hiding this comment.
2. resolveifshortcut on ui thread 📘 Rule violation ➹ Performance
Drag-and-drop handlers call FileUtil.resolveIfShortcut synchronously while processing JavaFX drag events, which may perform filesystem IO and block the UI thread. This risks UI freezes during drag/drop operations.
Agent Prompt
## Issue description
The UI drag-and-drop handlers synchronously resolve `.lnk` shortcuts, which can involve filesystem IO and block the JavaFX UI thread.
## Issue Context
`FileUtil.resolveIfShortcut` parses `.lnk` files via `mslinks.ShellLink`, which may be slow and should not run during UI event handling.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java[541-545]
- jabgui/src/main/java/org/jabref/gui/frame/FrameDndHandler.java[238-244]
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[540-561]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+168
to
+173
| // Resolve any shortcuts to their targets and filter to only .bib files. | ||
| // The resulting list must remain modifiable for downstream processing (iterator.remove() calls below). | ||
| List<Path> resolvedFiles = filesToOpen.stream() | ||
| .map(FileUtil::resolveIfShortcut) | ||
| .filter(FileUtil::isBibFile) | ||
| .collect(Collectors.toList()); |
Contributor
There was a problem hiding this comment.
3. Uppercase .bib not opened 🐞 Bug ✓ Correctness
OpenDatabaseAction.openFiles now filters files with FileUtil.isBibFile, which is effectively case-sensitive because getFileExtension returns the raw extension. This can cause valid libraries like "FILE.BIB" (or other non-lowercase variants) to no longer open or raise an already-open tab.
Agent Prompt
### Issue description
`OpenDatabaseAction.openFiles` now filters inputs using `FileUtil.isBibFile`. Because `isBibFile` does an exact match against "bib" and `getFileExtension` returns the raw extension (not lowercased), valid libraries like `MYLIB.BIB` can be filtered out and therefore will no longer open.
### Issue Context
Before this PR, `openFiles` would open any existing file path (no extension gate). The new filter is intended to prevent trying to open `.lnk` binaries as libraries, but it introduces a regression for mixed/uppercase extensions.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[76-96]
- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java[518-524]
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java[168-173]
- jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java[423-433]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
faneeshh
changed the title
Contributor
Author
Should work now |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues and pull requests
Closes #15036
PR Description
Implemented support for dragging and dropping Windows shortcut (.lnk) files into JabRef to open libraries. By resolving these shortcuts using the mslinks library, users can now open BibTeX databases directly via shortcuts, preventing file resolution errors. The implementation ensures that only shortcuts pointing to valid .bib files are accepted and processed.
Steps to test
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)