Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test cases for URL empty query and fragment #25829

Closed

Conversation

watilde
Copy link
Contributor

@watilde watilde commented Sep 29, 2020

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @domenic want to double check with whatwg-url?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are incorrect because they use the about:blank base. What we really need to test is the no base case.

Since the only place no-base is testable is in new URL(), I think urltestdata.json is probably not the right place to put this. A separate .js file would work.

Or, if we did want to put them in urltestdata.json, then they need to omit the "base" key, and all the test harness code that uses urltestdata.json needs to get updated to skip such "base"-less test cases, unless they are testing the URL constructor.

@watilde
Copy link
Contributor Author

watilde commented Sep 30, 2020

I think we can update bURL in the tests to return no base URL instance when a base is not included since we currently have no test case that doesn't have a base in urltestdata.json so far.

@watilde
Copy link
Contributor Author

watilde commented Sep 30, 2020

I'll add a new js file for no base URL if my updated commit doesn't look good.

@@ -5,8 +5,7 @@ function setBase(base) {
}

function bURL(url, base) {
base = base || "about:blank"
setBase(base)
if (base) setBase(base)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work. It will leave the base URL for the document as the URL it is loaded in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we would have to skip this type of test when the base is lacking (and say we do it because you cannot unset the base URL of a document).

@@ -5,7 +5,7 @@
<div id=log></div>
<script>
function bURL(url, base) {
return new URL(url, base || "about:blank")
return base ? new URL(url, base) : new URL(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does work.

@guybedford
Copy link
Contributor

@watilde are you still planning on pushing this forward? It would be nice to see it merged.

@domenic domenic closed this in 6315c77 Jul 7, 2021
@watilde watilde deleted the url/empty-query-fragment branch July 8, 2021 01:07
@watilde
Copy link
Contributor Author

watilde commented Jul 8, 2021

@guybedford Thank you for picking up this one!

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 18, 2021
…ent tests, a=testonly

Automatic update from web-platform-tests
Test URLs with empty query and fragment

Closes whatwg/url#539.

Closes web-platform-tests/wpt#25829 by superseding it.

Co-authored-by: Daijiro Wachi <daijiro.wachi@gmail.com>
--

wpt-commits: 6315c7757a6675f39262167c8196ce562f2a6778
wpt-pr: 29579
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 20, 2021
…ent tests, a=testonly

Automatic update from web-platform-tests
Test URLs with empty query and fragment

Closes whatwg/url#539.

Closes web-platform-tests/wpt#25829 by superseding it.

Co-authored-by: Daijiro Wachi <daijiro.wachi@gmail.com>
--

wpt-commits: 6315c7757a6675f39262167c8196ce562f2a6778
wpt-pr: 29579
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 20, 2021
…ent tests, a=testonly

Automatic update from web-platform-tests
Test URLs with empty query and fragment

Closes whatwg/url#539.

Closes web-platform-tests/wpt#25829 by superseding it.

Co-authored-by: Daijiro Wachi <daijiro.wachi@gmail.com>
--

wpt-commits: 6315c7757a6675f39262167c8196ce562f2a6778
wpt-pr: 29579
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Jul 21, 2021
…ent tests, a=testonly

Automatic update from web-platform-tests
Test URLs with empty query and fragment

Closes whatwg/url#539.

Closes web-platform-tests/wpt#25829 by superseding it.

Co-authored-by: Daijiro Wachi <daijiro.wachi@gmail.com>
--

wpt-commits: 6315c7757a6675f39262167c8196ce562f2a6778
wpt-pr: 29579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants