-
Notifications
You must be signed in to change notification settings - Fork 3k
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 various tests for error event specialness #5014
Conversation
Follows whatwg/html#2398; see also whatwg/html#2296.
Firefox (nightly channel)Testing web-platform-tests at revision be6e3bca3cfabd6da653db10529ff6d07ae3a003 All results7 tests ran/html/webappapis/scripting/events/event-handler-processing-algorithm-error/document-synthetic-errorevent.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/document-synthetic-event.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/script-element.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-runtime-error.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-synthetic-errorevent.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-synthetic-event.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/worker.html
|
Chrome (unstable channel)Testing web-platform-tests at revision be6e3bca3cfabd6da653db10529ff6d07ae3a003 All results7 tests ran/html/webappapis/scripting/events/event-handler-processing-algorithm-error/document-synthetic-errorevent.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/document-synthetic-event.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/script-element.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-runtime-error.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-synthetic-errorevent.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/window-synthetic-event.html
/html/webappapis/scripting/events/event-handler-processing-algorithm-error/worker.html
|
One failing test for Chrome here, a bunch for Firefox. Won't test further, too time consuming, will wait for the improved tooling to materialize. |
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.
Quite comprehensive, changes possibly needed depending on discussion on spec side.
|
||
promise_test(t => { | ||
window.onerror = t.step_func((...args) => { | ||
assert_greater_than(args.length, 1); |
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.
Is there a separate test requiring the precise number? I take it this is to avoid failing in Edge because it has 6 arguments?
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.
Good point, I don't think there is one, or at least not an easy-to-find one. I'll add some.
return true; | ||
}); | ||
|
||
const eventWatcher = new EventWatcher(t, window, "error"); |
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.
Regarding my comments in whatwg/html#2398 about synthetic events, this with one other type would be the test for that behavior.
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.
Then it wouldn't trigger .onerror = ...
, which is what this section of the spec is about.
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.
Yes :)
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.
New tests LGTM, but I think there's still nothing that would fail if there are 6 arguments. I can't remember if that's actually the case anywhere, but I seem to remember an oddity in the number of arguments.
Right, Edge has only 4. Nobody has 6. I guess we might as well add more coverage though... |
Follows whatwg/html#2398; see also whatwg/html#2296.