fix: improve minimap behavior for multi-span highlights#92
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 010295dd2f
ℹ️ 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".
|
|
||
| const groupId = highlightElement.dataset.groupId; | ||
| const highlightElements = groupId | ||
| ? Array.from(document.querySelectorAll(`.text-highlighter-extension[data-group-id='${groupId}']`)) |
There was a problem hiding this comment.
Escape groupId before querySelectorAll selector interpolation
groupId is inserted raw into an attribute selector here, so a valid imported ID containing selector metacharacters (for example ' or ]) will cause querySelectorAll(...) to throw a SyntaxError and abort updateMarkers(), which leaves the minimap broken for that page. This is reachable because highlight import currently accepts any non-empty string for groupId; use CSS.escape(groupId) (or a non-selector lookup) before building the selector (and apply the same fix to the identical pattern in highlightTemporarily).
Useful? React with 👍 / 👎.
Summary
Testing