From 15c93b56ed91b9bdeba64a2af34674f4f170e8ec Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 8 Jul 2025 15:37:46 -0700 Subject: [PATCH] Add support for fragment redirects This adds the ability to redirect URLs with `#` fragments. This is useful when section headers get renamed or moved to other pages. This works both for deleted pages and existing pages. The implementation requires the use of JavaScript in order to manipulate the location. (Ideally this would be handled on the server side.) This also makes it so that deleted page redirects preserve the fragment ID. Previously if you had a deleted page redirect, and the user went to something like `page.html#foo`, it would redirect to `bar.html` without the fragment. I think preserving the fragment is probably a better behavior. If the new page doesn't have the fragment ID, then no harm is really done. This is technically an open redirect, but I don't think that there is too much danger with preserving a fragment ID? --- guide/src/format/configuration/renderers.md | 10 +- src/front-end/templates/index.hbs | 15 +++ src/front-end/templates/redirect.hbs | 24 +++++ src/renderer/html_handlebars/hbs_renderer.rs | 96 ++++++++++++++++--- test_book/book.toml | 25 ++++- tests/gui/redirect.goml | 51 ++++++++++ tests/testsuite/redirects.rs | 31 ++++++ .../redirect_existing_page/book.toml | 5 + .../redirect_existing_page/src/SUMMARY.md | 3 + .../redirect_existing_page/src/chapter_1.md | 1 + .../book.toml | 5 + .../src/SUMMARY.md | 3 + .../src/chapter_1.md | 1 + .../redirects_are_emitted_correctly/book.toml | 1 + .../expected/nested/page.html | 24 +++++ .../expected/overview.html | 24 +++++ .../src/SUMMARY.md | 1 + .../src/chapter_2.md | 1 + 18 files changed, 306 insertions(+), 15 deletions(-) create mode 100644 tests/gui/redirect.goml create mode 100644 tests/testsuite/redirects/redirect_existing_page/book.toml create mode 100644 tests/testsuite/redirects/redirect_existing_page/src/SUMMARY.md create mode 100644 tests/testsuite/redirects/redirect_existing_page/src/chapter_1.md create mode 100644 tests/testsuite/redirects/redirect_removed_with_fragments_only/book.toml create mode 100644 tests/testsuite/redirects/redirect_removed_with_fragments_only/src/SUMMARY.md create mode 100644 tests/testsuite/redirects/redirect_removed_with_fragments_only/src/chapter_1.md create mode 100644 tests/testsuite/redirects/redirects_are_emitted_correctly/src/chapter_2.md diff --git a/guide/src/format/configuration/renderers.md b/guide/src/format/configuration/renderers.md index a827d2936f..7c1129e3af 100644 --- a/guide/src/format/configuration/renderers.md +++ b/guide/src/format/configuration/renderers.md @@ -310,13 +310,21 @@ This is useful when you move, rename, or remove a page to ensure that links to t [output.html.redirect] "/appendices/bibliography.html" = "https://rustc-dev-guide.rust-lang.org/appendix/bibliography.html" "/other-installation-methods.html" = "../infra/other-installation-methods.html" + +# Fragment redirects also work. +"/some-existing-page.html#old-fragment" = "some-existing-page.html#new-fragment" + +# Fragment redirects also work for deleted pages. +"/old-page.html" = "new-page.html" +"/old-page.html#old-fragment" = "new-page.html#new-fragment" ``` The table contains key-value pairs where the key is where the redirect file needs to be created, as an absolute path from the build directory, (e.g. `/appendices/bibliography.html`). The value can be any valid URI the browser should navigate to (e.g. `https://rust-lang.org/`, `/overview.html`, or `../bibliography.html`). This will generate an HTML page which will automatically redirect to the given location. -Note that the source location does not support `#` anchor redirects. + +When fragment redirects are specified, the page must use JavaScript to redirect to the correct location. This is useful if you rename or move a section header. Fragment redirects work with existing pages and deleted pages. ## Markdown Renderer diff --git a/src/front-end/templates/index.hbs b/src/front-end/templates/index.hbs index 139503260a..ae929655ed 100644 --- a/src/front-end/templates/index.hbs +++ b/src/front-end/templates/index.hbs @@ -341,6 +341,21 @@ {{/if}} {{/if}} + {{#if fragment_map}} + + {{/if}} + diff --git a/src/front-end/templates/redirect.hbs b/src/front-end/templates/redirect.hbs index d0532a5049..86f53bcdc7 100644 --- a/src/front-end/templates/redirect.hbs +++ b/src/front-end/templates/redirect.hbs @@ -8,5 +8,29 @@

Redirecting to... {{url}}.

+ + diff --git a/src/renderer/html_handlebars/hbs_renderer.rs b/src/renderer/html_handlebars/hbs_renderer.rs index a144b32b57..f93aeacf2c 100644 --- a/src/renderer/html_handlebars/hbs_renderer.rs +++ b/src/renderer/html_handlebars/hbs_renderer.rs @@ -111,6 +111,14 @@ impl HtmlHandlebars { .insert("section".to_owned(), json!(section.to_string())); } + let redirects = collect_redirects_for_path(&filepath, &ctx.html_config.redirect)?; + if !redirects.is_empty() { + ctx.data.insert( + "fragment_map".to_owned(), + json!(serde_json::to_string(&redirects)?), + ); + } + // Render the handlebars template with the data debug!("Render template"); let rendered = ctx.handlebars.render("index", &ctx.data)?; @@ -266,15 +274,27 @@ impl HtmlHandlebars { } log::debug!("Emitting redirects"); + let redirects = combine_fragment_redirects(redirects); - for (original, new) in redirects { - log::debug!("Redirecting \"{}\" → \"{}\"", original, new); + for (original, (dest, fragment_map)) in redirects { // Note: all paths are relative to the build directory, so the // leading slash in an absolute path means nothing (and would mess // up `root.join(original)`). let original = original.trim_start_matches('/'); let filename = root.join(original); - self.emit_redirect(handlebars, &filename, new)?; + if filename.exists() { + // This redirect is handled by the in-page fragment mapper. + continue; + } + if dest.is_empty() { + bail!( + "redirect entry for `{original}` only has source paths with `#` fragments\n\ + There must be an entry without the `#` fragment to determine the default \ + destination." + ); + } + log::debug!("Redirecting \"{}\" → \"{}\"", original, dest); + self.emit_redirect(handlebars, &filename, &dest, &fragment_map)?; } Ok(()) @@ -285,23 +305,17 @@ impl HtmlHandlebars { handlebars: &Handlebars<'_>, original: &Path, destination: &str, + fragment_map: &BTreeMap, ) -> Result<()> { - if original.exists() { - // sanity check to avoid accidentally overwriting a real file. - let msg = format!( - "Not redirecting \"{}\" to \"{}\" because it already exists. Are you sure it needs to be redirected?", - original.display(), - destination, - ); - return Err(Error::msg(msg)); - } - if let Some(parent) = original.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("Unable to ensure \"{}\" exists", parent.display()))?; } + let js_map = serde_json::to_string(fragment_map)?; + let ctx = json!({ + "fragment_map": js_map, "url": destination, }); let f = File::create(original)?; @@ -934,6 +948,62 @@ struct RenderItemContext<'a> { chapter_titles: &'a HashMap, } +/// Redirect mapping. +/// +/// The key is the source path (like `foo/bar.html`). The value is a tuple +/// `(destination_path, fragment_map)`. The `destination_path` is the page to +/// redirect to. `fragment_map` is the map of fragments that override the +/// destination. For example, a fragment `#foo` could redirect to any other +/// page or site. +type CombinedRedirects = BTreeMap)>; +fn combine_fragment_redirects(redirects: &HashMap) -> CombinedRedirects { + let mut combined: CombinedRedirects = BTreeMap::new(); + // This needs to extract the fragments to generate the fragment map. + for (original, new) in redirects { + if let Some((source_path, source_fragment)) = original.rsplit_once('#') { + let e = combined.entry(source_path.to_string()).or_default(); + if let Some(old) = e.1.insert(format!("#{source_fragment}"), new.clone()) { + log::error!( + "internal error: found duplicate fragment redirect \ + {old} for {source_path}#{source_fragment}" + ); + } + } else { + let e = combined.entry(original.to_string()).or_default(); + e.0 = new.clone(); + } + } + combined +} + +/// Collects fragment redirects for an existing page. +/// +/// The returned map has keys like `#foo` and the value is the new destination +/// path or URL. +fn collect_redirects_for_path( + path: &Path, + redirects: &HashMap, +) -> Result> { + let path = format!("/{}", path.display().to_string().replace('\\', "/")); + if redirects.contains_key(&path) { + bail!( + "redirect found for existing chapter at `{path}`\n\ + Either delete the redirect or remove the chapter." + ); + } + + let key_prefix = format!("{path}#"); + let map = redirects + .iter() + .filter_map(|(source, dest)| { + source + .strip_prefix(&key_prefix) + .map(|fragment| (format!("#{fragment}"), dest.to_string())) + }) + .collect(); + Ok(map) +} + #[cfg(test)] mod tests { use crate::config::TextDirection; diff --git a/test_book/book.toml b/test_book/book.toml index c89a3e51c4..854b5e7d3e 100644 --- a/test_book/book.toml +++ b/test_book/book.toml @@ -25,4 +25,27 @@ expand = true heading-split-level = 2 [output.html.redirect] -"/format/config.html" = "configuration/index.html" +"/format/config.html" = "../prefix.html" + +# This is a source without a fragment, and one with a fragment that goes to +# the same place. The redirect with the fragment is not necessary, since that +# is the default behavior. +"/pointless-fragment.html" = "prefix.html" +"/pointless-fragment.html#foo" = "prefix.html#foo" + +"/rename-page-and-fragment.html" = "prefix.html" +"/rename-page-and-fragment.html#orig" = "prefix.html#new" + +"/rename-page-fragment-elsewhere.html" = "prefix.html" +"/rename-page-fragment-elsewhere.html#orig" = "suffix.html#new" + +# Rename fragment on an existing page. +"/prefix.html#orig" = "prefix.html#new" +# Rename fragment on an existing page to another page. +"/prefix.html#orig-new-page" = "suffix.html#new" + +"/full-url-with-fragment.html" = "https://www.rust-lang.org/#fragment" + +"/full-url-with-fragment-map.html" = "https://www.rust-lang.org/" +"/full-url-with-fragment-map.html#a" = "https://www.rust-lang.org/#new1" +"/full-url-with-fragment-map.html#b" = "https://www.rust-lang.org/#new2" diff --git a/tests/gui/redirect.goml b/tests/gui/redirect.goml new file mode 100644 index 0000000000..26572354a8 --- /dev/null +++ b/tests/gui/redirect.goml @@ -0,0 +1,51 @@ +go-to: |DOC_PATH| + "format/config.html" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html"}) + +// Check that it preserves fragments when redirecting. +go-to: |DOC_PATH| + "format/config.html#fragment" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html#fragment"}) + +// The fragment one here isn't necessary, but should still work. +go-to: |DOC_PATH| + "pointless-fragment.html" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html"}) +go-to: |DOC_PATH| + "pointless-fragment.html#foo" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html#foo"}) + +// Page rename, and a fragment rename. +go-to: |DOC_PATH| + "rename-page-and-fragment.html" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html"}) +go-to: |DOC_PATH| + "rename-page-and-fragment.html#orig" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html#new"}) + +// Page rename, and the fragment goes to a *different* page from the default. +go-to: |DOC_PATH| + "rename-page-fragment-elsewhere.html" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html"}) +go-to: |DOC_PATH| + "rename-page-fragment-elsewhere.html#orig" +assert-window-property: ({"location": |DOC_PATH| + "suffix.html#new"}) + +// Goes to an external site. +go-to: |DOC_PATH| + "full-url-with-fragment.html" +assert-window-property: ({"location": "https://www.rust-lang.org/#fragment"}) + +// External site with fragment renames. +go-to: |DOC_PATH| + "full-url-with-fragment-map.html#a" +assert-window-property: ({"location": "https://www.rust-lang.org/#new1"}) +go-to: |DOC_PATH| + "full-url-with-fragment-map.html#b" +assert-window-property: ({"location": "https://www.rust-lang.org/#new2"}) + +// Rename fragment on an existing page. +go-to: |DOC_PATH| + "prefix.html#orig" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html#new"}) + +// Other fragments aren't affected. +go-to: |DOC_PATH| + "index.html" // Reset page since redirects are processed on load. +go-to: |DOC_PATH| + "prefix.html" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html"}) +go-to: |DOC_PATH| + "index.html" // Reset page since redirects are processed on load. +go-to: |DOC_PATH| + "prefix.html#dont-change" +assert-window-property: ({"location": |DOC_PATH| + "prefix.html#dont-change"}) + +// Rename fragment on an existing page to another page. +go-to: |DOC_PATH| + "index.html" // Reset page since redirects are processed on load. +go-to: |DOC_PATH| + "prefix.html#orig-new-page" +assert-window-property: ({"location": |DOC_PATH| + "suffix.html#new"}) diff --git a/tests/testsuite/redirects.rs b/tests/testsuite/redirects.rs index 3501c2236a..d92bf766bf 100644 --- a/tests/testsuite/redirects.rs +++ b/tests/testsuite/redirects.rs @@ -16,3 +16,34 @@ fn redirects_are_emitted_correctly() { file!["redirects/redirects_are_emitted_correctly/expected/nested/page.html"], ); } + +// Invalid redirect with only fragments. +#[test] +fn redirect_removed_with_fragments_only() { + BookTest::from_dir("redirects/redirect_removed_with_fragments_only").run("build", |cmd| { + cmd.expect_failure().expect_stderr(str![[r#" +[TIMESTAMP] [INFO] (mdbook::book): Book building has started +[TIMESTAMP] [INFO] (mdbook::book): Running the html backend +[TIMESTAMP] [ERROR] (mdbook::utils): Error: Rendering failed +[TIMESTAMP] [ERROR] (mdbook::utils): [TAB]Caused By: Unable to emit redirects +[TIMESTAMP] [ERROR] (mdbook::utils): [TAB]Caused By: redirect entry for `old-file.html` only has source paths with `#` fragments +There must be an entry without the `#` fragment to determine the default destination. + +"#]]); + }); +} + +// Invalid redirect for an existing page. +#[test] +fn redirect_existing_page() { + BookTest::from_dir("redirects/redirect_existing_page").run("build", |cmd| { + cmd.expect_failure().expect_stderr(str![[r#" +[TIMESTAMP] [INFO] (mdbook::book): Book building has started +[TIMESTAMP] [INFO] (mdbook::book): Running the html backend +[TIMESTAMP] [ERROR] (mdbook::utils): Error: Rendering failed +[TIMESTAMP] [ERROR] (mdbook::utils): [TAB]Caused By: redirect found for existing chapter at `/chapter_1.html` +Either delete the redirect or remove the chapter. + +"#]]); + }); +} diff --git a/tests/testsuite/redirects/redirect_existing_page/book.toml b/tests/testsuite/redirects/redirect_existing_page/book.toml new file mode 100644 index 0000000000..129ddb9189 --- /dev/null +++ b/tests/testsuite/redirects/redirect_existing_page/book.toml @@ -0,0 +1,5 @@ +[book] +title = "redirect_existing_page" + +[output.html.redirect] +"/chapter_1.html" = "other-page.html" diff --git a/tests/testsuite/redirects/redirect_existing_page/src/SUMMARY.md b/tests/testsuite/redirects/redirect_existing_page/src/SUMMARY.md new file mode 100644 index 0000000000..7390c82896 --- /dev/null +++ b/tests/testsuite/redirects/redirect_existing_page/src/SUMMARY.md @@ -0,0 +1,3 @@ +# Summary + +- [Chapter 1](./chapter_1.md) diff --git a/tests/testsuite/redirects/redirect_existing_page/src/chapter_1.md b/tests/testsuite/redirects/redirect_existing_page/src/chapter_1.md new file mode 100644 index 0000000000..b743fda354 --- /dev/null +++ b/tests/testsuite/redirects/redirect_existing_page/src/chapter_1.md @@ -0,0 +1 @@ +# Chapter 1 diff --git a/tests/testsuite/redirects/redirect_removed_with_fragments_only/book.toml b/tests/testsuite/redirects/redirect_removed_with_fragments_only/book.toml new file mode 100644 index 0000000000..2b51fba70f --- /dev/null +++ b/tests/testsuite/redirects/redirect_removed_with_fragments_only/book.toml @@ -0,0 +1,5 @@ +[book] +title = "redirect_removed_with_fragments_only" + +[output.html.redirect] +"/old-file.html#foo" = "chapter_1.html" diff --git a/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/SUMMARY.md b/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/SUMMARY.md new file mode 100644 index 0000000000..7390c82896 --- /dev/null +++ b/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/SUMMARY.md @@ -0,0 +1,3 @@ +# Summary + +- [Chapter 1](./chapter_1.md) diff --git a/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/chapter_1.md b/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/chapter_1.md new file mode 100644 index 0000000000..b743fda354 --- /dev/null +++ b/tests/testsuite/redirects/redirect_removed_with_fragments_only/src/chapter_1.md @@ -0,0 +1 @@ +# Chapter 1 diff --git a/tests/testsuite/redirects/redirects_are_emitted_correctly/book.toml b/tests/testsuite/redirects/redirects_are_emitted_correctly/book.toml index 46e4b8652e..47c603bde5 100644 --- a/tests/testsuite/redirects/redirects_are_emitted_correctly/book.toml +++ b/tests/testsuite/redirects/redirects_are_emitted_correctly/book.toml @@ -3,4 +3,5 @@ title = "redirects_are_emitted_correctly" [output.html.redirect] "/overview.html" = "index.html" +"/overview.html#old" = "index.html#new" "/nested/page.html" = "https://rust-lang.org/" diff --git a/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/nested/page.html b/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/nested/page.html index 08d6e92957..a5e2479670 100644 --- a/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/nested/page.html +++ b/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/nested/page.html @@ -8,5 +8,29 @@

Redirecting to... https://rust-lang.org/.

+ + diff --git a/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/overview.html b/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/overview.html index 8032308497..006e75f746 100644 --- a/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/overview.html +++ b/tests/testsuite/redirects/redirects_are_emitted_correctly/expected/overview.html @@ -8,5 +8,29 @@

Redirecting to... index.html.

+ + diff --git a/tests/testsuite/redirects/redirects_are_emitted_correctly/src/SUMMARY.md b/tests/testsuite/redirects/redirects_are_emitted_correctly/src/SUMMARY.md index 4f68394610..a5d68ad232 100644 --- a/tests/testsuite/redirects/redirects_are_emitted_correctly/src/SUMMARY.md +++ b/tests/testsuite/redirects/redirects_are_emitted_correctly/src/SUMMARY.md @@ -1,3 +1,4 @@ # Redirects - [Chapter 1](chapter_1.md) +- [Chapter 2](chapter_2.md) diff --git a/tests/testsuite/redirects/redirects_are_emitted_correctly/src/chapter_2.md b/tests/testsuite/redirects/redirects_are_emitted_correctly/src/chapter_2.md new file mode 100644 index 0000000000..7ebb5961e3 --- /dev/null +++ b/tests/testsuite/redirects/redirects_are_emitted_correctly/src/chapter_2.md @@ -0,0 +1 @@ +# Chapter 2