Fix crash when scrolling through cited by in Citation relations #12877
Conversation
Uses Webview instead of Label and TextFlowLimited when trying to display right to left text (like persian or arabic) in BibEntryView. This resolves the issue caused by a javafx bug that causes an exception when rendering wrapped RTL text. Fixes JabRef#12410
|
|
||
| private static Node createTextNode(String text, String fieldName) { | ||
| if (isRTL(text)) { | ||
| WebView webView = new WebView(); |
There was a problem hiding this comment.
Creating a WebView is an expensive operation, you should use our WebViewStore to get a pre-allocated one
There was a problem hiding this comment.
Creating a webview for each entry in the citation relations tab to fix a crash in JabRef would crash your whole computer. This is not acceptable even with the WebViewStore. WebView is like a lightweight browser, it allocates a substantial amount of memory. We made it work for entry preview in the entry editor because entries are previewed one by one, so we can reuse the same webview instance for all entries and update the content, we can't do that here.
We need another solution.
There was a problem hiding this comment.
Thank you for your feedback, I will try to find another solution then.
There was a problem hiding this comment.
After exploring more, I've implemented a simpler approach that uses Labels and simple Text node for rtl text, but as single-line text with horizontal scrolling, avoiding the wrapping of the text, as you can see in this video:
fix.webm
Would this be an acceptable compromise? It is not an ideal solution, but it looks like the best solution would be for the JavaFX bug to be fixed. Like this, at least the OS and JabRef don't crash.
|
Please check the submodules changes https://devdocs.jabref.org/code-howtos/faq.html#submodules |
Siedlerchr
added
the
status: changes required
|
@koppor no submodule diff check -> false positive (you can reopen the test PR for experiments). |
src/main/resources/csl-styles
Outdated
There was a problem hiding this comment.
Please revert the submodule changes as per https://devdocs.jabref.org/code-howtos/faq.html#submodules
There was a problem hiding this comment.
Thank you for your feedback.
I have reverted the submodule changes. I am currently exploring different alternative solutions to fix the issue, without using WebViews, since they are expensive.
|
@GuilhermeRibeiroPereira you can mark the PR ready for review when done |
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
@trag-bot didn't find any issues in the code! ✅✨ |
Uses Webview instead of Label and TextFlowLimited
when trying to display right to left text
(like persian or arabic) in BibEntryView. This resolves the issue caused by a javafx bug that causes an exception when rendering wrapped RTL text.
Fixes #12410
Closes #12410
I found out that this crash happens due to a bug in javafx described here: https://bugs.openjdk.org/browse/JDK-8340499?jql=labels%20%3D%20dcsad. There is an entry in the Citation Relation tab (at cited by) for the DOI referred by the user that has text in Persian, which is right to left (RTL), and when trying to display the text, which contains several lines and is wrapped, using labels for the title, authors and journal, the behavior described in the previous link occurs and the program and the OS crash and stop, due to a lot of exception dialogs appearing on the screen. This also happens for the summary, which is a textFlowLimited.
After testing, the bug seems to remain present in more recent versions of javafx, even though the issue has been closed.
One solution I found was, in BibEntryView.java, to use a WebView that contains only the text for these elements, when they have RTL text, since it has good support for RTL text and does not cause crashes. The downside is that these entries that contain RTL text appear different from the others, which can be seen in the video. It shows that a crash no longer occurs when scrolling through the "cited by" of the article with DOI: 10.1016/j.nimb.2024.165287.
https://www.youtube.com/watch?v=R-f_hdGdoQ8
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)