Merge #95 from 86-anchor-escape

Author: goerz

.github/workflows/ci.yml

@@ -82,6 +82,7 @@ jobs:
               Pkg.PackageSpec(name="Documenter", version="1.0.0"),
               Pkg.PackageSpec(name="MarkdownAST", version="0.1.2"),
               Pkg.PackageSpec(name="OrderedCollections", version="1.6.0"),
+              Pkg.PackageSpec(name="Bijections", version="0.1.4"),
           ])
           Pkg.precompile()
           Pkg.status()

NEWS.md

@@ -6,6 +6,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased][]
 
+### Added
+
+* The `CitationBibliography` plugin object now has an internal field `anchor_keys` that is a bijective mapping of citation keys to HTML anchor names. The anchor names are normalized versions of the citation keys that are restricted to ASCII alphanumerics, dashes (`-`) and underscores (`_`). This provides [compatibility with HTML4](https://www.w3.org/TR/html4/types.html#type-id) and additionally [avoids issues with CSS selectors](https://stackoverflow.com/a/79022). It also works around restrictions of the `Documenter.DOM` framework that is used internally to render HTML content. [[#95][]]
+
+
+### Fixed
+
+* Citation keys the contain special characters (like colons) no longer produce broken links. This is achieved by normalizing HTML anchor names to contain only alphanumeric ASCII characters, dashes, and underscores [[#86][], [#95][]]
+
 
 ## [Version 1.3.7][1.3.7] - 2025-03-29
 
@@ -198,8 +207,10 @@ There were several bugs and limitations in version `1.2.x` for which some existi
 [1.2.0]: https://github.com/JuliaDocs/DocumenterCitations.jl/compare/v1.1.0...v1.2.0
 [1.1.0]: https://github.com/JuliaDocs/DocumenterCitations.jl/compare/v1.0.0...v1.1.0
 [1.0.0]: https://github.com/JuliaDocs/DocumenterCitations.jl/compare/v0.2.12...v1.0.0
+[#95]: https://github.com/JuliaDocs/DocumenterCitations.jl/pull/95
 [#89]: https://github.com/JuliaDocs/DocumenterCitations.jl/pull/89
 [#87]: https://github.com/JuliaDocs/DocumenterCitations.jl/pull/87
+[#86]: https://github.com/JuliaDocs/DocumenterCitations.jl/issues/86
 [#83]: https://github.com/JuliaDocs/DocumenterCitations.jl/pull/83
 [#80]: https://github.com/JuliaDocs/DocumenterCitations.jl/issues/80
 [#79]: https://github.com/JuliaDocs/DocumenterCitations.jl/pull/79

Project.toml

@@ -6,6 +6,7 @@ version = "1.3.7+dev"
 [deps]
 AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"
 Bibliography = "f1be7e48-bf82-45af-a471-ae754a193061"
+Bijections = "e2ed5e7c-b2de-5872-ae92-c73ca462fb04"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
 Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
 Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
@@ -17,6 +18,7 @@ Unicode = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"
 [compat]
 AbstractTrees = "0.4"
 Bibliography = "0.2.15, 0.3"
+Bijections = "0.1.4"
 Dates = "1"
 Documenter = "1"
 Logging = "1"

src/DocumenterCitations.jl

@@ -9,6 +9,7 @@ using Documenter.Writers.HTMLWriter
 import MarkdownAST
 import AbstractTrees
 
+using Bijections: Bijections
 using Logging
 using Markdown
 using Bibliography: Bibliography, xyear, xlink, xtitle
@@ -48,6 +49,11 @@ should not be considered part of the stable API.
 * `anchor_map`: an [`AnchorMap`](https://documenter.juliadocs.org/stable/lib/internals/anchors/#Documenter.AnchorMap)
   object that keeps track of the link anchors for references in bibliography
   blocks
+* `anchor_keys`: a [bijective map](https://github.com/scheinerman/Bijections.jl?tab=readme-ov-file#bijections)
+  of citation keys to HTML anchor names. Whenever possible, an anchor name is
+  identical to the citation key, but anchor names are restricted to consist
+  only of ASCII letters, digits, and the symbols `-`, `_`. Thus, citation keys
+  are normalized to meet that restriction.
 """
 struct CitationBibliography <: Documenter.Plugin
 
@@ -72,6 +78,9 @@ struct CitationBibliography <: Documenter.Plugin
     # canonical bibliography blocks
     anchor_map::Documenter.AnchorMap
 
+    # Map citation key => anchor name
+    anchor_keys::Bijections.Bijection{String,String}
+
 end
 
 function CitationBibliography(bibfile::AbstractString=""; style=nothing)
@@ -117,13 +126,15 @@ function CitationBibliography(bibfile::AbstractString=""; style=nothing)
     citations = OrderedDict{String,Int64}()
     page_citations = Dict{String,Set{String}}()
     anchor_map = Documenter.AnchorMap()
+    anchor_keys = Bijections.Bijection{String,String}()
     return CitationBibliography(
         bibfile,
         style,
         entries,
         citations,
         page_citations,
-        anchor_map
+        anchor_map,
+        anchor_keys
     )
 end
 

src/expand_bibliography.jl

@@ -4,7 +4,7 @@ _ALLOW_PRE_13_FALLBACK = true
 
 Runs after [`CollectCitations`](@ref) but before [`ExpandCitations`](@ref).
 
-Each bibliography is rendered into HTML as a a [definition
+Each bibliography is rendered into HTML as a [definition
 list](https://www.w3schools.com/tags/tag_dl.asp), a [bullet
 list](https://www.w3schools.com/tags/tag_ul.asp), or an
 [enumeration](https://www.w3schools.com/tags/tag_ol.asp) depending on
@@ -362,18 +362,18 @@ function expand_bibliography(node::MarkdownAST.Node, meta, page, doc)
     end
     for (key, entry) in entries_to_show
         if fields[:Canonical]
-            anchor_key = key
+            anchor_key = get_anchor_key(key, bib.anchor_keys)
             # Add anchor that citations can link to from anywhere in the docs.
-            if Documenter.anchor_exists(anchors, key)
+            if Documenter.anchor_exists(anchors, anchor_key)
                 # Skip entries that already have a canonical bib entry
                 # elsewhere. This is expected behavior, not an error/warning,
                 # allowing to split the canonical bibliography in multiple
                 # parts.
                 @debug "Skipping key=$(key) (existing anchor)"
                 continue
             else
-                @debug "Defining anchor for key=$(key)"
-                Documenter.anchor_add!(anchors, entry, key, page.build)
+                @debug "Defining anchor $(repr(anchor_key)) for key=$(repr(key))"
+                Documenter.anchor_add!(anchors, entry, anchor_key, page.build)
             end
         else
             anchor_key = nothing
@@ -406,6 +406,78 @@ function expand_bibliography(node::MarkdownAST.Node, meta, page, doc)
 end
 
 
+# Generate a suitably normalized (restricted ASCII) HTML anchor name from a
+# citation key.
+#
+# The [HTML4 standard requires](https://www.w3.org/TR/html4/types.html#type-id)
+# that anchor names must begin with a letter ([A-Za-z]) and may be followed by
+# any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"),
+# colons (":"), and periods ("."). Dots and colons are further problematic for
+# compatibility with CSS selectors, see https://stackoverflow.com/a/79022.
+# Even more importantly, these characters are not supported by the
+# `Documenter.DOM` framework that we use to generate HTML: it will silently
+# drop anything after a colon or period. The numerical `suffix_index` gets
+# appended to the anchor name if > 0 and can be used to disambiguate labels.
+function get_anchor_key(
+    citation_key::String,
+    cache::Bijections.Bijection{String,String};
+    suffix_index::Int64=1
+)
+    if haskey(cache, citation_key)
+        anchor_key = cache[citation_key]
+    else
+        anchor_key = normalize_anchor(citation_key) # => [A-Za-z0-0_-]
+        if suffix_index > 1
+            anchor_key *= "-$suffix_index"
+        end
+        if !startswith(anchor_key, r"[A-Za-z]")
+            # Anchors must start with a letter. Instead of rejecting "invalid"
+            # anchors, we just prepend something arbitrary.
+            anchor_key = "cit-" * anchor_key
+        end
+        try
+            # The Bijection type takes care of all the work of checking for
+            # duplicates here.
+            cache[citation_key] = anchor_key
+        catch
+            suffix_index += 1
+            msg = "HTML anchor for citation key $(repr(citation_key)) normalizes to ambiguous $(repr(anchor_key)) conflicting with citation key $(repr(cache(anchor_key))). Disambiguating with suffix \"-$(suffix_index)\""
+            @warn(msg)
+            if suffix_index < 100
+                return get_anchor_key(citation_key, cache; suffix_index)
+            else
+                error("Internal error: cannot find disambiguated anchor key")
+            end
+        end
+        @debug "Generated anchor key $(repr(anchor_key)) for citation key $(repr(citation_key))"
+    end
+    return anchor_key
+end
+
+
+# Transform an arbitrary string `s` into a normalized string containing only
+# ASCII letters, numbers, and the symbols `_` and `-`, i.e., matching the
+# regex `r"^[A-Za-z0-9._-]+$"`. Letters with diacritics are normalized into
+# their ASCII equivalents, `:` and `/` are converted to `_`, and all other
+# characters are dropped.
+function normalize_anchor(s::AbstractString)
+    s_norm = Unicode.normalize(s, :NFKD)  # decompose diacritics
+    chars = Char[]
+    for c in s_norm
+        if ('A' <= c <= 'Z') ||
+           ('a' <= c <= 'z') ||
+           ('0' <= c <= '9') ||
+           c == '_' ||
+           c == '-'
+            push!(chars, c)
+        elseif (c == ':') || (c == '/') || (c == '.')
+            push!(chars, '_')
+        end
+    end
+    return String(chars)
+end
+
+
 # Deal with `@__FILE__` in `Pages`, convert it to the name of the current file.
 function _resolve__FILE__(Pages, page)
     __FILE__ = let ex = Meta.parse("_ = @__FILE__", 1; raise=false)[1]

src/expand_citations.jl

@@ -133,18 +133,19 @@ function expand_citation(
         @assert cit isa DirectCitationLink
         # E.g., "[Semi-AD paper](@cite GoerzQ2022)"
         key = cit.key
-        anchor = Documenter.anchor(anchors, key)
-        if isnothing(anchor)
-            link_text = ast_linktext(cit.node)
-            @error "expand_citation$rec: No destination for key=$(repr(key)) → unlinked text $(repr(link_text))"
-            return Documenter.mdparse(link_text; mode=:span)
-        else
+        if haskey(bib.anchor_keys, key)
+            anchor_key = bib.anchor_keys[key]
+            anchor = Documenter.anchor(anchors, anchor_key)
             expanded_node = MarkdownAST.copy_tree(node)
             path = relpath(anchor.file, dirname(page.build))
             expanded_node.element.destination =
                 string(path, Documenter.anchor_fragment(anchor))
             @debug "expand_citation$rec: $cit → link to $(expanded_node.element.destination)"
             return expanded_node
+        else
+            link_text = ast_linktext(cit.node)
+            @error "expand_citation$rec: No destination for key=$(repr(key)) → unlinked text $(repr(link_text))"
+            return Documenter.mdparse(link_text; mode=:span)
         end
     end
 end

test/Project.toml

@@ -1,6 +1,7 @@
 [deps]
 AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c"
 Bibliography = "f1be7e48-bf82-45af-a471-ae754a193061"
+Bijections = "e2ed5e7c-b2de-5872-ae92-c73ca462fb04"
 Coverage = "a2441757-f6aa-5fb2-8edb-039e3f45d037"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
 Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"

test/runtests.jl

@@ -56,6 +56,11 @@ using DocumenterCitations
         include("test_keys_with_underscores.jl")
     end
 
+    println("\n* anchor_keys (test_anchor_keys.jl):")
+    @time @safetestset "anchor_keys" begin
+        include("test_anchor_keys.jl")
+    end
+
     println("\n* integration test (test_integration.jl):")
     @time @safetestset "integration" begin
         include("test_integration.jl")

test/test_anchor_keys.jl

@@ -0,0 +1,98 @@
+using DocumenterCitations
+using DocumenterCitations: get_anchor_key
+using Test
+using Bijections
+using TestingUtilities: @Test  # much better at comparing strings
+using IOCapture: IOCapture
+
+include("run_makedocs.jl")
+
+
+@testset "Documenter.DOM anchors" begin
+    Documenter.DOM.@tags div
+    @test string(div["#anchor"]("")) == "<div id=\"anchor\"></div>"
+    @test string(div["#anchor-suffix"]("")) == "<div id=\"anchor-suffix\"></div>"
+    @test string(div["#anchor_suffix"]("")) == "<div id=\"anchor_suffix\"></div>"
+    # The way that Documenter.DOM eats `:` and `.` is the reason we have to
+    # strip those characters from citation keys. If that behavior ever changes,
+    # we can reconsider.
+    @test_broken string(div["#anchor:suffix"]("")) == "<div id=\"anchor:suffix\"></div>"
+    @test_broken string(div["#anchor.suffix"]("")) == "<div id=\"anchor.suffix\"></div>"
+end
+
+
+@testset "anchor key ambiguity" begin
+
+    cache = Bijections.Bijection{String,String}()
+
+    anchor_key = get_anchor_key("AbsilMahonySepulchre:2008", cache)
+    @test anchor_key == "AbsilMahonySepulchre_2008"
+
+    # cache hit
+    @test cache("AbsilMahonySepulchre_2008") == "AbsilMahonySepulchre:2008"
+    anchor_key = get_anchor_key("AbsilMahonySepulchre:2008", cache)
+    @test anchor_key == "AbsilMahonySepulchre_2008"
+
+    # Invalid: starts with number
+    anchor_key = get_anchor_key("2008_AbsilMahonySepulchre", cache)
+    @test anchor_key == "cit-2008_AbsilMahonySepulchre"
+
+    # Ambiguous: periods are not allowed allowed (substituted with '_')
+    c = IOCapture.capture(rethrow=Union{}) do
+        get_anchor_key("AbsilMahonySepulchre.2008", cache)
+    end
+    @test c.value == "AbsilMahonySepulchre_2008-2"
+    msg = "Warning: HTML anchor for citation key \"AbsilMahonySepulchre.2008\" normalizes to ambiguous \"AbsilMahonySepulchre_2008\" conflicting with citation key \"AbsilMahonySepulchre:2008\". Disambiguating with suffix \"-2\""
+    @test contains(c.output, msg)
+
+    # Ambiguous key: `=` is not allowed (dropped)
+    c = IOCapture.capture(rethrow=Union{}) do
+        get_anchor_key("AbsilMahonySepulchre_=2008", cache)
+    end
+    @test c.value == "AbsilMahonySepulchre_2008-3"
+    msg = "Warning: HTML anchor for citation key \"AbsilMahonySepulchre_=2008\" normalizes to ambiguous \"AbsilMahonySepulchre_2008\" conflicting with citation key \"AbsilMahonySepulchre:2008\". Disambiguating with suffix \"-2\""
+    @test contains(c.output, msg)
+    msg = "Warning: HTML anchor for citation key \"AbsilMahonySepulchre_=2008\" normalizes to ambiguous \"AbsilMahonySepulchre_2008-2\" conflicting with citation key \"AbsilMahonySepulchre.2008\". Disambiguating with suffix \"-3\""
+    @test contains(c.output, msg)
+
+end
+
+
+@testset "keys with symbols" begin
+
+    # https://github.com/JuliaDocs/DocumenterCitations.jl/issues/86
+
+    bib = CitationBibliography(
+        joinpath(@__DIR__, "test_anchor_keys", "src", "refs.bib"),
+        style=:numeric
+    )
+
+    run_makedocs(
+        joinpath(@__DIR__, "test_anchor_keys");
+        sitename="Test",
+        plugins=[bib],
+        pages=["Home" => "index.md", "References" => "references.md",],
+        warnonly=true,
+        check_success=true
+    ) do dir, result, success, backtrace, output
+
+        @test success
+
+        @test bib.anchor_keys["Chirikjian:2012"] == "Chirikjian_2012"
+        @test bib.anchor_keys["Chirikjian_2012"] == "Chirikjian_2012-2"
+
+        @test contains(output, "normalizes to ambiguous \"Chirikjian_2012\"")
+
+        #! format: off
+        index_html = read(joinpath(dir, "build", "index.html"), String)
+        @Test contains(index_html, "<a href=\"references/#Chirikjian_2012\">")
+        @Test contains(index_html, "<a href=\"references/#Chirikjian_2012-2\">")
+
+        references_html = read(joinpath(dir, "build", "references", "index.html"), String)
+        @Test contains(references_html, "<div id=\"Chirikjian_2012\">")
+        @Test contains(references_html, "<div id=\"Chirikjian_2012-2\">")
+        #! format: on
+
+    end
+
+end

test/test_anchor_keys/src/index.md

@@ -0,0 +1,5 @@
+# Testing citation keys with special symbols
+
+You can read more about the theory of Lie groups for example in [Chirikjian:2012](@cite).
+
+Note the ambiguous citations keys, as for Ref. [Chirikjian_2012](@cite) are automatically disambiguated.