Conversation
…at canonical URL Firefox (MV3) re-injects content scripts when a page changes the URL via history.pushState. Sites like archive.is append a #selection-NNN.N-NNN.N fragment to the URL on text selection, causing Firefox to re-inject the content script with the selection URL. Highlights were then saved under this transient URL instead of the canonical base URL, making them appear to disappear when the selection was cleared and the URL reverted. Strip the #selection-... fragment from window.location.href before assigning currentUrl so highlights are always stored and retrieved under the stable base URL. Closes #90 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd94ffc770
ℹ️ 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".
…UrlChange main added a navigation bridge that fires handleUrlChange on history.pushState events. Without this fix, archive.is selection URL changes would also trigger handleUrlChange with the #selection-... URL, causing highlights to be cleared and reloaded from the wrong key. Apply the same #selection-... normalization inside handleUrlChange so that selection fragment URL changes are treated as no-op navigation events. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@codex review latest code |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#selection-<N>.<N>-<N>.<N>fragment fromwindow.location.hrefwhen initializingcurrentUrlso highlights are always stored under the canonical base URLRoot Cause
Firefox (MV3) re-injects content scripts whenever a page changes its URL via
history.pushState. Sites like archive.is append a#selection-2049.0-2049.50fragment to the URL on text selection. This caused Firefox to re-inject the content script with the selection URL, settingcurrentUrlto the transient selection URL instead of the base URL. Highlights were then saved under this transient key. When the selection was cleared and the URL reverted, the content script was re-injected again with the base URL and found no highlights — making them appear to vanish (they were still in storage, just under the wrong key).Chrome is not affected because it does not re-inject content scripts on
history.pushState.Change
The regex only strips the archive.is-specific
#selection-NNN.N-NNN.Npattern, leaving all other hash fragments (e.g., SPA route hashes like#/page) untouched.Test plan
Closes #90
🤖 Generated with Claude Code