Skip to content

Commit 326a6db

Browse files
authored
Merge remote-tracking branch 'origin/main' into fix/biblio-specref-guard
# Conflicts: # builds/respec-aom.js # builds/respec-aom.js.map # builds/respec-dini.js # builds/respec-dini.js.map # builds/respec-w3c.js # builds/respec-w3c.js.map # tests/spec/core/biblio-spec.js
2 parents 5b661fe + 844a8b2 commit 326a6db

13 files changed

Lines changed: 553 additions & 288 deletions

builds/respec-aom.js

Lines changed: 57 additions & 57 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

builds/respec-aom.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

builds/respec-dini.js

Lines changed: 58 additions & 58 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

builds/respec-dini.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

builds/respec-w3c.js

Lines changed: 126 additions & 126 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

builds/respec-w3c.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "respec",
3-
"version": "37.3.5",
3+
"version": "37.3.6",
44
"license": "W3C",
55
"description": "A technical specification pre-processor.",
66
"engines": {
@@ -81,7 +81,7 @@
8181
"server": "serve",
8282
"start": "node ./tools/dev-server.cjs",
8383
"test:build": "jasmine --random=false ./tests/test-build.cjs",
84-
"test:headless": "jasmine --random=false ./tests/headless.cjs",
84+
"test:headless": "jasmine --random=false ./tests/headless.cjs ./tests/darkmode-race.cjs",
8585
"test": "pnpm test:unit && pnpm test:integration",
8686
"test:unit": "karma start ./tests/unit/karma.conf.cjs --single-run",
8787
"test:integration": "karma start ./tests/spec/karma.conf.cjs --single-run"

src/core/biblio.js

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,20 @@ export const biblio = {};
1212

1313
export const name = "core/biblio";
1414

15-
const bibrefsURL = new URL("https://api.specref.org/bibrefs?refs=");
15+
/** Keep Specref first: a citation then resolves the same here as in every other spec tool. */
16+
const bibrefsURLs = [
17+
new URL("https://api.specref.org/bibrefs?refs="),
18+
new URL("https://respec.org/bibrefs?refs="),
19+
];
20+
21+
/** Without this, a service that connects and never replies blocks the fallback. */
22+
const FETCH_TIMEOUT_MS = 5000;
1623

1724
// Opportunistically dns-prefetch to bibref server, as we don't know yet
1825
// if we will actually need to download references yet.
1926
const link = createResourceHint({
2027
hint: "dns-prefetch",
21-
href: bibrefsURL.origin,
28+
href: bibrefsURLs[0].origin,
2229
});
2330
document.head.appendChild(link);
2431
/** @type {(value: Conf['biblio']) => void} */
@@ -29,33 +36,56 @@ const done = new Promise(resolve => {
2936
doneResolver = resolve;
3037
});
3138

39+
/**
40+
* Asks each bibliography service in turn and returns the first usable answer.
41+
* A service only counts as answering once its body parses, so an error page
42+
* that comes back as 200 and HTML still falls through to the next one.
43+
*
44+
* @param {string} refs comma separated reference ids
45+
* @returns {Promise<{ data: Conf['biblio'], expires: string | null } | null>}
46+
*/
47+
async function fetchBibrefs(refs) {
48+
for (const url of bibrefsURLs) {
49+
let response;
50+
try {
51+
response = await fetch(url.href + refs, {
52+
signal: AbortSignal.timeout(FETCH_TIMEOUT_MS),
53+
});
54+
} catch (err) {
55+
console.warn(`Could not reach ${url.origin} for references.`, err);
56+
continue;
57+
}
58+
if (response.status !== 200) {
59+
console.warn(`${url.origin} answered ${response.status} for references.`);
60+
continue;
61+
}
62+
try {
63+
const data = await response.json();
64+
return { data, expires: response.headers.get("Expires") };
65+
} catch (err) {
66+
console.warn(`${url.origin} sent references that are not JSON.`, err);
67+
}
68+
}
69+
return null;
70+
}
71+
3272
/** @param {string[]} refs */
33-
export async function updateFromNetwork(
34-
refs,
35-
options = { forceUpdate: false }
36-
) {
73+
export async function updateFromNetwork(refs) {
3774
const refsToFetch = [...new Set(refs)].filter(ref => ref.trim());
3875
// Update database if needed, if we are online
3976
if (!refsToFetch.length || navigator.onLine === false) {
4077
return null;
4178
}
42-
let response;
43-
try {
44-
response = await fetch(bibrefsURL.href + refsToFetch.join(","));
45-
} catch (err) {
46-
console.error(err);
47-
return null;
48-
}
49-
if ((!options.forceUpdate && !response.ok) || response.status !== 200) {
79+
const found = await fetchBibrefs(refsToFetch.join(","));
80+
if (!found) {
5081
return null;
5182
}
52-
/** @type {Conf['biblio']} */
53-
const data = await response.json();
83+
const { data, expires: expiresHeader } = found;
5484
// SpecRef updates every hour, so we should follow suit
5585
// https://github.com/tobie/specref#hourly-auto-updating
5686
const oneHourFromNow = Date.now() + 1000 * 60 * 60 * 1;
5787
try {
58-
const expiresValue = Date.parse(response.headers.get("Expires") || "");
88+
const expiresValue = Date.parse(expiresHeader || "");
5989
const expires = Number.isNaN(expiresValue)
6090
? oneHourFromNow
6191
: Math.min(expiresValue, oneHourFromNow);
@@ -173,7 +203,7 @@ export class Plugin {
173203
const externalRefs = split.noData.map(item => item.id);
174204
if (externalRefs.length) {
175205
// Going to the network for refs we don't have
176-
const data = await updateFromNetwork(externalRefs, { forceUpdate: true });
206+
const data = await updateFromNetwork(externalRefs);
177207
Object.assign(biblio, data);
178208
}
179209
Object.assign(biblio, this.conf.localBiblio);

src/w3c/style.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,13 @@ export function run(conf) {
137137
href="${darkModeStyleURL.href}"
138138
/>`;
139139
if (isDark) darkLink.media = "(prefers-color-scheme: dark)";
140+
// Setting `.disabled` on a still-loading link does not stick in Chrome, and the next write
141+
// drops the sheet (#5436), so set the state before the element enters the document.
142+
if (!isDark) darkLink.setAttribute("disabled", "");
140143
document.head.appendChild(darkLink);
141144
if (isDark) {
142145
// As required by W3C Pub Rules.
143146
sub("beforesave", styleMover(darkModeStyleURL));
144-
} else {
145-
// `disabled` rather than `media="not all"` because fixup.js sets
146-
// `darkCss.media = ""`, which would wipe a media query. Must be set after
147-
// insertion: per HTML the setter is a no-op while the link's associated CSS
148-
// style sheet is still null.
149-
darkLink.disabled = true;
150147
}
151148
}
152149

tests/darkmode-race.cjs

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Regression test for speced/respec#5436. Only Chrome reproduces the race, so this passes on
2+
// other engines whether or not the fix is present.
3+
const http = require("http");
4+
const path = require("path");
5+
const fs = require("fs");
6+
const puppeteer = require("puppeteer");
7+
8+
// A color nothing else in the page uses, so its presence is unmistakably this stylesheet.
9+
const DARK_MARKER = "rgb(11, 22, 33)";
10+
const LIGHT = "rgb(255, 255, 255)";
11+
// Cold CI runners need far longer than puppeteer's 30s default to start Chrome, which
12+
// tools/respecDocWriter.js and tests/headless.cjs both budget 120s for.
13+
const LAUNCH_TIMEOUT = 120000;
14+
15+
describe("W3C - Style - dark stylesheet arriving late (#5436)", () => {
16+
let server;
17+
let browser;
18+
let port;
19+
const repoRoot = path.resolve(__dirname, "..");
20+
21+
beforeAll(async () => {
22+
// Launch plus processing plus slack, the same shape tests/headless.cjs uses.
23+
jasmine.DEFAULT_TIMEOUT_INTERVAL = LAUNCH_TIMEOUT + 60000;
24+
server = http.createServer((req, res) => {
25+
if (req.url === "/") {
26+
res.writeHead(200, { "Content-Type": "text/html" });
27+
res.end(`
28+
<!DOCTYPE html>
29+
<html>
30+
<head>
31+
<meta charset="utf-8">
32+
<title>Race Condition Test</title>
33+
<script>
34+
var respecConfig = {
35+
specStatus: "ED",
36+
shortName: "x",
37+
group: "webapps",
38+
editors: [{name: "T"}],
39+
xref: false
40+
};
41+
</script>
42+
<script src="/builds/respec-w3c.js"></script>
43+
</head>
44+
<body>
45+
<section id="abstract"><p>Abstract</p></section>
46+
<section id="sotd"><p>SOTD</p></section>
47+
</body>
48+
</html>
49+
`);
50+
} else {
51+
const requestPath = new URL(req.url, "http://localhost").pathname;
52+
const filePath = path.resolve(repoRoot, `.${requestPath}`);
53+
const relativePath = path.relative(repoRoot, filePath);
54+
if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) {
55+
res.writeHead(404);
56+
res.end("Not Found");
57+
return;
58+
}
59+
fs.readFile(filePath, (err, data) => {
60+
if (err) {
61+
res.writeHead(404);
62+
res.end("Not Found");
63+
return;
64+
}
65+
const ext = path.extname(filePath);
66+
const contentTypes = {
67+
".js": "text/javascript",
68+
".css": "text/css",
69+
};
70+
res.writeHead(200, {
71+
"Content-Type": contentTypes[ext] || "text/plain",
72+
});
73+
res.end(data);
74+
});
75+
}
76+
});
77+
78+
await new Promise(resolve => {
79+
server.listen(0, () => {
80+
port = server.address().port;
81+
resolve();
82+
});
83+
});
84+
85+
browser = await puppeteer.launch({
86+
headless: true,
87+
timeout: LAUNCH_TIMEOUT,
88+
});
89+
});
90+
91+
afterAll(async () => {
92+
if (browser) await browser.close();
93+
if (server) await new Promise(resolve => server.close(resolve));
94+
});
95+
96+
// Loads the document with the dark stylesheet held back, so it lands between ReSpec's write
97+
// and fixup.js's, and returns what the body settled on.
98+
async function settled(scheme) {
99+
const page = await browser.newPage();
100+
await page.emulateMediaFeatures([
101+
{ name: "prefers-color-scheme", value: scheme },
102+
]);
103+
104+
await page.setRequestInterception(true);
105+
page.on("request", request => {
106+
if (/dark\.css/.test(request.url())) {
107+
// These bytes are local and known, so a dark stylesheet that never arrived cannot be
108+
// mistaken for a correct light result. fixup.js and the maturity stylesheet still come
109+
// from www.w3.org, as they do across this suite, so this does need the network.
110+
setTimeout(() => {
111+
request.respond({
112+
status: 200,
113+
contentType: "text/css",
114+
body: `body { background-color: ${DARK_MARKER} !important; }`,
115+
});
116+
}, 500);
117+
} else {
118+
request.continue();
119+
}
120+
});
121+
122+
await page.goto(`http://localhost:${port}/`);
123+
// fixup.js builds this control, so its presence means both scripts have run.
124+
await page.waitForSelector("input[name=color-scheme]");
125+
await new Promise(resolve => setTimeout(resolve, 3000));
126+
const bg = await page.evaluate(
127+
() => getComputedStyle(document.body).backgroundColor
128+
);
129+
return { page, bg };
130+
}
131+
132+
it("stays light on a light system when the dark stylesheet arrives late", async () => {
133+
// Only this direction is pinned. The reported dark-system case passes on main in 5 of 5
134+
// runs here, so a spec for it would never fail without the fix; reproducing that direction
135+
// needs the live stylesheet and still only fails about 19 times in 20.
136+
const { page, bg } = await settled("light");
137+
expect(bg).toBe(LIGHT);
138+
139+
// A light result could also mean the stylesheet never loaded, so prove it can apply.
140+
await page.evaluate(() => {
141+
document.querySelector("input[name=color-scheme][value=dark]")?.click();
142+
});
143+
await page.waitForFunction(
144+
expected => getComputedStyle(document.body).backgroundColor === expected,
145+
{ timeout: 5000 },
146+
DARK_MARKER
147+
);
148+
});
149+
});

0 commit comments

Comments
 (0)