Prevent the pathname setter from erasing the path of path-only URLs, …

[karwa]

…as that would make them cannot-be-a-base URLs.

Fixes #581

JSDom patch: jsdom/whatwg-url#178

• At least two implementers are interested (and none opposed):
  • Me 😅
  • …
• Tests are written and can be reviewed and commented upon at:
  • [URL] Ensure that path-only URLs do not have their paths erased by th… web-platform-tests/wpt#27720
• Implementation bugs are filed:
  • Chrome: …
  • Firefox: …
  • Safari: …

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[annevk]

Thanks for spotting this, this does look like a somewhat serious oversight.

[annevk]

Why only when host is null? It seems this needs to generally happen.

[karwa]

Because non-special URLs with a host (e.g. foo://somehost) can have an empty string as a path. The issue is limited to path-only URLs.

[domenic]

Editorially LGTM and matches the tests and jsdom/whatwg-url implementation.

In terms of interop and implementation alignment, of the three tests added in web-platform-tests/wpt#27720:

• Safari passes test 1 and 3, but not test 2
• Firefox and Chrome pass test 1, but not test 2 or test 3
• jsdom/whatwg-url, and thus the current spec, passes tests 1 and 2 but not test 3

Should we consider changing the spec for test 2? I don't have a strong intuition on the model here though so maybe there's a reason that would be bad.

[annevk]

Chrome and Firefox still don't have non-special URLs implemented correctly so I rather not count them.

Safari doesn't seem to allow for removal of the path if there is a host for non-special URLs. I tend to side with OP that it should be possible. That is, if foo://host doesn't create a path and foo://host/ does, it seems nice if you can get from the latter to the former by setting pathname to the empty string.

cc @achristensen07

[karwa]

I think it is valid (or, well, not disallowed) for some particular schemes/application to treat an empty path differently from a "/" path.

For instance if you're connecting to a remote server: for some hypothetical scheme, myproto://user@host could mean to connect to the user's home directory, and myproto://user@host/ could mean to connect to the remote server's root directory. It may/may not be a good design choice, but if it's allowed, somebody could be relying on it.

[rmisev]

LGTM

[annevk]

@achristensen07 any thoughts on this?

[TimothyGu]

It looks like this PR only changes the behavior of test 3 in web-platform-tests/wpt#27720, and Safari already implements the new behavior for test 3. @annevk, would this be enough to count as implementer interest?

[annevk]

I would still prefer to wait for @achristensen07 or someone else from WebKit to weigh in as test 2 does require implementation changes.

[achristensen07]

I haven't gone through the setters like I have the constructor. I think test 2 shows a bug in WebKit that I'm willing to try to change. NSURLComponents already has the behavior that would make test 2 pass, and parsing foo://somehost doesn't add a slash to the path, so I think the path setter shouldn't either, unless called with something that isn't empty and doesn't start with a slash.

[annevk]

Thanks @achristensen07! @karwa can you file a bug against WebKit for that? I don't think we need bugs for Chrome/Firefox as they haven't yet caught up here.

[karwa]

Filed https://bugs.webkit.org/show_bug.cgi?id=227073

[annevk]

I filed nodejs/node#39059 on Node.js.

[annevk]

Thanks @karwa for finding and fixing this! 🎉