Repository navigation
fix(documenting-html): use unique element IDs, keep same-name endpoints and escape search results - #54
Conversation
…ts and escape search results
Endpoint IDs were built by stripping every non-alphanumeric character from
the endpoint name, which produced empty IDs ("!!!"), IDs that collide
between an endpoint and a group ("Users"), and IDs that are invalid CSS
selectors ("2FA setup" -> "2FAsetup"). The scroll spy then threw a
SyntaxError from querySelector and aborted the page script.
- Derive endpoint IDs from method and route path with an ep- prefix, group
IDs with a grp- prefix, and append a short hash so IDs are never empty or
colliding. The scroll spy resolves them with getElementById.
- Deduplicate endpoints by method and route path instead of method and name,
so distinct routes sharing a name are no longer dropped.
- The copy-embed button now carries the endpoint ID, and embed-target looks
endpoints up by ID, falling back to the name for previously copied embeds.
- Escape endpoint names, paths, groups and methods before they are written
to the search results with innerHTML.
Note: anchors produced by the previous ID scheme (for example #Users) no
longer resolve.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f8761fe00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| string hash = Convert.ToHexString ( SHA256.HashData ( Encoding.UTF8.GetBytes ( unsafeId ) ) ) [..8].ToLowerInvariant (); | ||
| return $"{new string ( unsafeId.Where ( char.IsLetterOrDigit ).ToArray () )}-{hash}"; |
There was a problem hiding this comment.
The eight-character SHA-256 slice retains only 32 bits, so it cannot guarantee the uniqueness this change relies on. For example, Get ^/x--_-_-__-_-_____--z$ and Get ^/x__-___--____-_--_-z$ both sanitize to Getxz and have the same retained digest (dd883b3e); these are valid distinct regex route patterns. Documenting both produces duplicate summary IDs, causing navigation, search, and embed lookup to resolve only the first endpoint. Retain the full digest (or resolve collisions) when constructing the ID.
Useful? React with 👍 / 👎.
1 participant
What
Fixes in the HTML documentation exporter around element IDs, endpoint de-duplication, the search dialog and embeds.
TransformIdstripped every non-alphanumeric character, producing empty IDs (!!!), IDs that collide between an endpoint and a group (Users), and IDs that are invalid CSS selectors (2FA setupbecomes2FAsetup). The scroll spy usedquerySelector(href), which threwSyntaxErrorand aborted the whole page script. Endpoint IDs are now derived from method and route path with anep-prefix, group IDs get agrp-prefix, and a short hash is appended so IDs are never empty or colliding. The scroll spy usesgetElementById.Method Name, soGET /items/aandGET /items/bannotated with the same name collapsed into one. They are now de-duplicated byMethod Path, which still removes a route declared twice.embed-targetresolves by ID, falling back to the name so embeds copied earlier keep working. Same-name endpoints can now be embedded separately.innerHTMLfrom text read back throughtextContent, so a name such asList<T>or one containing markup became DOM. Values are now escaped, including around the<mark>highlights.Verification
This extension has no test project, so I verified it with an external probe (not included in the PR): a small server with seven endpoints that cover empty, duplicate and invalid IDs, same-name routes and hostile names, driven by jsdom.
maindotnet buildofSisk.Documenting.Htmlsucceeds.Notes and limitations
#Usersno longer resolve, because the ID scheme changed. Embed URLs that use the endpoint name still work through the fallback.embed-targethandler (rvalidation and JS encoding). I could not reproduce those problems onmain:GetDocumentationUrlalready requires an absolute http/httpsr, and the value is already passed throughJavaScriptEncoder. The probe checks these cases and they pass on both builds.embed-targetvalues still return the existingconsole.errorscript; I did not change that response.