fix: stop readability breaking on mid-load toggles and hydrating pages - #850
Merged
Conversation
Toggling readability while a page was still loading (readyState 'interactive') attached a listener for a DOMContentLoaded event that had already fired, so it never ran — the loader stayed up forever, leaving the tab blank. Now only waits for the event when readyState is still 'loading'; otherwise it proceeds immediately. Separately, readability silently failed to auto-apply on page load for client-rendered pages (e.g. Gatsby/JAMstack blogs) whose real content isn't in the DOM yet at that point, but worked fine when toggled later by hand. The mount now retries a few times with short delays, and watches for the mounted reader being wiped out by a later hydration reconciliation pass (observed on one such site, ~500ms after a successful mount), re-applying if that happens. Verified against Wikipedia, Medium, Ars Technica, and The Guardian (toggle on/off both work and revert cleanly), plus github.com/reddit.com correctly staying disabled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…resh badge on toggle Split readability's apply()/remove() orchestration out of index.ts into a dedicated apply.ts, and added unit tests for it. While testing, found the mid-load-hydration retry added a real regression: if the initial mount failed and scheduled a retry, then the user turned readability off before that retry fired, the stale retry would still run afterward and silently re-mount the reader, overriding the "off" toggle. Retries are now tied to a generation counter that remove() bumps and cancels, so a superseded retry is a no-op. Also dropped the MutationObserver-based "clobbering" watch added for one Gatsby/JAMstack site's post-hydration DOM reconciliation — not worth the added complexity and harder-to-test surface for a narrow edge case. Separately, toggling readability from the popup/editor updated the stored value but never refreshed the toolbar badge (only EnableStyle/ DisableStyle did). SetReadability's background handler now re-fetches the tab's styles and calls updateIcon() too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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
DOMContentLoadedlistener attached after the event already fired (readyStateinteractive) never ran, leaving the loader stuck forever (repro: The Verge).remove()cancels.SetReadabilitynow refreshes it likeEnableStyle/DisableStylealready do.apply()/remove()out ofindex.tsintoapply.tsand adds unit tests, including a regression test for the toggle-off race.Test plan
yarn lint,yarn jest(138 passing, 8 new),yarn build🤖 Generated with Claude Code