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

Update PDF.js version #40

Merged
merged 1 commit into from
Sep 10, 2021
Merged

Update PDF.js version #40

merged 1 commit into from
Sep 10, 2021

Conversation

dheles
Copy link
Contributor

@dheles dheles commented Sep 3, 2021

JIRA Ticket: https://jira.lyrasis.org/browse/ISLANDORA-2530

What does this Pull Request do?

The PDF viewer provided by the pdfjs module has begun failing to load pages. Checking in your browser's debugging console indicates PDF.js is the culprit. Updating the version used in the module fixes the problem.

What's new?

Updates PDF.js to the current version (2.10.377)

How should this be tested?

  • Load a PDF and view it using the un-updated viewer
  • see pages fail to load
  • check your browser's console to see an error related to PDF.js
  • deploy the updated version of the module
  • reload your PDF in the viewer
  • confirm the pages load
  • (optional) check your console to confirm PDF.js 2.10.377 is being utilized

Additional Notes:

The new version of PDF.js additionally adds some new viewing options, e.g. horizontal scrolling

Interested parties

@Islandora/7-x-1-x-committers

@manez
Copy link
Member

manez commented Sep 3, 2021

Unless I'm misunderstanding, it looks like there are two PRs open on this same issue: #39 ?

@DonRichards and @dheles since it's so hard to get 7.x PRs tested and merged these days, maybe you two can coordinate, pick one as canonical, and have the other test it?

@dheles
Copy link
Contributor Author

dheles commented Sep 3, 2021

Oops, my bad. I hadn't noticed Don's PR when submitting. Though when I tested PDF.js 1.10.100, it didn't fix the issue with the particular PDF that was failing to load for me. @DonRichards I presume that version fixes the issue in the context in which you you were testing. Did you happen to test v2.x as well?

@dheles dheles changed the title Update PDF.js version (#1) Update PDF.js version Sep 7, 2021
@DonRichards
Copy link
Member

@dheles I submitted it as a safe alternative but I just tested the latest version (this version) and I don't see any issues with it.

Copy link
Member

@DonRichards DonRichards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works as expected

@DonRichards
Copy link
Member

24 hours and I'll merge

@DonRichards DonRichards merged commit a3711d4 into Islandora:7.x Sep 10, 2021
@dheles
Copy link
Contributor Author

dheles commented Sep 10, 2021

Thanks, @DonRichards !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants