Skip to content

Commit

Permalink
lib,url: correct URL's argument to pass idlharness
Browse files Browse the repository at this point in the history
`url.idl` defines URL's constructor as:

```
constructor(USVString url, optional USVString base);
```

`idlharness.any.js` checks its length as `1`. So we should remove
constructor's second argument and use `arguments[1]` in constructor's
logic.

Refs: https://url.spec.whatwg.org/#idl-index

PR-URL: #39848
Backport-PR-URL: #40383
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
XadillaX authored and targos committed Nov 4, 2021
1 parent abf3b84 commit 0a8c331
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/internal/url.js
Expand Up @@ -617,7 +617,7 @@ function onParseHashComplete(flags, protocol, username, password,
}

class URL {
constructor(input, base) {
constructor(input, base = undefined) {
// toUSVString is not needed.
input = `${input}`;
let base_context;
Expand Down
40 changes: 39 additions & 1 deletion test/common/wpt.js
Expand Up @@ -288,6 +288,7 @@ class WPTRunner {
this.resource = new ResourceLoader(path);

this.flags = [];
this.dummyGlobalThisScript = null;
this.initScript = null;

this.status = new StatusLoader(path);
Expand Down Expand Up @@ -318,6 +319,43 @@ class WPTRunner {
this.initScript = script;
}

get fullInitScript() {
if (this.initScript === null && this.dummyGlobalThisScript === null) {
return null;
}

if (this.initScript === null) {
return this.dummyGlobalThisScript;
} else if (this.dummyGlobalThisScript === null) {
return this.initScript;
}

return `${this.fullInitScript}\n\n//===\n${this.initScript}`;
}

/**
* Pretend the runner is run in `name`'s environment (globalThis).
* @param {'Window'} name
* @see {@link https://github.com/nodejs/node/blob/24673ace8ae196bd1c6d4676507d6e8c94cf0b90/test/fixtures/wpt/resources/idlharness.js#L654-L671}
*/
pretendGlobalThisAs(name) {
switch (name) {
case 'Window': {
this.dummyGlobalThisScript =
'global.Window = Object.getPrototypeOf(globalThis).constructor;';
break;
}

// TODO(XadillaX): implement `ServiceWorkerGlobalScope`,
// `DedicateWorkerGlobalScope`, etc.
//
// e.g. `ServiceWorkerGlobalScope` should implement dummy
// `addEventListener` and so on.

default: throw new Error(`Invalid globalThis type ${name}.`);
}
}

// TODO(joyeecheung): work with the upstream to port more tests in .html
// to .js.
runJsTests() {
Expand Down Expand Up @@ -368,7 +406,7 @@ class WPTRunner {
testRelativePath: relativePath,
wptRunner: __filename,
wptPath: this.path,
initScript: this.initScript,
initScript: this.fullInitScript,
harness: {
code: fs.readFileSync(harnessPath, 'utf8'),
filename: harnessPath,
Expand Down
6 changes: 3 additions & 3 deletions test/wpt/status/url.json
Expand Up @@ -13,9 +13,6 @@
"urlencoded-parser.any.js": {
"fail": "missing Request and Response"
},
"idlharness.any.js": {
"fail": "getter/setter names are wrong, etc."
},
"urlsearchparams-constructor.any.js": {
"fail": "FormData is not defined"
},
Expand All @@ -30,5 +27,8 @@
},
"url-setters-a-area.window.js": {
"skip": "already tested in url-setters.any.js"
},
"idlharness.any.js": {
"fail": "Fixed in a semver-major change: https://github.com/nodejs/node/pull/39752"
}
}
1 change: 1 addition & 0 deletions test/wpt/test-url.js
Expand Up @@ -15,4 +15,5 @@ runner.setInitScript(`
global.DOMException = DOMException;
`);

runner.pretendGlobalThisAs('Window');
runner.runJsTests();

0 comments on commit 0a8c331

Please sign in to comment.