Skip to content

fix(templ): accept slug for subpage macros - #686

Draft
caugner wants to merge 4 commits into
mainfrom
normalize-subpage-macro-url-arg
Draft

fix(templ): accept slug for subpage macros#686
caugner wants to merge 4 commits into
mainfrom
normalize-subpage-macro-url-arg

Conversation

@caugner

@caugner caugner commented May 13, 2026

Copy link
Copy Markdown
Contributor

Description

Updates the {{QuickLinksWithSubpages}} and {{ListSubpagesForSidebar}} macros to accept a slug (without /{locale}/docs/ prefix), without triggering a flaw.

Motivation

Avoid unnecessary flaws (the slug in the context of a locale is not ambiguous).

Additional details

Previously, this triggered flaws like this:

fixed legacy url: Learn/Common_questions -> /es/docs/Learn/Common_questions

Related issues and pull requests

Part of mdn/fred#1462.

Related to mdn/translated-content#36029.

@github-actions

Copy link
Copy Markdown
Contributor

afcf314 was deployed to: https://rari-pr686.review.mdn.allizom.net/

@caugner
caugner marked this pull request as ready for review May 13, 2026 16:03
@caugner
caugner requested a review from a team as a code owner May 13, 2026 16:03
@caugner
caugner requested a review from LeoMcA May 13, 2026 16:03

@LeoMcA LeoMcA left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR feels in conflict with mdn/translated-content#36029 - I'm not sure why we need to "fix" behaviour in rari if we can fix it in translated content, especially because these macros aren't used at all in en-US

Comment on lines +70 to 76
fn prefix_check_is_locale_specific() {
// A `/fr/docs/...` URL processed under en-US is treated as a slug
// and gets re-prefixed.
assert_eq!(
fix_broken_legacy_url("/Web", Locale::EnUs),
"/en-US/docs/Web"
normalize_url("/fr/docs/Web", Locale::EnUs),
"/en-US/docs/fr/docs/Web"
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels like wrong behaviour

Comment on lines +9 to +17
if input.is_empty() {
return String::new();
}
let prefix = format!("/{}/docs", locale.as_url_str());
if input.starts_with(&prefix) {
input.to_string()
} else {
(trimmed.starts_with("docs/"), trimmed)
};
format!("{prefix}/{}", input.trim_start_matches('/'))
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's a number of helper functions in crates/rari-doc/src/resolve.rs which feel like they'd be useful here

@caugner
caugner marked this pull request as draft May 20, 2026 13:41
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.

3 participants