Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write XMP Button should be disabled when action is in progress #8691

Closed
koppor opened this issue Apr 18, 2022 · 7 comments · Fixed by #8728
Closed

Write XMP Button should be disabled when action is in progress #8691

koppor opened this issue Apr 18, 2022 · 7 comments · Fixed by #8728
Labels
component: entry-editor component: ui good first issue An issue intended for project-newcomers. Varies in difficulty.

Comments

@koppor
Copy link
Member

koppor commented Apr 18, 2022

We offer the functionality to write XMP to a PDF:

grafik

We tried to fix it at #8658

However, the fix does not work:

com.google.common.util.concurrent.UncheckedExecutionException: java.lang.IllegalStateException: 1-based index not found: 8 at [email protected]/com.google.common.cache.LocalCache$Segment.get(Unknown Source) at [email protected]/com.google.common.cache.LocalCache.get(Unknown Source) at [email protected]/com.google.common.cache.LocalCache.getOrLoad(Unknown Source) at [email protected]/com.google.common.cache.LocalCache$LocalLoadingCache.get(Unknown Source) at [email protected]/com.google.common.cache.LocalCache$LocalLoadingCache.getUnchecked(Unknown Source) at [email protected]/org.jabref.logic.pdf.FileAnnotationCache.getFromCache(Unknown Source) at [email protected]/org.jabref.gui.entryeditor.fileannotationtab.FileAnnotationTabViewModel.lambda$reloadAnnotations$1(Unknown Source) at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(Unknown Source) at java.base/java.security.AccessController.doPrivileged(Unknown Source) at [email protected]/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(Unknown Source) at [email protected]/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(Unknown Source) at [email protected]/com.sun.glass.ui.win.WinApplication._runLoop(Native Method) at [email protected]/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(Unknown Source) at java.base/java.lang.Thread.run(Unknown Source) Caused by: java.lang.IllegalStateException: 1-based index not found: 8 at [email protected]/org.apache.pdfbox.pdmodel.PDPageTree.get(Unknown Source) at [email protected]/org.apache.pdfbox.pdmodel.PDPageTree.get(Unknown Source) at [email protected]/org.apache.pdfbox.pdmodel.PDPageTree.get(Unknown Source) at [email protected]/org.apache.pdfbox.pdmodel.PDPageTree.get(Unknown Source) at [email protected]/org.jabref.logic.pdf.PdfAnnotationImporter.importAnnotations(Unknown Source) at [email protected]/org.jabref.logic.pdf.EntryAnnotationImporter.lambda$importAnnotationsFromFiles$2(Unknown Source) at java.base/java.util.Optional.ifPresent(Unknown Source) at [email protected]/org.jabref.logic.pdf.EntryAnnotationImporter.importAnnotationsFromFiles(Unknown Source) at [email protected]/org.jabref.logic.pdf.FileAnnotationCache$1.load(Unknown Source) at [email protected]/org.jabref.logic.pdf.FileAnnotationCache$1.load(Unknown Source) at [email protected]/com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(Unknown Source) at [email protected]/com.google.common.cache.LocalCache$Segment.loadSync(Unknown Source) at [email protected]/com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(Unknown Source) ... 14 more

Proposal: When clicking on the button

  1. JabRef disalbes the button
  2. JabRef does its work
  3. JabRef enabled the button

This a) provides the user a feedback that JabRef is working and b) removes synchronized statements from the code.

@koppor koppor added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Apr 18, 2022
@Siedlerchr Siedlerchr changed the title Button should be disabled when action is in progress Write XMP Button should be disabled when action is in progress Apr 19, 2022
@melmorsy
Copy link
Contributor

Hi,
I'm interested in working on this issue, it's my first code contribution to open-source. I managed to get JabRef up and running from IntelliJ and I guess I found the button/action of interest (writeMetadataToPdf at LinkedFilesEditor, or am I at the wrong place?), however I'm not sure how to reproduce this issue, it would be great if someone could help please with providing steps to reproduce this issue from scratch as it's my first time to use JabRef.
Thanks

@ThiloteE
Copy link
Member

Mhm I was the user that provided this error message to Oliver. I have not found out how to trigger the error message completely though. What I know:

  1. Have an entry with bibliographic data
  2. Have an pdf file attached to this entry
  3. Press the button to write XMP metadata multiple times very fast in a row
  4. ? - I think there could be an additional condition. Maybe it has something to do with file annotations tab (which is trying to read annotations from the document).

Additional Info:

This bug here is the underlying problem, which necessitates that saving to and reading from the PDF file should not happen at the same time. So in the context of the error message: I assume, apparently something still tries to read or write, while a write process is still ongoing, which is BAD.

@ThiloteE
Copy link
Member

Thank you for your interest by the way! :-)

If you are here, you are at the right place:

image

@melmorsy
Copy link
Contributor

Thank you @ThiloteE ! :) - I'm planning to push the proposed enhancement to disable the button once it's clicked then re-enable it once the write is done, will have a look at the contribution guidelines before pushing the change though, let me know please if you have any suggestions.

melmorsy pushed a commit to melmorsy/jabref that referenced this issue Apr 26, 2022
Disable the button before starting the 'write' task then re-enables the button after the task is complete
@the-star-sea
Copy link
Contributor

hello.I am a new bee of open source project and I am interested in this issue.Could I have a try?

@ThiloteE
Copy link
Member

Hello @the-star-sea I am thankful for your interest about working on JabRef.

It looks like melmorsy already is working on this, so unless melmorsy can't make it work or has lost interest, maybe it would be better to choose another issue.

If you are pretty new to open source programming, have a look at issues with the tag "good first issues". If you are a student, the "candidates for university projects" page offers some issues of varying difficulty and scope and that have been estimated to be compatible with university courses as well and often bring a larger feature to JabRef. Of course, feel free to tackle one of the other "normal" issues around, if you feel you have the knowledge or the willingness to take them on. There are many around!

Because there are so many issues, prioritization would boost JabRef's development. For example, if you have decided to work on a issue related to "search", have a look at multiple issues with the label "search" and only afterwards decide on which one you want to work on. What should be avoided: Having a look at the newest issues and blindly choose an issue from the first page. Thank you! There might be important issues on the first page, but there might also be very important issues further down the list. Labels will help you a lot. Furthermore, bugs are actively prioritized by JabRef maintainers and getting rid of high priority ones feels extra nice!

Having said all that, at the end of the day, of course, you are the one free to choose what to work on and what feature you prefer JabRef to have. Such is the nature of open source software.

Thank you!

@melmorsy
Copy link
Contributor

Hi,
Apologies for the delayed reply, I have got a PR with a fix proposal however there's a suggestion there to use SimpleCommand that I didn't manage to figure out how it would work unfortunately: #8728 - I'll add a comment to the PR to see how could we conclude this, or please let me know if you have any suggestions

melmorsy pushed a commit to melmorsy/jabref that referenced this issue May 23, 2022
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Create WriteMetadataToPdfCommand, bind its 'executable' property to the button's 'disable' property
melmorsy pushed a commit to melmorsy/jabref that referenced this issue May 23, 2022
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Create WriteMetadataToPdfCommand, bind its 'executable' property to the button's 'disable' property
melmorsy pushed a commit to melmorsy/jabref that referenced this issue May 23, 2022
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Resolve checkstyle issues
melmorsy pushed a commit to melmorsy/jabref that referenced this issue May 23, 2022
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Resolve checkstyle issues
Siedlerchr pushed a commit that referenced this issue May 30, 2022
…s in progress (#8728)

* fixes #8691
Disable the button before starting the 'write' task then re-enables the button after the task is complete

* Update CHANGELOG.md

* fixes #8691
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Create WriteMetadataToPdfCommand, bind its 'executable' property to the button's 'disable' property

* fixes #8691
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Create WriteMetadataToPdfCommand, bind its 'executable' property to the button's 'disable' property

* Update WriteMetadataToPdfCommand.java

Resolving merge conflict

* fixes #8691
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Resolve checkstyle issues

* fixes #8691
Disable the button before starting the 'write' task then re-enables the button after the task is complete
Resolve checkstyle issues

Co-authored-by: Mohamed El-Morsy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: entry-editor component: ui good first issue An issue intended for project-newcomers. Varies in difficulty.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants