Skip to content

Make print page (print.html) links link to anchors on the print page#1738

Closed
HollowMan6 wants to merge 2 commits intorust-lang:masterfrom
HollowMan6:master
Closed

Make print page (print.html) links link to anchors on the print page#1738
HollowMan6 wants to merge 2 commits intorust-lang:masterfrom
HollowMan6:master

Conversation

@HollowMan6
Copy link
Contributor

Resolves #1736

Let all the anchors id on the print page to have a path id prefix to
help locate.

e.g. bar/foo.md#abc -> #bar-foo-abc

Also append a dummy div to the start of the original page to make sure
that original page links without an anchor can also be located.

Signed-off-by: Hollow Man hollowman@opensuse.org

@HollowMan6 HollowMan6 marked this pull request as draft February 2, 2022 13:53
@HollowMan6 HollowMan6 force-pushed the master branch 7 times, most recently from 39ffbc0 to e3cca8f Compare February 2, 2022 16:00
@HollowMan6 HollowMan6 marked this pull request as ready for review February 2, 2022 16:06
@HollowMan6
Copy link
Contributor Author

Tested on the following Rust Bookshelf books with the js code here checking broken links also unavailable anchors on print.html.

With this PR now all the links on the print page are self-contained, no broken links found except for those who are broken originally, or has self-made JavaScript that needs adaptation (ferris.js in Rust Programming Language).

Title Source Original Book Online Version
Cargo Book Source HTML
Edition Guide Source HTML
Embedded Rust Book Source HTML
Mdbook User Guide Source HTML
Rust Reference Source HTML
Rust By Example Source HTML
Rust Programming Language Source HTML
Rustc Book Source HTML
Rustdoc Book Source HTML
Rustonomicon Source HTML

Please review, Will appreciate if this PR can be merged.

cc: @ehuss

@dyaso
Copy link

dyaso commented Jul 15, 2022

This was very helpful for me, it would be nice if this could be merged so i didn't have to build HollowMan6's fork myself in order to generate PDFs whose internal links work

@HollowMan6 HollowMan6 force-pushed the master branch 3 times, most recently from f1ceb69 to 38ecc41 Compare September 27, 2022 21:52
@ehuss ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Oct 13, 2022
@HollowMan6 HollowMan6 force-pushed the master branch 2 times, most recently from 57db987 to 00d56ab Compare October 16, 2022 08:28
@HollowMan6
Copy link
Contributor Author

Hi @ehuss , I've noticed that you added the S-waiting-on-author label to this PR recently, but I can't see any code change requests and even reviews for this PR. Am I missing something and what should I response/clarify?

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

This is just a partial review, there are some other things that I'd like to follow up on.

@ehuss
Copy link
Contributor

ehuss commented Oct 17, 2022

but I can't see any code change requests and even reviews for this PR. Am I missing something and what should I response/clarify?

I'm so sorry. I had a partial review that I wrote a long time ago, but never finished and didn't click the submit button. But GitHub displays the review on the conversation page as-if it was submitted (with a little icon that I overlooked), so I thought I had submitted it.

@HollowMan6
Copy link
Contributor Author

This is just a partial review, there are some other things that I'd like to follow up on.

Done! Thanks for reviewing. Would love to see it get merged in the near future.

@idoleat
Copy link

idoleat commented Nov 12, 2024

Should we change the label to S-waiting-on-review ?

@Dylan-DPC Dylan-DPC added S-waiting-on-review Status: waiting on a review and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Nov 20, 2024
@HollowMan6 HollowMan6 force-pushed the master branch 2 times, most recently from bf27a4f to 7e568d9 Compare March 31, 2025 22:57
@HollowMan6
Copy link
Contributor Author

Added print page support for the newly added footnote back references feature #2626

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

Signed-off-by: Hollow Man <hollowman@opensuse.org>
Let all the anchors id on the print page to have a path id prefix to
help locate.

e.g. bar/foo.md#abc -> #bar-foo-abc

Also append a dummy div to the start of the original page to make sure
that original page links without an anchor can also be located.

Fix to remove all the `./` in the normalized path id so that for
"./foo/bar.html#abc" we still get "#foo-bar-abc"

Add support for redirect link anchors in print page so that anchors can
also be redirected, also handle URL redirect links on print page

Handle all the elements id to add a path prefix, also make path id to
all be the lower case

Fix for print page footnote links by adding the path id prefix

Signed-off-by: Hollow Man <hollowman@opensuse.org>
@ehuss
Copy link
Contributor

ehuss commented Sep 17, 2025

I appreciate your patience here. In #2844 we've taken a different approach to how HTML is processed. As part of that process, I have updated the print page to now have all anchor links. If you'd like, it would be helpful if you can give it a try and let me know how it works for you.

@rustbot
Copy link
Collaborator

rustbot commented Sep 17, 2025

☔ The latest upstream changes (possibly 1c034bd) made this pull request unmergeable. Please resolve the merge conflicts.

@HollowMan6
Copy link
Contributor Author

Hi @ehuss! Thank you for supporting this and relieving my pain from resolving the conflicts. I tested several books with the mdbook-pdf CI/CD, and I think your changes look good, though they break the code at my side for generating the ToC from mdbook for PDFs, as it relies on the path ID prefix to locate anchors in print.html, and I can't come up with a better solution for supporting it, so probably I will mark this feature as deprecated and continue with the wkhtmltopdf style ToC.

I'll close this now and revert my changes back to v0.4.52 for backward compatibility.

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.

Convert all the links in the generated print.html for linking inside the book into URL fragment form

9 participants