mirror of
https://github.com/librespeed/speedtest.git
synced 2026-10-21 07:40:50 +00:00
Stop forcing the IE11 upload workaround on Safari via user agent sniffing
The upload test reported low and unstable numbers on Safari (macOS and iOS) while the download test was correct. Cause: the "start" handler in speedtest_worker.js matched every Safari user agent and set settings.forceIE11Workaround = true. In ulTest() that flag selects a fundamentally different, less accurate measurement path: instead of sending one xhr_ul_blob_megabytes blob (20 MB by default) and accumulating event.loaded from upload.onprogress, it sends a stream of 256 KB requests and adds a fixed 256 KB to totLoaded on each onload. Progress is therefore quantised to whole requests and each request pays a full round trip before the next one starts, so on fast or high-latency links the measured speed is both too low and noisy. The workaround is not needed. Its comment claims it is only required for MPOT, but the block never checked settings.mpot. The same block also excludes crios and fxios, which on iOS run the very same WebKit networking stack as Safari, so the user agent test cannot have been guarding an engine-level bug in the first place. Current WebKit implements xhr.upload.onprogress, and the existing feature detection in ulTest() (the try/catch that probes xhr.upload.onprogress) already selects the workaround for browsers that genuinely lack it, without sniffing. Changes: - Remove the Safari user agent block. Safari now takes the regular upload path, the same one Chrome and Firefox on iOS already took. - Guard the Edge and PlayStation 4 blocks with typeof s.forceIE11Workaround === "undefined", matching how the other quirks avoid clobbering explicitly passed settings. Previously an explicit forceIE11Workaround: false in the start command was silently overwritten, so the workaround was not switchable from outside the worker. Default behaviour for those browsers is unchanged. - Add tests/e2e/worker-browser-quirks.spec.js, which loads the worker under spoofed user agents and asserts the resolved settings. - Update doc.md so Safari is no longer listed as affected. The Edge and PlayStation 4 blocks are deliberately left outside the enable_quirks condition: those browsers have no working upload.onprogress, so running them with enable_quirks: false would report 0 or Fail rather than a less precise number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S1bUKsok59shM69PwLMMRM
This commit is contained in:
parent
22b89a76c5
commit
4a95894f7a
3 changed files with 100 additions and 9 deletions
2
doc.md
2
doc.md
|
|
@ -732,7 +732,7 @@ To keep track of the amount of transferred data, the XHR Level 2 `upload.onprogr
|
|||
|
||||
This test has a couple of complications:
|
||||
|
||||
* Some browsers don't have a working `upload.onprogress` event. For this, we use a small blobs instead of a large one and we keep track of progress using the `onload` event. This is referred to as IE11 Workaround (but the same bug was also found in some versions of Edge and Safari)
|
||||
* Some browsers don't have a working `upload.onprogress` event. For this, we use a small blobs instead of a large one and we keep track of progress using the `onload` event. This is referred to as IE11 Workaround (the same bug was also found in some versions of Edge and in the PlayStation 4 browser). Affected browsers are detected by checking whether `xhr.upload` is usable, not by sniffing the user agent. Safari is __not__ affected and uses the regular, more accurate upload test
|
||||
* When `mpot` is set to `true`, an empty request must first be sent in order to load the CORS headers before the test can start
|
||||
|
||||
See the code for more implementation details.
|
||||
|
|
|
|||
|
|
@ -146,22 +146,22 @@ this.addEventListener("message", function(e) {
|
|||
}
|
||||
}
|
||||
if (/Edge.(\d+\.\d+)/i.test(ua)) {
|
||||
//Edge 15 introduced a bug that causes onprogress events to not get fired, we have to use the "small chunks" workaround that reduces accuracy
|
||||
settings.forceIE11Workaround = true;
|
||||
if (typeof s.forceIE11Workaround === "undefined") {
|
||||
//Edge 15 introduced a bug that causes onprogress events to not get fired, we have to use the "small chunks" workaround that reduces accuracy
|
||||
settings.forceIE11Workaround = true;
|
||||
}
|
||||
}
|
||||
if (/PlayStation 4.(\d+\.\d+)/i.test(ua)) {
|
||||
//PS4 browser has the same bug as IE11/Edge
|
||||
settings.forceIE11Workaround = true;
|
||||
if (typeof s.forceIE11Workaround === "undefined") {
|
||||
//PS4 browser has the same bug as IE11/Edge
|
||||
settings.forceIE11Workaround = true;
|
||||
}
|
||||
}
|
||||
if (/Chrome.(\d+)/i.test(ua) && /Android|iPhone|iPad|iPod|Windows Phone/i.test(ua)) {
|
||||
//cheap af
|
||||
//Chrome mobile introduced a limitation somewhere around version 65, we have to limit XHR upload size to 4 megabytes
|
||||
settings.xhr_ul_blob_megabytes = 4;
|
||||
}
|
||||
if (/^((?!chrome|android|crios|fxios).)*safari/i.test(ua)) {
|
||||
//Safari also needs the IE11 workaround but only for the MPOT version
|
||||
settings.forceIE11Workaround = true;
|
||||
}
|
||||
//telemetry_level has to be parsed and not just copied
|
||||
if (typeof s.telemetry_level !== "undefined") settings.telemetry_level = s.telemetry_level === "basic" ? 1 : s.telemetry_level === "full" ? 2 : s.telemetry_level === "debug" ? 3 : 0; // telemetry level
|
||||
//transform test_order to uppercase, just in case
|
||||
|
|
|
|||
91
tests/e2e/worker-browser-quirks.spec.js
Normal file
91
tests/e2e/worker-browser-quirks.spec.js
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
const fs = require("node:fs");
|
||||
const path = require("node:path");
|
||||
const { test, expect } = require("@playwright/test");
|
||||
|
||||
const workerSource = fs.readFileSync(path.join(__dirname, "..", "..", "speedtest_worker.js"), "utf8");
|
||||
|
||||
const userAgents = {
|
||||
safariMac:
|
||||
"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Safari/605.1.15",
|
||||
safariIos:
|
||||
"Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Mobile/15E148 Safari/604.1",
|
||||
chromeIos:
|
||||
"Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/123.0.6312.52 Mobile/15E148 Safari/604.1",
|
||||
firefoxIos:
|
||||
"Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/124.0 Mobile/15E148 Safari/605.1.15",
|
||||
chromeAndroid:
|
||||
"Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Mobile Safari/537.36",
|
||||
edge16:
|
||||
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Edge/16.16299",
|
||||
playstation4: "Mozilla/5.0 (PlayStation 4 5.55) AppleWebKit/601.2 (KHTML, like Gecko)"
|
||||
};
|
||||
|
||||
/*
|
||||
Loads speedtest_worker.js into a page that reports the given user agent, feeds it a "start"
|
||||
command and reports back the settings the worker resolved. The "_" test step is a no-op delay,
|
||||
so the worker settles its settings without performing any network I/O.
|
||||
*/
|
||||
async function resolveSettings(browser, userAgent, customSettings) {
|
||||
const context = await browser.newContext({ userAgent });
|
||||
try {
|
||||
const page = await context.newPage();
|
||||
await page.goto("about:blank");
|
||||
await page.addScriptTag({ content: workerSource });
|
||||
return await page.evaluate(custom => {
|
||||
window.dispatchEvent(new MessageEvent("message", { data: "start " + JSON.stringify(custom) }));
|
||||
return {
|
||||
userAgent: navigator.userAgent,
|
||||
forceIE11Workaround: settings.forceIE11Workaround,
|
||||
xhr_ul_blob_megabytes: settings.xhr_ul_blob_megabytes
|
||||
};
|
||||
}, Object.assign({ test_order: "_" }, customSettings));
|
||||
} finally {
|
||||
await context.close();
|
||||
}
|
||||
}
|
||||
|
||||
test.describe("speedtest_worker browser quirks", () => {
|
||||
test("Safari uses the accurate upload test instead of the IE11 workaround", async ({ browser }) => {
|
||||
for (const ua of [userAgents.safariMac, userAgents.safariIos]) {
|
||||
const resolved = await resolveSettings(browser, ua);
|
||||
expect(resolved.userAgent, "context user agent should be applied").toBe(ua);
|
||||
expect(resolved.forceIE11Workaround, `${ua} should not force the IE11 workaround`).toBe(false);
|
||||
expect(resolved.xhr_ul_blob_megabytes).toBe(20);
|
||||
}
|
||||
});
|
||||
|
||||
test("Safari, Chrome and Firefox on iOS resolve to the same upload path", async ({ browser }) => {
|
||||
const safari = await resolveSettings(browser, userAgents.safariIos);
|
||||
const chrome = await resolveSettings(browser, userAgents.chromeIos);
|
||||
const firefox = await resolveSettings(browser, userAgents.firefoxIos);
|
||||
expect(safari.forceIE11Workaround).toBe(chrome.forceIE11Workaround);
|
||||
expect(safari.forceIE11Workaround).toBe(firefox.forceIE11Workaround);
|
||||
});
|
||||
|
||||
test("Safari in MPOT mode also uses the accurate upload test", async ({ browser }) => {
|
||||
const resolved = await resolveSettings(browser, userAgents.safariMac, { mpot: true });
|
||||
expect(resolved.forceIE11Workaround).toBe(false);
|
||||
});
|
||||
|
||||
test("Chrome mobile still caps the upload blob at 4 megabytes", async ({ browser }) => {
|
||||
const resolved = await resolveSettings(browser, userAgents.chromeAndroid);
|
||||
expect(resolved.xhr_ul_blob_megabytes).toBe(4);
|
||||
expect(resolved.forceIE11Workaround).toBe(false);
|
||||
});
|
||||
|
||||
test("Edge and the PlayStation 4 browser keep the IE11 workaround by default", async ({ browser }) => {
|
||||
for (const ua of [userAgents.edge16, userAgents.playstation4]) {
|
||||
const resolved = await resolveSettings(browser, ua);
|
||||
expect(resolved.forceIE11Workaround, `${ua} should keep the IE11 workaround`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("an explicitly passed forceIE11Workaround is not overwritten by the quirks", async ({ browser }) => {
|
||||
for (const ua of [userAgents.edge16, userAgents.playstation4]) {
|
||||
const off = await resolveSettings(browser, ua, { forceIE11Workaround: false });
|
||||
expect(off.forceIE11Workaround, `${ua} should honour an explicit false`).toBe(false);
|
||||
}
|
||||
const on = await resolveSettings(browser, userAgents.safariMac, { forceIE11Workaround: true });
|
||||
expect(on.forceIE11Workaround, "an explicit true should still force the workaround").toBe(true);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue