Skip to content

Support external renderers to PDF (#17635) #21098

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PlushBeaver
Copy link

For some formats, conversion to HTML is unavailable or lossy, while quality conversion of printing to PDF is available.
Examples include Office PPTX slides and TeX papers.

Add new option value markup.XXX.RENDER_CONTENT_MODE = "pdf".
It requires markup.XXX.NEED_POSTPROCESS = false.
In this mode, external renderer is only invoked for the standalone page.
Embedded rendering outputs a PDF.js widget with a link to that page.

The first commit is a required fix for this PR, although not limited to it.

It was always attempted to read the file to be rendered as UTF-8.
The encoding was determined heuristically, regardless of the file type.
Sometimes binary files were not recognized as binary.
Renderers of binary formats were then fed with corrupted data:
binary stream treated as text and encoded in UTF-8.
Only apply heuristics for textual formats, read other formats as-is.

Fixes: b01dce2 ("Allow render HTML with css/js external links (go-gitea#19017)")
Cc: xiaolunwen@gmail.com

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
For some formats, conversion to HTML is unavailable or lossy,
while quality conversion of printing to PDF is available.
Examples include Office PPTX slides and TeX papers.

Add new option value markup.XXX.RENDER_CONTENT_MODE = "pdf".
It requires markup.XXX.NEED_POSTPROCESS = false.
In this mode, external renderer is only invoked for the standalone page.
Embedded rendering outputs a PDF.js widget with a link to that page.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 7, 2022
@lunny lunny added this to the 1.18.0 milestone Sep 7, 2022
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Just adding a blocking comment, as this touches sensitive areas in regards to security, and so just ensuring that gets an extra focus in any review.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 7, 2022
@wxiaoguang
Copy link
Contributor

Just adding a blocking comment, as this touches sensitive areas in regards to security, and so just ensuring that gets an extra focus in any review.

Maybe consider with this one together.

@lunny
Copy link
Member

lunny commented Nov 1, 2022

Should fix #17635

@jolheiser jolheiser modified the milestones: 1.19.0, 1.20.0 Jan 31, 2023
@delvh delvh removed this from the 1.20.0 milestone Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants