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

To be able to render remote images locally. #2256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amouhzi
Copy link

@amouhzi amouhzi commented Oct 5, 2020

This Pull Request allows us to load local images accessible in the browser and which are stored in the root document:

Example:

<img src="/image.png" />

@bsweeney
Copy link
Member

bsweeney commented Oct 6, 2020

Can you provide a bit more context? I've been able to render that type of image reference without issue. Also, I think this could break rendering <img src="image.png" /> depending on how chroot is defined, would it not? Is there a relevant issue and, if not, can you provide context here, such the PHP around instantiating Dompdf and rendering the document?

@amouhzi
Copy link
Author

amouhzi commented Oct 6, 2020

Usually we put the image files in the document root of the web server. So these files can be accessed from the browser just with a url like: /image.png, but the library cannot find them because it looks for a file with this as the full path and cannot find it which makes sense. In order to be able to use the same html for the display in the browser and for the generation of pdf, this file must also be found by the library without having to make two versions of the html. So I added conditions that allow to search for the file in the chroot only if could not be found.

@amouhzi amouhzi changed the title Fix rendering local images To be able to render remote images locally. Oct 6, 2020
@amouhzi
Copy link
Author

amouhzi commented Oct 6, 2020

Here is an example: https://github.com/amouhzi/symfony-dompdf-demo

@bsweeney
Copy link
Member

bsweeney commented Oct 7, 2020

I'll have to think through this scenario a bit more.

@amouhzi
Copy link
Author

amouhzi commented Jan 22, 2021

any news about this ?

@bsweeney
Copy link
Member

Nothing yet. I'm not entirely opposed to the idea I just need to think about the implementation. A few comments/questions:

  1. The implementation as written won't work. The chroot option was modified to be an array instead of a single path to accommodate usage scenarios where resource paths don't share a common root.
  2. Even without the change to the chroot option, why not use the base path instead. This would make a bit more sense from the perspective of what the base (or root) path should be.
  3. Wondering if this should maybe be an options setting.

Regarding the example of loading "/image.png" ... one way to accommodate this without any changes would be to set the base href to the site domain. Then "/image.png" gets interpreted as "http://example.com/image.png". The only drawback is that it does incur a bit of a performance penalty in having to load the resource through the web server instead of using local file access.

@bsweeney
Copy link
Member

FYI in thinking through this some more it's probably a useful option to specify actual chroot-like functionality, but it's going to require more work thank done here since you're co-opting the functionality around the (poorly named) existing chroot option.

@bsweeney bsweeney changed the base branch from master to develop April 13, 2021 15:57
@bsweeney bsweeney added this to the 2.0.0 milestone Apr 13, 2021
@bsweeney bsweeney changed the base branch from develop to master November 8, 2021 17:06
@bsweeney bsweeney modified the milestones: 2.0.0, 2.1.0 May 9, 2022
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.

None yet

2 participants