Skip to content

fix(w3c/style): disable the dark stylesheet before inserting the link - #5439

Merged
marcoscaceres merged 6 commits into
mainfrom
fix/dark-theme-race
Sep 2, 2026
Merged

fix(w3c/style): disable the dark stylesheet before inserting the link#5439
marcoscaceres merged 6 commits into
mainfrom
fix/dark-theme-race

Conversation

@marcoscaceres

@marcoscaceres marcoscaceres commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Refs #5436.

Since v37.3.3 a spec that never opted into dark mode paints the wrong color scheme on roughly one cold load in five in Chrome, in both directions, because ReSpec assigned disabled to the dark stylesheet link on the statement after inserting it, and a write that lands mid-fetch leaves Chrome applying the sheet anyway and dropping it on the next write. This sets the attribute before insertion instead, so the link is never in the document both enabled and still loading.

AI generated the logic here. Gemini wrote the test from the two reports before the fix existed; only the light-system direction is pinned, because a dark-system spec passed on the base commit in 5 of 5 runs and so could never fail without the fix.

Proof: npx jasmine --random=false tests/darkmode-race.cjs fails on the base commit in 3 of 3 runs with Expected 'rgb(11, 22, 33)' to be 'rgb(255, 255, 255)', and passes in 3 of 3 on the head commit. That color is what the test serves in place of dark.css, so a failure means the dark stylesheet applied for a reader whose system asked for light.

Co-authored-by: Gemini 2.5 Pro <noreply@google.com>

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Run {

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents Chrome from briefly enabling the W3C dark stylesheet before its disabled state is established.

Changes:

  • Sets disabled before inserting the dark stylesheet.
  • Adds integration and Puppeteer regression coverage.
  • Includes the race test in headless testing.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
src/w3c/style.js Disables the dark stylesheet before insertion.
tests/spec/w3c/style-spec.js Updates stylesheet-state coverage.
tests/darkmode-race.cjs Adds a Chrome race regression test.
package.json Runs the regression with headless tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/spec/w3c/style-spec.js Outdated
Comment thread tests/darkmode-race.cjs
Comment thread tests/darkmode-race.cjs Outdated
Comment thread tests/darkmode-race.cjs Outdated
Comment thread tests/darkmode-race.cjs Outdated
Comment thread tests/darkmode-race.cjs
emwhipple30-svg

This comment was marked as low quality.

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

Confirmed working as expected locally. Ran 50 times in chrome, no race condition.

@marcoscaceres marcoscaceres added the AI Contains AI-generated logic; see AI_POLICY.md label Sep 2, 2026
@marcoscaceres
marcoscaceres merged commit 844a8b2 into main Sep 2, 2026
9 checks passed
@marcoscaceres
marcoscaceres deleted the fix/dark-theme-race branch September 2, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Contains AI-generated logic; see AI_POLICY.md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants