-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Supply url_fetcher to weasyprint to support /media and /static files
#9394
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for inventree-web-pui-preview canceled.
|
|
@LqdBcnAtWork FYI there are already template tags available to the reporting system for displaying uploaded images:
|
|
|
||
| logger = structlog.getLogger('inventree') | ||
|
|
||
| WE_BASE_URL = 'http://localhost' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An issue here is that the request URL may or may not exactly match the server's internal URL, depending on proxying settings. Adding a hard-coded URL here is not a good idea.
|
Just a clarifying question, would it be possible to use these somehow with That was the "problem" this code was meant to solve. The notes editor uses relative paths (eg: This pull request makes it so |
|
@LqdBcnAtWork yeah I definitely appreciate what you are trying to achieve here. Having another read through, this might not be a terrible idea ;)
So, there are still some issues to deal with here:
|
|
Let us know if you want pointers how the requested changes might be achieved |
|
@LqdBcnAtWork are you still looking into this? |
|
My apologies, I am still planning on doing more on this. But I've been pulled aside to other projects for the time being. I'll get back to this eventually. It's becoming a conversation of when, not if, we'll switch to Inventree. |
|
@LqdBcnAtWork any interest in this still? I think it would be great to get this implemented |
|
Why do we need this at all? Doesn't this work out of the box, because weasyprint can also connect to the server? |
The issue isn't if weasyprint can connect to the server or not. The issue is that weasyprint had no idea it was supposed to connect to a server. No base URL was supplied to weasyprint. As such any relative urls (like image attachments typically are) would error and the image would get ignored. I suppose giving weasyprint what it needs to connect to the server might be a better solution. I'm getting my fork caught back up so I can poke at this again. I'll give that idea a try in a minute. |
|
I don't think connecting to the server is the right approach here. You already have all you need:
|
|
I got it working without any path substitution magic. This may not necessarily be the best route, but I wanted test it anyway. I had to copy the request from This has the benefit of being completely agnostic to storage backend. Downsides: requires a Path substitution magic is probably better. But I have no idea how to make it work with other storage backends. |
|
I also got this working as a prototype. It wouldn't need any changes made to weasyprint. But it does break images when I haven't dug into only injecting the extension when the plan is to use weasyprint. I have no idea how that would work to be honest. This would need to be placed somewhere. (Open for thoughts as to where would be best) from markdown.extensions import Extension
from markdown import Markdown
from markdown.treeprocessors import Treeprocessor
class ImgSrcFixerTreeProcessor(Treeprocessor):
def run(self, root):
for el in list(root.iter("img")):
src = el.attrib.get("src")
print(f"img src='{src}'")
if isinstance(src, str) and src.startswith(MEDIA_URL):
pth = MEDIA_ROOT.joinpath(src[len(MEDIA_URL):]).as_uri()
print(f"img path='{pth}'")
el.attrib["src"] = pth
class ImgSrcFixerExt(Extension):
def extendMarkdown(self, md: Markdown) -> None:
md.treeprocessors.register(
ImgSrcFixerTreeProcessor(md),
"path-fixer",
0
)Then the 'MARKDOWN_EXTENSIONS': ['markdown.extensions.extra', ImgSrcFixerExt()],
'WHITELIST_PROTOCOLS': ['http', 'https', 'file'], |
Pull request with changes from a comment made on #9351.
This allows
weasyprintto grab files from/mediaand/staticlocations by redirecting them toopen()calls with respect toMEDIA_ROOTandSTATIC_ROOTsettings.Probably could be made better. But at least ensures that the resulting path is always still a child of the parent folder.