Use full URL parsing and search params for frame.src (WebKit#552)

The string-based concateation was rather brittle and can cause subtle bugs.

- Fix issue where hash and url params were not merged properly
- Add warnUnused argument to Params to only warn at top-level

Author: camillobruni

resources/shared/params.mjs

@@ -36,9 +36,9 @@ export class Params {
     // External config url to override internal tests.
     config = "";
 
-    constructor(searchParams = undefined) {
+    constructor(searchParams = undefined, warnUnused = false) {
         if (searchParams)
-            this._copyFromSearchParams(searchParams);
+            this._copyFromSearchParams(searchParams, warnUnused);
         if (!this.developerMode) {
             Object.freeze(this.viewport);
             Object.freeze(this);
@@ -52,7 +52,7 @@ export class Params {
         return parseInt(number);
     }
 
-    _copyFromSearchParams(searchParams) {
+    _copyFromSearchParams(searchParams, warnUnused) {
         this.viewport = this._parseViewport(searchParams);
         this.startAutomatically = this._parseBooleanParam(searchParams, "startAutomatically");
         this.iterationCount = this._parseIntParam(searchParams, "iterationCount", 1);
@@ -69,9 +69,11 @@ export class Params {
         this.measurePrepare = this._parseBooleanParam(searchParams, "measurePrepare");
         this.config = this._parseConfig(searchParams);
 
-        const unused = Array.from(searchParams.keys());
-        if (unused.length > 0)
-            console.error("Got unused search params", unused);
+        if (warnUnused) {
+            const unused = Array.from(searchParams.keys());
+            if (unused.length > 0)
+                console.error(`Got unused search params: ${unused.join(", ")}`);
+        }
     }
 
     _parseBooleanParam(searchParams, paramKey) {
@@ -219,12 +221,14 @@ function isValidJsonUrl(url) {
 
 export const defaultParams = new Params();
 
+export let paramsError = null;
 let maybeCustomParams = defaultParams;
 if (globalThis?.location?.search) {
     const searchParams = new URLSearchParams(globalThis.location.search);
     try {
-        maybeCustomParams = new Params(searchParams);
+        maybeCustomParams = new Params(searchParams, true);
     } catch (e) {
+        paramsError = e;
         console.error("Invalid URL Param", e, "\nUsing defaults as fallback:", maybeCustomParams);
     }
 }

resources/suite-runner.mjs

@@ -105,8 +105,10 @@ export class SuiteRunner {
             const frame = this.#frame;
             frame.onload = () => resolve();
             frame.onerror = () => reject();
-            const splitUrl = this.#suite.url.split("?");
-            frame.src = `${splitUrl[0]}?${splitUrl[1] ?? ""}&${this.#params.toSearchParams()}`;
+            const url = new URL(this.#suite.url, document.baseURI);
+            for (const [key, value] of this.#params.toSearchParamsObject())
+                url.searchParams.append(key, value);
+            frame.src = url.href;
         });
     }
 

tests/unittests/params.mjs

@@ -109,5 +109,24 @@ describe("Params", () => {
             );
             expect(params.suites).to.eql(["SuiteB", "Suite1", "SuiteA"]);
         });
+        it("should warn on unused params when warnUnused is true", () => {
+            const consoleErrorStub = sinon.stub(console, "error");
+            try {
+                new Params(new URLSearchParams({ unknownParam: "value" }), true);
+                expect(consoleErrorStub.calledOnce).to.be(true);
+                expect(consoleErrorStub.calledWith("Got unused search params: unknownParam")).to.be(true);
+            } finally {
+                consoleErrorStub.restore();
+            }
+        });
+        it("should not warn on unused params when warnUnused is false", () => {
+            const consoleErrorStub = sinon.stub(console, "error");
+            try {
+                new Params(new URLSearchParams({ unknownParam: "value" }), false);
+                expect(consoleErrorStub.called).to.be(false);
+            } finally {
+                consoleErrorStub.restore();
+            }
+        });
     });
 });