From 4cfd9176b885d9b5d1a9744b469e94a934571188 Mon Sep 17 00:00:00 2001 From: Mike Yumatov Date: Tue, 23 Jun 2026 13:09:55 +0300 Subject: [PATCH] fix(renderer): resolve relative .md links against a leaf page's parent dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relative `.md` links from a leaf page resolved one directory too deep — `[x](./other.md)` in `docs/specs/notif.md` produced `/specs/notif/other` instead of the sibling `/specs/other` — because the renderer treated every page's clean URL as its containing directory. That is correct only for directory-index pages (`index.md`, the README homepage); a leaf page (`name.md`) resolves relative links against its parent, per CommonMark. Carry an `is_dir` flag from the scanner through Document -> Page -> RenderConfig (true for a directory URL: index.md/README/virtual; false for a leaf file). Apply it as a one-time base-path correction at the link seam: for a leaf page, drop the page's own URL slug before resolving relative links. The RenderBackend trait and the resolver are untouched, and README/`docs/`-prefix homepage links are unchanged. `is_dir` is serde default-true on the serialized Document (S3 bundle) and Page (structure cache) so data written before the field existed deserializes to the prior behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + crates/rw-renderer/src/config.rs | 6 +++ crates/rw-renderer/src/link.rs | 54 +++++++++++++++++++ crates/rw-renderer/src/renderer.rs | 84 ++++++++++++++++++++++++++++++ crates/rw-renderer/src/walker.rs | 3 +- crates/rw-site/src/page.rs | 35 +++++++++++-- crates/rw-site/src/site.rs | 29 +++++++++++ crates/rw-site/src/site_state.rs | 2 + crates/rw-storage-fs/src/lib.rs | 30 +++++++++++ crates/rw-storage-s3/src/format.rs | 5 ++ crates/rw-storage/src/mock.rs | 8 +++ crates/rw-storage/src/storage.rs | 29 +++++++++++ 12 files changed, 282 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d3e6d7e..80aa11f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Relative `.md` links from a leaf page (e.g. `[sibling](./other.md)` in `docs/specs/notif.md`) now resolve to the sibling page (`/specs/other`) instead of a non-existent path nested under the current page (`/specs/notif/other`). Links now follow standard CommonMark semantics — resolved relative to the source file's directory — for both leaf pages and `index.md` directory pages. Links from README/`index.md` homepages (including the `docs/` source-dir prefix case) are unchanged. - Opening an inline-comment deep link (`#comment-`) no longer leaves the comment thread pinned in the wrong vertical position. The thread in the right-margin column (and the narrow-screen comment popover) could land hundreds of pixels above its highlighted passage and stay there when content above the passage reflowed *after* the thread was positioned — e.g. a web-font swap on first load, or a late-loading image or diagram. Threads now re-align whenever their highlighted passage moves, not only when the article is resized, so they track the highlight through any late layout shift. A normal click was never affected (it happens after the page has settled). ## [0.1.27] - 2026-06-22 diff --git a/crates/rw-renderer/src/config.rs b/crates/rw-renderer/src/config.rs index f639f8d5..1a1e0c74 100644 --- a/crates/rw-renderer/src/config.rs +++ b/crates/rw-renderer/src/config.rs @@ -66,6 +66,11 @@ pub(crate) struct RenderConfig { /// Origin prefix (with trailing slash) for files outside `source_dir`. /// Set by [`with_origin`](crate::MarkdownRenderer::with_origin). pub(crate) origin_prefix: Option, + /// True when the current page's URL denotes a directory (`index.md` or the + /// root/README homepage) rather than a single file (a leaf `name.md`). Leaf + /// pages resolve relative links against their *containing directory*, so the + /// page's own URL slug is dropped from the link base. Defaults to `true`. + pub(crate) is_dir: bool, /// `[[wikilink]]` parsing enabled. pub(crate) wikilinks: bool, /// Extract title from first H1. @@ -82,6 +87,7 @@ impl RenderConfig { Self { base_path: None, origin_prefix: None, + is_dir: true, wikilinks: false, extract_title: false, sections: None, diff --git a/crates/rw-renderer/src/link.rs b/crates/rw-renderer/src/link.rs index 16146bd5..81316aa4 100644 --- a/crates/rw-renderer/src/link.rs +++ b/crates/rw-renderer/src/link.rs @@ -23,6 +23,30 @@ pub(crate) fn strip_origin<'a>(cfg: &RenderConfig, url: &'a str) -> Cow<'a, str> Cow::Borrowed(url) } +/// The base path for resolving relative links, corrected for the page's source +/// shape. +/// +/// Directory pages (`index.md`, the root/README homepage) resolve relative +/// links against their own URL. Leaf pages (`name.md`) resolve against their +/// *containing directory*, so the page's own final URL segment is dropped — +/// matching `CommonMark`, where `./sibling.md` is a sibling of the source file. +/// +/// Kept separate from [`RenderConfig::base_path`], which wikilink resolution +/// reads unchanged; only plain-link resolution uses this corrected base. +/// `/specs/notif` (leaf) -> `/specs`; `/guide` (leaf) -> `/`. +pub(crate) fn link_base(cfg: &RenderConfig) -> Option<&str> { + let base = cfg.base_path.as_deref()?; + if cfg.is_dir { + return Some(base); + } + // Drop the page's own final URL segment, keeping the leading slash, so the + // result is the containing directory. A leaf at the root collapses to `/`. + match base.trim_end_matches('/').rsplit_once('/') { + Some((dir, _)) if !dir.is_empty() => Some(dir), + _ => Some("/"), + } +} + /// Build ref data attributes for a resolved path, if applicable. /// /// Returns `None` for: @@ -75,6 +99,36 @@ mod tests { assert_matches!(result, Cow::Borrowed(_)); } + #[test] + fn link_base_is_dir_returns_base_unchanged() { + let mut c = cfg(); + c.base_path = Some("/specs/notif".to_owned()); + c.is_dir = true; + assert_eq!(link_base(&c), Some("/specs/notif")); + } + + #[test] + fn link_base_leaf_drops_last_segment() { + let mut c = cfg(); + c.base_path = Some("/specs/notif".to_owned()); + c.is_dir = false; + assert_eq!(link_base(&c), Some("/specs")); + } + + #[test] + fn link_base_leaf_at_root_drops_to_root() { + let mut c = cfg(); + c.base_path = Some("/guide".to_owned()); + c.is_dir = false; + assert_eq!(link_base(&c), Some("/")); + } + + #[test] + fn link_base_none_when_unset() { + let c = cfg(); + assert_eq!(link_base(&c), None); + } + #[test] fn section_ref_attrs_non_internal_returns_none() { let c = cfg(); diff --git a/crates/rw-renderer/src/renderer.rs b/crates/rw-renderer/src/renderer.rs index 7206d614..688eda1e 100644 --- a/crates/rw-renderer/src/renderer.rs +++ b/crates/rw-renderer/src/renderer.rs @@ -116,6 +116,21 @@ impl MarkdownRenderer { self } + /// Set whether the current page's URL denotes a directory (`true`, from + /// `index.md` or the root/README homepage) rather than a single file + /// (`false`, a leaf `name.md`). Defaults to `true`. + /// + /// A leaf page resolves relative links against its *containing directory* + /// (`CommonMark` semantics): `./sibling.md` is a sibling of the source file, + /// not a child of it. Setting this `false` drops the page's own URL slug + /// from the link base. Only affects the HTML backend's relative-link + /// resolution; wikilink resolution is unaffected. + #[must_use] + pub fn with_is_dir(mut self, is_dir: bool) -> Self { + self.config.is_dir = is_dir; + self + } + /// Set the origin (source directory name) for files outside `source_dir`. /// /// When set, relative links starting with this prefix (e.g., `docs/guide.md`) @@ -583,6 +598,75 @@ mod tests { assert!(result.html.contains(r#"href="https://example.com""#)); } + fn render_leaf(markdown: &str, base_path: &str) -> RenderResult { + MarkdownRenderer::::new() + .with_base_path(base_path) + .with_is_dir(false) + .render(markdown, Pipeline::new()) + } + + #[test] + fn test_leaf_sibling_link_resolves_against_parent_dir() { + let result = render_leaf("[x](./inbox.md)", "/specs/notif"); + assert!( + result.html.contains(r#"href="/specs/inbox""#), + "Expected href=\"/specs/inbox\", got: {}", + result.html + ); + } + + #[test] + fn test_leaf_parent_link() { + let result = render_leaf("[x](../x.md)", "/specs/notif"); + assert!( + result.html.contains(r#"href="/x""#), + "Expected href=\"/x\", got: {}", + result.html + ); + } + + #[test] + fn test_leaf_subdir_link() { + let result = render_leaf("[x](sub/y.md)", "/specs/notif"); + assert!( + result.html.contains(r#"href="/specs/sub/y""#), + "Expected href=\"/specs/sub/y\", got: {}", + result.html + ); + } + + #[test] + fn test_leaf_link_with_fragment() { + let result = render_leaf("[x](./page.md#frag)", "/specs/notif"); + assert!( + result.html.contains(r#"href="/specs/page#frag""#), + "Expected href=\"/specs/page#frag\", got: {}", + result.html + ); + } + + #[test] + fn test_leaf_root_level_sibling() { + // Leaf `guide.md` at docs root (URL `/guide`); sibling lives at root. + let result = render_leaf("[x](./sibling.md)", "/guide"); + assert!( + result.html.contains(r#"href="/sibling""#), + "Expected href=\"/sibling\", got: {}", + result.html + ); + } + + #[test] + fn test_index_sibling_link_unchanged() { + // Default (is_dir = true): base is the page's own dir. + let result = render_with_base_path("[x](./inbox.md)", "/specs/notif"); + assert!( + result.html.contains(r#"href="/specs/notif/inbox""#), + "Expected href=\"/specs/notif/inbox\", got: {}", + result.html + ); + } + #[test] fn test_duplicate_heading_ids() { let result = render_html("## FAQ\n\n## FAQ\n\n## FAQ"); diff --git a/crates/rw-renderer/src/walker.rs b/crates/rw-renderer/src/walker.rs index 7283de09..64c4efff 100644 --- a/crates/rw-renderer/src/walker.rs +++ b/crates/rw-renderer/src/walker.rs @@ -557,7 +557,8 @@ impl<'r, B: RenderBackend> Walker<'r, B> { } Tag::Link { dest_url, .. } => { let dest_url = link::strip_origin(self.cfg, &dest_url); - let href = B::transform_link(&dest_url, self.cfg.base_path.as_deref()); + let base = link::link_base(self.cfg); + let href = B::transform_link(&dest_url, base); let section_ref = link::section_ref_attrs(self.cfg, &href); let section_attrs = section_ref.as_ref().map(|(r, p)| (r.as_str(), p.as_str())); self.with_markup_buffer(|out| B::link_start(&href, section_attrs, out)); diff --git a/crates/rw-site/src/page.rs b/crates/rw-site/src/page.rs index 7b8335ee..a894bb5b 100644 --- a/crates/rw-site/src/page.rs +++ b/crates/rw-site/src/page.rs @@ -113,6 +113,23 @@ pub struct Page { /// Ordered list of child page slugs for navigation ordering. #[serde(default, skip_serializing_if = "Option::is_none")] pub pages: Option>, + /// Whether this page's content is backed by a directory index (`index.md` + /// or the root/README homepage) rather than a leaf `name.md`. Controls how + /// the renderer resolves relative `.md` links (see + /// [`Document::is_dir`](rw_storage::Document)). + /// + /// Defaults to `true` when absent, for backward compatibility — see + /// [`default_is_dir`]. + #[serde(default = "default_is_dir")] + pub is_dir: bool, +} + +/// Serde default for [`Page::is_dir`]. `Page` is persisted in the +/// structure cache (`CachedSiteState`), so an entry cached before this field +/// existed must still deserialize; such a site had no leaf pages, so +/// directory-style resolution preserves the links it was cached with. +fn default_is_dir() -> bool { + true } /// One segment of the breadcrumb trail leading to a page. @@ -319,7 +336,7 @@ impl PageRenderer { } let markdown_text = self.storage.read(path)?; - let renderer = self.create_renderer(path, page.origin.as_deref(), ctx); + let renderer = self.create_renderer(path, page.origin.as_deref(), page.is_dir, ctx); let pipeline = self.create_pipeline(ctx); let result = renderer.render(&markdown_text, pipeline); @@ -413,10 +430,12 @@ impl PageRenderer { &self, base_path: &str, origin: Option<&str>, + is_dir: bool, ctx: &RenderContext, ) -> MarkdownRenderer { - let mut renderer = - MarkdownRenderer::::new().with_base_path(format!("/{base_path}")); + let mut renderer = MarkdownRenderer::::new() + .with_base_path(format!("/{base_path}")) + .with_is_dir(is_dir); if let Some(origin) = origin { renderer = renderer.with_origin(origin); @@ -537,9 +556,19 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, } } + #[test] + fn page_is_dir_defaults_true_when_absent() { + // A structure-cache entry (CachedSiteState) written before `is_dir` + // existed has no such key; it must still deserialize, defaulting to true. + let json = r#"{"title":"Guide","path":"guide","has_content":true}"#; + let page: Page = serde_json::from_str(json).unwrap(); + assert!(page.is_dir); + } + #[test] fn test_render_page_returns_html() { let storage = MockStorage::new() diff --git a/crates/rw-site/src/site.rs b/crates/rw-site/src/site.rs index 4ca1a00a..9fd49286 100644 --- a/crates/rw-site/src/site.rs +++ b/crates/rw-site/src/site.rs @@ -561,6 +561,7 @@ impl Site { description: doc.description.clone(), origin: doc.origin.clone(), pages: doc.pages.clone(), + is_dir: doc.is_dir, }, parent_idx, doc.page_kind.as_deref(), @@ -1687,4 +1688,32 @@ mod tests { assert_eq!(nav.items[1].path, "configuration"); assert_eq!(nav.items[2].path, "advanced"); // unlisted, alphabetical } + + #[test] + fn test_leaf_page_relative_link_resolves_to_sibling() { + use rw_storage_fs::FsStorage; + + let temp = tempfile::tempdir().unwrap(); + let docs = temp.path().join("docs"); + let specs = docs.join("specs"); + std::fs::create_dir_all(&specs).unwrap(); + // Two sibling leaf pages: docs/specs/notif.md and docs/specs/inbox.md. + std::fs::write(specs.join("notif.md"), "# Notif\n\n[inbox](./inbox.md)").unwrap(); + std::fs::write(specs.join("inbox.md"), "# Inbox").unwrap(); + + let storage = FsStorage::new(docs); + let config = PageRendererConfig { + extract_title: true, + ..Default::default() + }; + let site = Site::new(Arc::new(storage), Arc::new(rw_cache::NullCache), config); + + let result = site.render("specs/notif").unwrap(); + + assert!( + result.html.contains(r#"href="/specs/inbox""#), + "leaf sibling link should resolve to /specs/inbox, got: {}", + result.html + ); + } } diff --git a/crates/rw-site/src/site_state.rs b/crates/rw-site/src/site_state.rs index e155af7d..2e17308d 100644 --- a/crates/rw-site/src/site_state.rs +++ b/crates/rw-site/src/site_state.rs @@ -1209,6 +1209,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; assert_eq!(page.title, "Guide"); @@ -3224,6 +3225,7 @@ mod tests { description: desc.map(str::to_owned), origin: None, pages: None, + is_dir: true, } } diff --git a/crates/rw-storage-fs/src/lib.rs b/crates/rw-storage-fs/src/lib.rs index c74ec49d..a6b799a8 100644 --- a/crates/rw-storage-fs/src/lib.rs +++ b/crates/rw-storage-fs/src/lib.rs @@ -301,6 +301,7 @@ impl FsStorage { description: meta.description, origin: None, pages: meta.pages, + is_dir: name_lower == "index.md", })) } else if let Some(meta_path) = &doc_ref.meta_path { let Ok(meta_yaml) = fs::read_to_string(meta_path) else { @@ -328,6 +329,7 @@ impl FsStorage { description: meta.description, origin: None, pages: meta.pages, + is_dir: true, })) } else { Ok(None) @@ -679,6 +681,7 @@ impl Storage for FsStorage { description: None, origin, pages: None, + is_dir: true, }); } @@ -2105,9 +2108,13 @@ mod tests { let home = docs.iter().find(|d| d.path.is_empty()).unwrap(); assert_eq!(home.origin, Some("docs".to_owned())); + // The README homepage is the root directory page, so its relative links + // resolve against the root (not nested under a leaf slug). + assert!(home.is_dir, "README homepage URL is a directory"); let guide = docs.iter().find(|d| d.path == "guide").unwrap(); assert_eq!(guide.origin, None); + assert!(!guide.is_dir, "docs/guide.md is a leaf page"); } #[test] @@ -2544,4 +2551,27 @@ mod tests { "missing 'docs/guide' in {got:?}" ); } + + #[test] + fn test_scan_classifies_is_dir() { + let temp_dir = create_test_dir(); + // Leaf page: docs/guide.md -> URL "guide", is_dir = false. + fs::write(temp_dir.path().join("guide.md"), "# Guide").unwrap(); + // Index page: docs/domain/index.md -> URL "domain", is_dir = true. + let domain = temp_dir.path().join("domain"); + fs::create_dir(&domain).unwrap(); + fs::write(domain.join("index.md"), "# Domain").unwrap(); + + let storage = FsStorage::new(temp_dir.path().to_path_buf()); + let docs = storage.scan().unwrap(); + + let guide = docs.iter().find(|d| d.path == "guide").unwrap(); + assert!( + !guide.is_dir, + "leaf guide.md URL is a file, not a directory" + ); + + let domain = docs.iter().find(|d| d.path == "domain").unwrap(); + assert!(domain.is_dir, "domain/index.md URL is a directory"); + } } diff --git a/crates/rw-storage-s3/src/format.rs b/crates/rw-storage-s3/src/format.rs index 7397aa99..7f2499c5 100644 --- a/crates/rw-storage-s3/src/format.rs +++ b/crates/rw-storage-s3/src/format.rs @@ -85,6 +85,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }, Document { path: "guide".to_owned(), @@ -95,6 +96,7 @@ mod tests { description: Some("Getting started".to_owned()), origin: None, pages: None, + is_dir: true, }, ]); @@ -152,6 +154,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; let json = serde_json::to_string(&doc).unwrap(); @@ -197,6 +200,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }]); manifest.mtimes.insert("guide".to_owned(), 1_713_000_000.0); @@ -229,6 +233,7 @@ mod tests { "getting-started".to_owned(), "configuration".to_owned(), ]), + is_dir: true, }]); let json = serde_json::to_string(&manifest).unwrap(); diff --git a/crates/rw-storage/src/mock.rs b/crates/rw-storage/src/mock.rs index c2546d82..de2a1947 100644 --- a/crates/rw-storage/src/mock.rs +++ b/crates/rw-storage/src/mock.rs @@ -109,6 +109,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self } @@ -132,6 +133,7 @@ impl MockStorage { description: None, origin: None, pages: Some(pages), + is_dir: true, }); self } @@ -155,6 +157,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self } @@ -179,6 +182,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self } @@ -197,6 +201,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self } @@ -218,6 +223,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self } @@ -249,6 +255,7 @@ impl MockStorage { description: None, origin: None, pages: None, + is_dir: true, }); self.contents.write().insert(path, content.into()); self @@ -392,6 +399,7 @@ impl Storage for MockStorage { description: d.description.clone(), origin: d.origin.clone(), pages: d.pages.clone(), + is_dir: d.is_dir, }) .collect()) } diff --git a/crates/rw-storage/src/storage.rs b/crates/rw-storage/src/storage.rs index b4e7384e..731982c1 100644 --- a/crates/rw-storage/src/storage.rs +++ b/crates/rw-storage/src/storage.rs @@ -18,6 +18,13 @@ use std::path::PathBuf; use crate::event::{StorageEventReceiver, WatchHandle}; use crate::metadata::Metadata; +/// Serde default for [`Document::is_dir`]: a bundle published before this +/// field existed has no leaf pages, so treating its pages as directories +/// preserves the link resolution they were rendered with. +fn default_is_dir() -> bool { + true +} + /// Document metadata returned by storage scan. /// /// Documents can be either real pages (with content) or virtual pages (metadata only). @@ -58,6 +65,16 @@ pub struct Document { /// Ordered list of child page slugs for navigation ordering. #[serde(default, skip_serializing_if = "Option::is_none")] pub pages: Option>, + /// True when this page's URL denotes a directory — its content comes from a + /// directory index file (`index.md`, or the README homepage) — rather than a + /// single file (a leaf `name.md`). + /// + /// Leaf pages resolve relative links against their parent directory, so the + /// renderer drops the page's own URL slug from the link base. Defaults to + /// `true` for backward compatibility with S3 bundles published before this + /// field existed (preserving their directory-style resolution). + #[serde(default = "default_is_dir")] + pub is_dir: bool, } /// Semantic error categories (inspired by Object Store + `OpenDAL`). @@ -348,6 +365,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; assert_eq!(doc.path, ""); @@ -367,6 +385,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; assert_eq!(doc.path, "guide"); @@ -386,6 +405,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; assert_eq!(doc.path, "domain/billing"); @@ -404,6 +424,7 @@ mod tests { description: None, origin: None, pages: None, + is_dir: true, }; assert_eq!(doc.path, "domains"); @@ -560,4 +581,12 @@ mod tests { assert_eq!(StorageErrorKind::Timeout.to_string(), "Timeout"); assert_eq!(StorageErrorKind::Other.to_string(), "Error"); } + + #[test] + fn document_is_dir_defaults_true_when_absent() { + // An S3 bundle published before `is_dir` existed has no such key. + let json = r#"{"path":"guide","title":"Guide","has_content":true}"#; + let doc: Document = serde_json::from_str(json).unwrap(); + assert!(doc.is_dir); + } }