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.

Sign up for GitHub

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

Jump to bottom

Correct result for passing DOMException.prototype to URLSearchParams #4581

Merged
merged 3 commits into from
Jan 26, 2017
Merged

Correct result for passing DOMException.prototype to URLSearchParams #4581

merged 3 commits into from
Jan 26, 2017

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 20, 2017

@annevk annevk mentioned this pull request Jan 20, 2017
Merged
@wpt-stability-bot
Copy link

wpt-stability-bot commented Jan 20, 2017 •
edited
Loading

Chrome (unstable channel)

Testing web-platform-tests at revision 98e4fcb
Using browser at version 55.0.2883.75
Starting 10 test iterations
All results were stable

All results

/url/urlsearchparams-constructor.html
Subtest Results
OK
Construct with object with two keys FAIL
URLSearchParams constructor, no arguments PASS
URLSearchParams constructor, empty string as argument PASS
Parse %00 PASS
Parse %20 PASS
Parse \0 PASS
URLSearchParams constructor, {} as argument FAIL
Parse space PASS
Custom [Symbol.iterator] FAIL
Basic URLSearchParams construction PASS
URLSearchParams constructor, DOMException.prototype as argument FAIL
Constructor with sequence of sequences of strings FAIL
Construct with object with + FAIL
Parse + PASS
Parse 💩 PASS
URLSearchParams constructor, string. PASS
URLSearchParams constructor, object. PASS
Parse %e2%8e%84 PASS
Parse %f0%9f%92%a9 PASS
Construct with array with two keys FAIL
Parse ⎄ PASS

@wpt-stability-bot
Copy link

wpt-stability-bot commented Jan 20, 2017 •
edited
Loading

Firefox (nightly channel)

Testing web-platform-tests at revision 98e4fcb
Using browser at version BuildID 20170123125947; SourceStamp 36486fdc3813ef7943ae5b07b4128866d1938a6c
Starting 10 test iterations
All results were stable

All results

/url/urlsearchparams-constructor.html
Subtest Results
OK
Construct with object with two keys FAIL
URLSearchParams constructor, no arguments PASS
URLSearchParams constructor, empty string as argument PASS
Parse %00 PASS
Parse %20 PASS
Parse \0 PASS
URLSearchParams constructor, {} as argument FAIL
Parse space PASS
Custom [Symbol.iterator] PASS
Basic URLSearchParams construction PASS
URLSearchParams constructor, DOMException.prototype as argument FAIL
Constructor with sequence of sequences of strings PASS
Construct with object with + FAIL
Parse + PASS
Parse 💩 PASS
URLSearchParams constructor, string. PASS
URLSearchParams constructor, object. PASS
Parse %e2%8e%84 PASS
Parse %f0%9f%92%a9 PASS
Construct with array with two keys PASS
Parse ⎄ PASS

domenic
domenic reviewed Jan 20, 2017
View reviewed changes
url/urlsearchparams-constructor.html
@@ -24,7 +24,7 @@

test(() => {
params = new URLSearchParams(DOMException.prototype);
assert_equals(params.toString(), "Error=")
assert_equals(params.toString(), "INDEX_SIZE_ERR=1&&HIERARCHY_REQUEST_ERR=3&WRONG_DOCUMENT_ERR=4&INVALID_CHARACTER_ERR=5&NO_MODIFICATION_ALLOWED_ERR=7&NOT_FOUND_ERR=8&NOT_SUPPORTED_ERR=9&INUSE_ATTRIBUTE_ERR=10&INVALID_STATE_ERR=11&SYNTAX_ERR=12&INVALID_MODIFICATION_ERR=13&NAMESPACE_ERR=14&SECURITY_ERR=18&NETWORK_ERR=19&ABORT_ERR=20&URL_MISMATCH_ERR=21&QUOTA_EXCEEDED_ERR=22&TIMEOUT_ERR=23&INVALID_NODE_TYPE_ERR=24&DATA_CLONE_ERR=25")
Copy link
Member

Choose a reason for hiding this comment

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

&& after INDEX_SIZE_ERR=1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well spotted. Sorry for that.

@domenic
Copy link
Member

domenic commented Jan 20, 2017

LGTM but we should get @cdumez's review too I think

@annevk annevk requested a review from cdumez January 20, 2017 17:25
cdumez
cdumez reviewed Jan 24, 2017
View reviewed changes
url/urlsearchparams-constructor.html
@@ -24,7 +24,7 @@

test(() => {
params = new URLSearchParams(DOMException.prototype);
assert_equals(params.toString(), "Error=")
assert_equals(params.toString(), "INDEX_SIZE_ERR=1&HIERARCHY_REQUEST_ERR=3&WRONG_DOCUMENT_ERR=4&INVALID_CHARACTER_ERR=5&NO_MODIFICATION_ALLOWED_ERR=7&NOT_FOUND_ERR=8&NOT_SUPPORTED_ERR=9&INUSE_ATTRIBUTE_ERR=10&INVALID_STATE_ERR=11&SYNTAX_ERR=12&INVALID_MODIFICATION_ERR=13&NAMESPACE_ERR=14&SECURITY_ERR=18&NETWORK_ERR=19&ABORT_ERR=20&URL_MISMATCH_ERR=21&QUOTA_EXCEEDED_ERR=22&TIMEOUT_ERR=23&INVALID_NODE_TYPE_ERR=24&DATA_CLONE_ERR=25")
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, Webkit nightly is getting:
INDEX_SIZE_ERR=1&DOMSTRING_SIZE_ERR=2&HIERARCHY_REQUEST_ERR=3&WRONG_DOCUMENT_ERR=4&INVALID_CHARACTER_ERR=5&NO_DATA_ALLOWED_ERR=6&NO_MODIFICATION_ALLOWED_ERR=7&NOT_FOUND_ERR=8&NOT_SUPPORTED_ERR=9&INUSE_ATTRIBUTE_ERR=10&INVALID_STATE_ERR=11&SYNTAX_ERR=12&INVALID_MODIFICATION_ERR=13&NAMESPACE_ERR=14&INVALID_ACCESS_ERR=15&VALIDATION_ERR=16&TYPE_MISMATCH_ERR=17&SECURITY_ERR=18&NETWORK_ERR=19&ABORT_ERR=20&URL_MISMATCH_ERR=21&QUOTA_EXCEEDED_ERR=22&TIMEOUT_ERR=23&INVALID_NODE_TYPE_ERR=24&DATA_CLONE_ERR=25

Copy link
Contributor

Choose a reason for hiding this comment

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

So it looks like we have the following extras:
DOMSTRING_SIZE_ERR=2
NO_DATA_ALLOWED_ERR=6
INVALID_ACCESS_ERR=15
VALIDATION_ERR=16
TYPE_MISMATCH_ERR=17

Copy link
Contributor

Choose a reason for hiding this comment

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

The Web IDL spec [1] still seems to contain the deprecated constants that WebKit supports. Am I missing something?

[1] https://heycam.github.io/webidl/#idl-DOMException-error-names

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the problem is with IDL. It only talks about one table, while in fact there are two. And for defining properties order is somewhat important, but with two tables order is lost.

I filed whatwg/webidl#284 and will push an update to this test shortly.

@annevk annevk mentioned this pull request Jan 25, 2017
Closed
@cdumez
Copy link
Contributor

cdumez commented Jan 26, 2017

LGTM.

@annevk annevk merged commit 405394a into master Jan 26, 2017
@annevk annevk deleted the annevk/urlsearchparams-domexception-prototype branch January 26, 2017 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez left review comments

@domenic domenic domenic left review comments

Assignees
No one assigned
Labels
url wg-webplatform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@annevk @wpt-stability-bot @domenic @cdumez @wpt-pr-bot