Fixed several issues in switch-dark-mode.js - #2670
Open
medmunds wants to merge 4 commits into
Open
Conversation
medmunds
commented
Jun 23, 2026
| // will use a host-only cookie. | ||
| ]); | ||
| const cookieSecure = window.location.protocol === 'https:'; | ||
| const cookieSameSite = 'Lax'; |
Contributor
Author
There was a problem hiding this comment.
Reviewer note: this deliberately uses SameSite=Lax, always. (The code it replaces downgrades to SameSite=None on https.) I can't see any reason the theme cookie would need to be included with cross-site requests to *.djangoproject.com from outside domains.
Changed cycleTheme() to treat <html data-theme> as source of current truth, rather than stored cookie. This fixes a bug where the theme switcher button would not loop through all options if the browser was blocking cookies. Removed call to initTheme() on prefers-color-scheme change. This was not needed for data-theme=auto (which is implemented entirely in CSS), and would cause the theme to reset to auto if the browser was blocking cookies.
switch-dark-mode.js had been calculating the domain for its cookie by removing everything up to the first dot in the hostname, so the theme choice would be shared between docs.djangoproject.com, code.djangoproject.com, etc. But that doesn't work for hostname=127.0.0.1 or localhost, and it's not ideal for PR preview domains like pr-12345.django.readthedocs.build. It also wouldn't work correctly if we ever wanted to use djangoproject.com (without a subdomain). Updated to determine the shared cookie domain from an allowlist of base domains and fall back to a host-only cookie anywhere else. In the process, also moved all related configuration to the top of the script, reduced duplicated code and use of inline magic values, tried to use consistent naming, and updated to newer JavaScript where helpful. (The script remains fully compatible with MDN baseline.)
Reloaded the theme setting from the cookie when the browser restores page state from the backward/forward cache. This fixes a problem where a theme change on one page appeared to be reverted when navigating via history buttons.
medmunds
force-pushed
the
2669-switch-dark-mode-js
branch
from
June 23, 2026 18:15
5a75a28 to
3cd1d3b
Compare
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.
Fixes #2669, which details four semi-related issues in switch-dark-mode.js:
This is currently split into four separate commits, one for each problem. They can be squashed, or pulled apart if we decide some of these aren't worth fixing.
The changes to address the third issue ended up large enough that I did some additional refactoring to try to make the script more maintainable. But I can go back and try to make a minimal-delta edit if we'd prefer that.