Commit d2556706 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Throw TypeError instead of InvalidStateError for custom elements

After [1], the correct error to throw when upgrading a custom
element is now TypeError, rather than InvalidStateError. Firefox
and Safari have already changed to this, Chrome is now the
odd-one-out.

[1] https://github.com/whatwg/html/pull/4525

Fixed: 1057423
Change-Id: Ifebcbc5823370a05cbebecf3ddf39d6ecbe260b6
Bug: 1057423
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2082086
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746476}
parent 0127d426
......@@ -219,15 +219,13 @@ bool ScriptCustomElementDefinition::RunConstructor(Element& element) {
if (try_catch.HasCaught())
return false;
// To report InvalidStateError Exception, when the constructor returns some
// different object
// Report a TypeError Exception if the constructor returns a different object.
if (result != &element) {
const String& message =
"custom element constructors must call super() first and must "
"not return a different object";
v8::Local<v8::Value> exception = V8ThrowDOMException::CreateOrEmpty(
script_state_->GetIsolate(), DOMExceptionCode::kInvalidStateError,
message);
v8::Local<v8::Value> exception =
V8ThrowException::CreateTypeError(script_state_->GetIsolate(), message);
if (!exception.IsEmpty())
V8ScriptRunner::ReportException(isolate, exception);
return false;
......
......@@ -128,8 +128,7 @@ void V8HTMLConstructor::HtmlConstructor(
// During upgrade an element has invoked the same constructor
// before calling 'super' and that invocation has poached the
// element.
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
"this instance is already constructed");
exception_state.ThrowTypeError("This instance is already constructed");
return;
}
}
......
......@@ -61,9 +61,7 @@ test_with_window((w) => {
'the recursive "new" should steal the upgrade candidate');
assert_equals(poacher, invocations[0],
'the recursize "new" should happen first');
assert_true(invocations[1] instanceof w.DOMException,
'the super call should have thrown a DOMException');
assert_equals(invocations[1].name, 'InvalidStateError',
'the exception should be an InvalidStateError');
assert_true(invocations[1] instanceof w.TypeError,
'the super call should have thrown a TypeError');
}, 'HTMLElement constructor: poach upgrade candidate');
</script>
......@@ -146,7 +146,7 @@ test_with_window((w) => {
}
w.customElements.define('a-a', A);
let d = w.document.createElement('div');
assert_reports(window, 'INVALID_STATE_ERR', () => {
assert_reports_js(window, TypeError, () => {
d.innerHTML = '<a-a>';
}, 'Creating an element that is already constructed marker should report ' +
'InvalidStateError');
......@@ -171,7 +171,7 @@ test_with_window((w) => {
}
w.customElements.define('a-a', A);
w.document.body.appendChild(e);
assert_array_equals(errors, ['InvalidStateError'], 'Upgrading an element ' +
'that is already constructed marker should throw InvalidStateError');
assert_array_equals(errors, ['TypeError'], 'Upgrading an element ' +
'that is already constructed marker should throw TypeError');
}, 'Already constructed marker, upgrade element');
</script>
......@@ -147,11 +147,6 @@ function assert_type_error_event(error) {
assert_equals(error.error.name, 'TypeError');
}
function assert_invalid_state_dom_error_event(error) {
assert_not_muted_error_event(error);
assert_equals(error.error.name, 'InvalidStateError');
}
test_with_window((w) => {
// "create an element" 6.1.3, throw a TypeError.
// https://dom.spec.whatwg.org/#concept-create-element
......@@ -160,10 +155,10 @@ test_with_window((w) => {
constructor_returns_bad_object + instantiate);
test_with_window((w) => {
// "upgrade an element" 10, throw an InvalidStateError DOMException.
// "upgrade an element" 10, throw a TypeError.
// https://html.spec.whatwg.org/multipage/scripting.html#upgrades
w.document.body.innerHTML = instantiate;
assert_invalid_state_dom_error_event(w.errors[0]);
assert_type_error_event(w.errors[0]);
}, 'Upgrade reaction invokes the constructor that returns a bad object',
constructor_returns_bad_object);
</script>
......
......@@ -70,3 +70,26 @@ function assert_reports(w, expected_error, func, description) {
// accessed by throwing the error
assert_throws(expected_error, () => { throw errors[0]; });
}
// Asserts that func synchronously invokes the error event handler in w
// with the expected error.
function assert_reports_js(w, expected_error, func, description) {
let old_onerror = w.onerror;
let errors = [];
w.onerror = (event, source, line_number, column_number, error) => {
errors.push(error);
return true; // the error is handled
};
try {
func();
} catch (e) {
assert_unreached(`should report, not throw, an exception: ${e}`);
} finally {
w.onerror = old_onerror;
}
assert_equals(errors.length, 1, 'only one error should have been reported');
assert_true(
typeof errors[0] === 'object' && errors[0] !== null,
'got something other than an error');
assert_equals(expected_error.name, errors[0].name);
}
......@@ -115,7 +115,7 @@ test_with_window((w) => {
}
}
w.customElements.define('a-a', X);
assert_array_equals(error_log, ['InvalidStateError'], 'returning object that is different from element should throw "InvalidStateError"');
assert_array_equals(error_log, ['TypeError'], 'returning object that is different from element should throw "TypeError"');
}, 'Upgrading an element with constructor that returns different object');
</script>
</body>
This is a testharness.js-based test.
PASS Node.prototype.cloneNode(false) must be able to clone a custom element
PASS Node.prototype.cloneNode(false) must be able to clone as a autonomous custom element when it contains is attribute
PASS Node.prototype.cloneNode(false) must be able to clone as a customized built-in element when it has an inconsistent "is" attribute
PASS Node.prototype.cloneNode(false) must be able to clone a custom element inside an iframe
PASS Node.prototype.cloneNode(true) must be able to clone a descendent custom element
PASS Node.prototype.cloneNode(true) must set parentNode, previousSibling, and nextSibling before upgrading custom elements
FAIL HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL Upgrading a custom element must throw TypeError when the custom element's constructor returns another element assert_equals: expected "TypeError" but got "InvalidStateError"
PASS Inserting an element must not try to upgrade a custom element when it had already failed to upgrade once
Harness: the test ran to completion.
This is a testharness.js-based test.
PASS Element.prototype.createElement must add an unresolved custom element to the upgrade candidates map
FAIL HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself after super() call assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL HTMLElement constructor must throw an TypeError when the top of the construction stack is marked AlreadyConstructed due to a custom element constructor constructing itself before super() call assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL Upgrading a custom element must throw an TypeError when the returned element is not SameValue as the upgraded element assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL Upgrading a custom element whose constructor returns a Text node must throw assert_equals: expected "TypeError" but got "InvalidStateError"
FAIL Upgrading a custom element whose constructor returns an Element must throw assert_equals: expected "TypeError" but got "InvalidStateError"
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment