enhancement: Don't strip iframe url if target is html#3207
Merged
Conversation
gabestein
commented
Dec 19, 2024
Comment on lines
+291
to
+297
| const renderableNodes = pagedTarget | ||
| ? [filterNonExportableNodes, addHrefsToNotes, blankIframes] | ||
| .filter((x): x is (nodes: any) => any => !!x) | ||
| .reduce((nodes, fn) => fn(nodes), pubDoc.content) | ||
| : [filterNonExportableNodes, addHrefsToNotes] | ||
| .filter((x): x is (nodes: any) => any => !!x) | ||
| .reduce((nodes, fn) => fn(nodes), pubDoc.content); |
Member
Author
There was a problem hiding this comment.
I tried to DRY this but these node traversing functions are called in order on each node in the document and then reduced into a single pubDoc, so you can't just, e.g., check for the paged target and then re-run and concatenate only the iFrame nodes (I think)
isTravis
approved these changes
Dec 19, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue(s) Resolved
Because we go from HTML to PDF, where iFrames are not supported, exporting to HTML previously stripped the iFrame url and replaced it with instructions to visit the main document to see interactive content.
This allows HTML exports to maintain the original URL by passing the target into the
renderStaticHtmlfunction such that if we target pagedJS outputs, we strip the url, but if we target an html download, we don't strip it.Test Plan
pub/jpfkepm9/draftScreenshots (if applicable)
Optional
Notes/Context/Gotchas
There was only one call-site for
renderStaticHtml, so I feel pretty good about this being a very limited change for the better.Supporting Docs