fix: enhance sidebar navigation link selection for decoded URLs#2708
fix: enhance sidebar navigation link selection for decoded URLs#2708sy-records merged 2 commits intodocsifyjs:developfrom
Conversation
|
@sy-records is attempting to deploy a commit to the Docsify Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes sidebar navigation link selection so the active sidebar item (and its generated TOC expansion) can be found even when the current route URL is percent-encoded but the sidebar link href is decoded (e.g., non‑Latin filenames like Chinese), addressing #2706.
Changes:
- Decode the computed active route URL and attempt to match the sidebar link by either encoded or decoded
href. - Update the sidebar active element lookup selector to support both forms.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/core/render/index.js
Outdated
| const activeElmHref = this.router.toURL(this.route.path); | ||
| const decodedHref = decodeURIComponent(activeElmHref); | ||
| const activeEl = /** @type {HTMLElement | null} */ ( | ||
| dom.find(`.sidebar-nav a[href="${activeElmHref}"]`) | ||
| dom.find( | ||
| `.sidebar-nav a[href="${activeElmHref}"]${activeElmHref !== decodedHref ? `, .sidebar-nav a[href="${decodedHref}"]` : ''}`, | ||
| ) |
There was a problem hiding this comment.
decodeURIComponent(activeElmHref) can throw on malformed percent-encoding (e.g. a user navigates to a hash containing a raw %), which would break sidebar rendering here. Also, using the decoded value directly inside a CSS attribute selector can make querySelector throw if decoding introduces selector-breaking characters like " or \. Consider a safe decode (try/catch + fallback to original) and/or avoid interpolating unescaped URL strings into a selector (e.g., iterate anchors and compare getAttribute('href'), or escape via CSS.escape with a fallback).
There was a problem hiding this comment.
Thanks @sy-records! Created the following codesandbox to test, both with current rc-4 (confirmed issue) and PR preview:
https://codesandbox.io/p/sandbox/2jjczx
With the example content as described in original issue, this fix looks to be working.
Great catch @a1ess!
Summary
Related issue, if any:
Close #2706
What kind of change does this PR introduce?
For any code change,
Does this PR introduce a breaking change?
Tested in the following browsers: