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

Fix path to image resources #1106

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Fix path to image resources #1106

merged 1 commit into from
Jan 16, 2025

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Jan 8, 2025

Follow up to #689

When ./js/pdfjs/web/images/ is resolved and pretty URLs are enabled the result is {SERVER}/apps/files_pdfviewer/js/pdfjs/web/images/, which works as expected. However, if pretty URLs are not enabled the absolute path becomes {SERVER}/index.php/apps/files_pdfviewer/js/pdfjs/web/images/ instead. As the images are static files they are not served by index.php, so they can not be found and therefore are not rendered in the PDF.

To solve that now the path is generated in PHP and provided through the template, similarly to how it is done, for example, for the path to the cmap files.

How to test

  • Do not setup pretty URLs in Nextcloud
    • If they are set already you can disable them removing htaccess.RewriteBase from config.php with occ config:system:delete htaccess.RewriteBase and then updating .htaccess with occ maintenance:update:htaccess
  • Upload a PDF file with annotations, for example, rc_annotation.pdf from PDF.js repository
  • Open the PDF file with the viewer

Result with this pull request

The note icon is shown in the annotation of the PDF

Result without this pull request

The note icon is not shown in the annotation of the PDF

@danxuliu
Copy link
Member Author

danxuliu commented Jan 8, 2025

/compile amend /

@danxuliu
Copy link
Member Author

danxuliu commented Jan 8, 2025

/backport to stable30

@danxuliu
Copy link
Member Author

danxuliu commented Jan 8, 2025

/backport to stable29

@danxuliu
Copy link
Member Author

danxuliu commented Jan 8, 2025

/backport to stable28

@szaimen
Copy link
Collaborator

szaimen commented Jan 15, 2025

Hi @danxuliu I'd like to test this but currently the installation fails when trying to test with my easy-test instance due to #1112. So this needs to go in first and afterwards I can try it out :)

When "./js/pdfjs/web/images/" is resolved and pretty URLs are enabled
the result is "{SERVER}/apps/files_pdfviewer/js/pdfjs/web/images/",
which works as expected. However, if pretty URLs are not enabled the
absolute path becomes
"{SERVER}/index.php/apps/files_pdfviewer/js/pdfjs/web/images/" instead.
As the images are static files they are not served by "index.php", so
they can not be found and therefore are not rendered in the PDF.

To solve that now the path is generated in PHP and provided through the
template, similarly to how it is done, for example, for the path to the
cmap files.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@szaimen szaimen force-pushed the fix-path-to-image-resources branch from 81fb907 to 343adfc Compare January 16, 2025 09:53
Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Tested and works :)

@danxuliu danxuliu merged commit 94cd880 into master Jan 16, 2025
39 checks passed
@szaimen
Copy link
Collaborator

szaimen commented Jan 16, 2025

However I found a small problem while testing which is probably unrelated:
image

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

Successfully merging this pull request may close these issues.

2 participants