-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 tests for constructible/subclassable EventTarget #6306
Conversation
*This report has been truncated because the total content is 2960860 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Firefox (nightly)Testing web-platform-tests at revision d547d0c All results6 tests ran/XMLHttpRequest/interfaces.html
/dom/events/EventTarget-constructible.any.html
/dom/events/EventTarget-constructible.any.worker.html
/dom/interfaces.html
|
Sauce (safari)Testing web-platform-tests at revision d547d0c All results6 tests ran/XMLHttpRequest/interfaces.html
/dom/events/EventTarget-constructible.any.html
/dom/events/EventTarget-constructible.any.worker.html
/dom/interfaces.html
/fullscreen/interfaces.html
/html/dom/interfaces.html
|
*This report has been truncated because the total content is 3000042 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Chrome (unstable)Testing web-platform-tests at revision d547d0c All results6 tests ran/XMLHttpRequest/interfaces.html
/dom/events/EventTarget-constructible.any.html
/dom/events/EventTarget-constructible.any.worker.html
/dom/interfaces.html
|
*This report has been truncated because the total content is 2836552 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Sauce (MicrosoftEdge)Testing web-platform-tests at revision d547d0c All results6 tests ran/XMLHttpRequest/interfaces.html
/dom/events/EventTarget-constructible.any.html
/dom/events/EventTarget-constructible.any.worker.html
/dom/interfaces.html
|
|
||
target.on("foo", listener); | ||
|
||
target.dispatch("foo", detail); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detail is not declared anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed. Hard to write tests without anything to test against.
21977bd
to
280765a
Compare
@@ -44,7 +44,7 @@ dictionary CustomEventInit : EventInit { | |||
}; | |||
|
|||
|
|||
//[Exposed=(Window,Worker)] | |||
[Constructor/*, Exposed=(Window,Worker)*/] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this still needs to be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. But, I figured it was best to stay consistent, and if someone wanted to go through and update that file en masse, they could.
For what it's worth, the following files need to be fixed to have tests not fail in a UA that implements constructible EventTarget:
because they have an I'll fix those files in https://bugzilla.mozilla.org/show_bug.cgi?id=1379688 if someone doesn't get to them first. |
It looks like at least some of those are fixed; see e.g. https://github.com/w3c/web-platform-tests/blob/b0efb74111af13f7abf3b3b1860667f98746b07f/eventsource/interfaces.html#L12 . In fact I think all of them are fixed in master... |
Ah, because the Blink folks ran into the same issue I did and synced their stuff back already, ok. ;) |
Follows whatwg/dom#467.