Add PDF Viewer

[tobiasdiez]

Since @lynyus wants to implement a fulltext pdf search 😛, we need a proper way to display the results. In this PR a very plain PDF viewer is implemented. Most #PDFs are displayed correctly, maybe with a few small glitches here and there. Basic features like scrolling, changing pages and zooming work. In addition, while being in "live mode" the linked document of the currently selected BibEntry is shown - this is very handy in a two-display setup since one can have JabRef's maintable on one screen and the always-current PDF on the second.

Feedback welcome! (I will update the language files before merge.)

• Similar project: https://github.com/IDRSolutions/maven-OpenViewerFX-src
• Extended example: https://github.com/james-d/PdfViewer/blob/master/src/pdfviewer/PdfViewer.fxml and https://github.com/james-d/PdfViewer/blob/master/src/pdfviewer/Controller.java

---

• Change in CHANGELOG.md described
• Tests created for changes
• Screenshots added (for bigger UI changes)
• Manually tested changed features in running JabRef
• Check documentation status (Issue created for outdated help page at help.jabref.org?)
• If you changed the localization: Did you run gradle localizationUpdate?

[stefan-kolb]

Code looks ok to me 👍 Need to try it out, still.

[Siedlerchr]

Code looks good,.but you still have GPL headers in the code. Maybe they got auto generated?

[Siedlerchr]

The header still days GPL

[tobiasdiez]

Good catch! Some of the code was written in an era, when such copyright notices were still included 😄

[stefan-kolb]

Ok, tested it a bit. Some thoughts:

• live view is nice 👍
• functionality currently quite hidden via view - show document viewer. Probably need a magnifier glass somewhere. Not sure where tho.
• PDF rendering is a bit slow.
• Also, it does not render backgrounds correctly and also the casing of some texts seem to be odd
  
  
  
  
• Highlighting does not display correctly
  
• Comments cannot be seen
• I can no longer navigate through the main panel with UP/DOWN buttons

We already had such a viewer before and removed it. Isn't it possible to do this natively with Acrobat or so? I doubt we will ever get a really good rendering by ourselves.

[tobiasdiez]

Thanks for the feedback. I also noticed some of these rendering issues (and, in addition, some math characters are displaced or not shown at all). I did some investigation and it seems that most of these problems are fixed/improved upon with PDFBox 2.0. But the update to 2.0 is still not possible for us #1096. The only other decent PDF rendering engine I found for java is itext, but it is licensed under the incompatible AGPL (this is also the reason why #2474 is blocked at the moment).

Do we want to include the PDF viewer nonetheless or do you think the quality is too bad for "production"?

We already had a PDF viewer? I was only aware of the PDF thumbnail, which was totally buged.
The idea behind this PR is not to implement a full-fledged PDF viewer and editor, but only provide a reasonable good minimal viewer. Its main purpose should be to let the user quickly decide whether he has picked the right document for further reading. In addition, I plan to show the preview also in the import dialog to give more context there. Some further plans are integration with and enrichment of the core bibliographic features of JabRef: pdf fulltext search, visualization of cites / cited by relationships, automatic import of referenced literature.
Thus the idea is kind of similar to the PDF previewer in firefox: provide a means for a quick inline PDF preview for convenience but don't try to write yet another PDF editor.

For me the navigation in the maintable with arrow keys still works.

[lenhard]

I also played around with it a little, and it seems to work fine in most cases. I got a lot of warnings, although the pdf displayed fine to me, so it would be cool if the tool could be a little less verbose. On open:

Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss
Apr 05, 2017 9:58:45 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-accent' while resolving lookups for '-fx-background-color' from rule '*.progress-indicator:indeterminate *.segment' in stylesheet jar:file:/C:/Program%20Files/Java/jdk1.8.0_121/jre/lib/ext/jfxrt.jar!/com/sun/javafx/scene/control/skin/modena/modena.bss

And then every time I scrolled to a new page:

09:58:46.787 [Thread-22] WARN  org.apache.pdfbox.pdmodel.graphics.xobject.PDXObjectImage - Colour key masking isn't supported
09:58:46.845 [Thread-22] INFO  org.apache.pdfbox.pdmodel.font.PDTrueTypeFont - Using font Times New Roman instead
09:58:46.864 [Thread-22] WARN  org.apache.pdfbox.pdmodel.graphics.xobject.PDXObjectImage - Colour key masking isn't supported
09:58:46.920 [Thread-22] INFO  org.apache.pdfbox.pdmodel.font.PDTrueTypeFont - Using font Times New Roman instead

Personally, I would not put too much effort in this as I think pdf viewing is not a central purpose of JabRef. I also hope that we do not open a can of worms here and once we have the feature, the users will find 1000 issues and improvements to pdf viewing, which would distract us from other tasks. However, JabRef is open source and if some of the developers want to have a pdf viewer in there and are ready to invest the effort to get it in, this is fine by me.

I wonder a little what the relation of this feature is to the pdf comments tab that we have now. Isn't this somehow a duplication of functionality? What are your thoughts about this?

[lenhard]

This is an architectural violation. Please reverse it: The conversion of ParsedFileField to a Path should be moved to FileUtil

[tobiasdiez]

May I also put FileUtil in model since it is our abstraction of the File object? Reason: I really want the ParsedFileField to have a method that returns the path to the linked file. There are around 15 places where expandFilename is used upon ParsedFileField::getLink and all of them could be replaced by the new toPath method.

[lenhard]

The thing is, FileUtil has more dependencies to logic classes, which would then also need to move to model. At some point, the distinction between the two packages would be completely lost.

Can't you just add a method public static Path getPath(ParsedFileField fileField) to FileUtil instead? Not much better than the current version, I know, but at least a little. Alternatively, extract the methods you need, and just those, into a new FileUtil class in model (maybe with a better name). It wouldn't hurt to break up these util classes into more meaningful smaller units. They all devolve into god classes...

[tobiasdiez]

I moved the method to FileUtil as you proposed and will open a new PR with the refactoring since it turned out to be a bit more extensive than I have expected.

[lenhard]

And one more thing that just came to my mind: What are the licenses of the libraries you are including here?

[tobiasdiez]

@lenhard Thanks for the additional feedback.
I changed the log output of pdfbox to errors, so all the warnings are hidden now. The "-fx-background-color" messages seem to be a bug in JavaFX at least it referenced a code point in JavaFX's own stylesheet modena.css.

Moreover, I added the licenses: Apache 2.0 and BSD 2, so this shouldn't be a problem.