Fix: Comprehensive page navigation for crossref entries in both internal and external viewers (#8128)#15504
Fix: Comprehensive page navigation for crossref entries in both internal and external viewers (#8128)#15504Sherry0121-AC wants to merge 51 commits intoJabRef:mainfrom
Conversation
Review Summary by QodoAdd comprehensive page navigation for crossref entries across all platforms
WalkthroughsDescription• Implement logical-to-physical PDF page translation for crossref entries • Add page number support across all desktop platforms (Windows, macOS, Linux) • Enable internal and external viewers to navigate to correct pages in parent PDFs • Support crossref fallback when child entries lack direct file attachments Diagramflowchart LR
A["Child Entry with crossref"] -->|resolve parent| B["Parent Entry"]
B -->|extract pages field| C["Parse Page Number"]
C -->|PDF with labels| D["PdfPageLabelResolver"]
D -->|logical to physical| E["Physical Page Number"]
E -->|pass to OS| F["Platform-specific Viewer"]
F -->|Windows| G["SumatraPDF/-page"]
F -->|macOS| H["Adobe/Skim/Browser"]
F -->|Linux| I["Evince/Okular/Zathura"]
File Changes1. jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java
|
Code Review by Qodo
1.
|
fe94d90 to
969f699
Compare
…21-AC/jabref into fix-8128-crossref-pages
…21-AC/jabref into fix-8128-crossref-pages
koppor
left a comment
There was a problem hiding this comment.
For what is this PR? University credits or GSoC?
|
A lot of slop int his. PR |
|
The Windows CI failed in :jablib:test (FileUtilTest.java), but this seems to be an existing flaky test on Windows regarding non-atomic file copying, and is entirely unrelated to my changes in this PR. Could you help trigger a re-run, or should I just ignore it for now? |
|
Hi @calixtus and @Siedlerchr , First of all, huge apologies for the "slop" and the messy commits! To give a bit of context: when I ran the test suite locally, a few unrelated tests failed on my machine (mostly due to macOS/Windows environment differences or flaky setups). My initial thought was just to fix them along the way to make the test suite more robust and get a clean local build. I completely understand now that this was out of scope and made the PR hard to review. I have just reverted all those unrelated test modifications. The PR is now clean and strictly focused only on the original crossref page navigation issue. Thanks for your patience, and the PR is ready for review again! |
|
Windows tests should pass now. |
|
@subhramit Thanks! Yes, the Windows tests are passing perfectly now. @calixtus @koppor I have updated the code based on your previous review comments, and all CI checks are currently green. Could you please take another look? Please let me know if there are any further issues to address. Thank you! |
|
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. |
|
Hi @calixtus @koppor @subhramit @Siedlerchr , the merge conflicts have been resolved and all checks are green. Are there any further issues to address, or is this ready to be merged? Thanks! |
We'll take a look soon. Currently we are a bit busy with other commitments. |
…21-AC/jabref into fix-8128-crossref-pages
Related issues and pull requests
Closes #8128
PR Description
This PR fixes the issue where opening sub-entries (e.g.,
@InCollection) via thecrossreffield failed to navigate to the correct start page in external and internal viewers. By implementing a logical-to-physical page translation layer and optimizing the navigation pipeline, JabRef now correctly targets the intended page, gracefully handling PDF front-matter offsets when present. This ensures a consistent and accurate research experience across all desktop platforms.Steps to test
@Bookentry in JabRef and link a local PDF file to it.(Optional but recommended: Use a PDF with roman-numeral front matter to verify the new logical-to-physical page translation feature).
@InCollectionentry. Setcrossrefpointing to the parent book's citation key, and set thepagesfield to a specific range (e.g.,186--206).Screenshots:



![External Viewer Jump]
Screenshots: The external browser successfully navigating to physical page 200 (logical page 186) after pressing F4.
![Internal Viewer Jump]



Screenshots: The internal document viewer accurately resolving the parent file and jumping to the correct page.
Known Limitations
Checklist