Commit beb1b77d authored by philipj's avatar philipj Committed by Commit bot

Make addEventListener/removeEventListener arguments non-optional

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/3rsQQJvhY8k/P2cAFq3hAwAJ

BUG=353484

Review URL: https://codereview.chromium.org/1461993002

Cr-Commit-Position: refs/heads/master@{#361316}
parent cdbfa816
......@@ -3,10 +3,10 @@ Test addEventListener() and removeEventListener() fail silently if arguments are
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS window.addEventListener("foo") is undefined
PASS window.removeEventListener("bar") is undefined
PASS window.addEventListener() is undefined
PASS window.removeEventListener() is undefined
PASS window.addEventListener("foo") threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS window.removeEventListener("bar") threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS window.addEventListener() threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS window.removeEventListener() threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS successfullyParsed is true
TEST COMPLETE
......
......@@ -7,11 +7,11 @@
<script>
description("Test addEventListener() and removeEventListener() fail silently if arguments are missing.");
shouldBe('window.addEventListener("foo")', 'undefined');
shouldBe('window.removeEventListener("bar")', 'undefined');
shouldThrow('window.addEventListener("foo")');
shouldThrow('window.removeEventListener("bar")');
shouldBe('window.addEventListener()', 'undefined');
shouldBe('window.removeEventListener()', 'undefined');
shouldThrow('window.addEventListener()');
shouldThrow('window.removeEventListener()');
</script>
</body>
</html>
......@@ -3,10 +3,10 @@ Test addEventListener() and removeEventListener() fail silently if arguments are
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS new XMLHttpRequest().addEventListener("foo") is undefined
PASS new XMLHttpRequest().removeEventListener("bar") is undefined
PASS new XMLHttpRequest().addEventListener() is undefined
PASS new XMLHttpRequest().removeEventListener() is undefined
PASS new XMLHttpRequest().addEventListener("foo") threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS new XMLHttpRequest().removeEventListener("bar") threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS new XMLHttpRequest().addEventListener() threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS new XMLHttpRequest().removeEventListener() threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS successfullyParsed is true
TEST COMPLETE
......
......@@ -7,11 +7,11 @@
<script>
description("Test addEventListener() and removeEventListener() fail silently if arguments are missing.");
shouldBe('new XMLHttpRequest().addEventListener("foo")', 'undefined');
shouldBe('new XMLHttpRequest().removeEventListener("bar")', 'undefined');
shouldThrow('new XMLHttpRequest().addEventListener("foo")');
shouldThrow('new XMLHttpRequest().removeEventListener("bar")');
shouldBe('new XMLHttpRequest().addEventListener()', 'undefined');
shouldBe('new XMLHttpRequest().removeEventListener()', 'undefined');
shouldThrow('new XMLHttpRequest().addEventListener()');
shouldThrow('new XMLHttpRequest().removeEventListener()');
</script>
</body>
</html>
......@@ -4,47 +4,35 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
Signature:
void addEventListener(DOMString type, EventListener listener, optional boolean useCapture)
PASS internals.isUseCounted(document, AddEventListenerNoArguments) is false
PASS document.addEventListener() is undefined
PASS internals.isUseCounted(document, AddEventListenerNoArguments) is true
PASS internals.isUseCounted(document, AddEventListenerOneArgument) is false
PASS document.addEventListener("foo") is undefined
PASS internals.isUseCounted(document, AddEventListenerOneArgument) is true
void addEventListener(DOMString type, EventListener? listener, optional boolean useCapture)
PASS document.addEventListener() threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS document.addEventListener("foo") threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS document.addEventListener("foo", listener) did not throw exception.
PASS document.addEventListener("", listener) did not throw exception.
PASS document.addEventListener("", function(){}) did not throw exception.
PASS document.addEventListener("bar", listener, false) did not throw exception.
PASS document.addEventListener("bar", listener, true) did not throw exception.
PASS document.addEventListener(null) is undefined
PASS document.addEventListener(null, listener) did not throw exception.
PASS document.addEventListener("foo", null) is undefined
PASS document.addEventListener("foo", null) did not throw exception.
PASS document.addEventListener("zork", listener, null) did not throw exception.
PASS document.addEventListener(undefined) is undefined
PASS document.addEventListener(undefined, listener) is undefined
PASS document.addEventListener("foo", undefined) is undefined
PASS document.addEventListener(undefined, listener) did not throw exception.
PASS document.addEventListener("foo", undefined) did not throw exception.
PASS document.addEventListener("zork", listener, undefined) did not throw exception.
Signature:
void removeEventListener(DOMString type, EventListener listener, optional boolean useCapture)
PASS internals.isUseCounted(document, RemoveEventListenerNoArguments) is false
PASS document.removeEventListener() is undefined
PASS internals.isUseCounted(document, RemoveEventListenerNoArguments) is true
PASS internals.isUseCounted(document, RemoveEventListenerOneArgument) is false
PASS document.removeEventListener("foo") is undefined
PASS internals.isUseCounted(document, RemoveEventListenerOneArgument) is true
void removeEventListener(DOMString type, EventListener? listener, optional boolean useCapture)
PASS document.removeEventListener() threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS document.removeEventListener("foo") threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS document.removeEventListener("foo", listener) did not throw exception.
PASS document.removeEventListener("foo", listener, true) did not throw exception.
PASS document.removeEventListener("bar", listener, false) did not throw exception.
PASS document.removeEventListener("bar", listener, false) did not throw exception.
PASS document.removeEventListener("bar", listener, true) did not throw exception.
PASS document.removeEventListener(null) is undefined
PASS document.removeEventListener(null, listener) did not throw exception.
PASS document.removeEventListener("foo", null) is undefined
PASS document.removeEventListener("foo", null) did not throw exception.
PASS document.removeEventListener("zork", listener, null) did not throw exception.
PASS document.removeEventListener(undefined) is undefined
PASS document.removeEventListener(undefined, listener) is undefined
PASS document.removeEventListener("foo", undefined) is undefined
PASS document.removeEventListener(undefined, listener) did not throw exception.
PASS document.removeEventListener("foo", undefined) did not throw exception.
PASS document.removeEventListener("zork", listener, undefined) did not throw exception.
PASS successfullyParsed is true
......
......@@ -9,75 +9,39 @@ function listener(event)
}
debug('Signature:')
debug('void addEventListener(DOMString type, EventListener listener, optional boolean useCapture)');
// FIXME: should throw on missing arguments: http://crbug.com/353484
// shouldThrow('document.addEventListener()');
// shouldThrow('document.addEventListener("foo")');
var AddEventListenerNoArguments = 656;
shouldBeFalse('internals.isUseCounted(document, AddEventListenerNoArguments)');
shouldBe('document.addEventListener()', 'undefined');
shouldBeTrue('internals.isUseCounted(document, AddEventListenerNoArguments)');
var AddEventListenerOneArgument = 657;
shouldBeFalse('internals.isUseCounted(document, AddEventListenerOneArgument)');
shouldBe('document.addEventListener("foo")', 'undefined');
shouldBeTrue('internals.isUseCounted(document, AddEventListenerOneArgument)');
debug('void addEventListener(DOMString type, EventListener? listener, optional boolean useCapture)');
shouldThrow('document.addEventListener()');
shouldThrow('document.addEventListener("foo")');
shouldNotThrow('document.addEventListener("foo", listener)');
shouldNotThrow('document.addEventListener("", listener)');
shouldNotThrow('document.addEventListener("", function(){})');
shouldNotThrow('document.addEventListener("bar", listener, false)');
shouldNotThrow('document.addEventListener("bar", listener, true)');
// null
shouldBe('document.addEventListener(null)', 'undefined');
shouldNotThrow('document.addEventListener(null, listener)'); // converted to "null"
// FIXME: throw on |null|: http://crbug.com/249598
// shouldThrow('document.addEventListener("foo", null)');
shouldBe('document.addEventListener("foo", null)', 'undefined');
shouldNotThrow('document.addEventListener(null, listener)');
shouldNotThrow('document.addEventListener("foo", null)');
shouldNotThrow('document.addEventListener("zork", listener, null)');
// undefined
// FIXME: behavior of undefined for mandatory arguments is unclear, but
// probably should throw
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=23532
shouldBe('document.addEventListener(undefined)', 'undefined');
// shouldThrow('document.addEventListener(undefined, listener)');
// shouldThrow('document.addEventListener("foo", undefined)');
shouldBe('document.addEventListener(undefined, listener)', 'undefined');
shouldBe('document.addEventListener("foo", undefined)', 'undefined');
shouldNotThrow('document.addEventListener(undefined, listener)');
shouldNotThrow('document.addEventListener("foo", undefined)');
shouldNotThrow('document.addEventListener("zork", listener, undefined)');
debug('');
debug('Signature:');
debug('void removeEventListener(DOMString type, EventListener listener, optional boolean useCapture)');
// FIXME: should throw on missing arguments: http://crbug.com/353484
// shouldThrow('document.removeEventListener()');
// shouldThrow('document.removeEventListener("foo")');
var RemoveEventListenerNoArguments = 658;
shouldBeFalse('internals.isUseCounted(document, RemoveEventListenerNoArguments)');
shouldBe('document.removeEventListener()', 'undefined');
shouldBeTrue('internals.isUseCounted(document, RemoveEventListenerNoArguments)');
var RemoveEventListenerOneArgument = 659;
shouldBeFalse('internals.isUseCounted(document, RemoveEventListenerOneArgument)');
shouldBe('document.removeEventListener("foo")', 'undefined');
shouldBeTrue('internals.isUseCounted(document, RemoveEventListenerOneArgument)');
debug('void removeEventListener(DOMString type, EventListener? listener, optional boolean useCapture)');
shouldThrow('document.removeEventListener()');
shouldThrow('document.removeEventListener("foo")');
shouldNotThrow('document.removeEventListener("foo", listener)');
shouldNotThrow('document.removeEventListener("foo", listener, true)');
shouldNotThrow('document.removeEventListener("bar", listener, false)');
shouldNotThrow('document.removeEventListener("bar", listener, false)');
shouldNotThrow('document.removeEventListener("bar", listener, true)');
// null
shouldBe('document.removeEventListener(null)', 'undefined');
shouldNotThrow('document.removeEventListener(null, listener)'); // converted to "null"
// FIXME: throw on |null|: http://crbug.com/249598
// shouldThrow('document.removeEventListener("foo", null)');
shouldBe('document.removeEventListener("foo", null)', 'undefined');
shouldNotThrow('document.removeEventListener(null, listener)');
shouldNotThrow('document.removeEventListener("foo", null)');
shouldNotThrow('document.removeEventListener("zork", listener, null)');
// undefined
// FIXME: behavior of undefined for mandatory arguments is unclear, but
// probably should throw
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=23532
shouldBe('document.removeEventListener(undefined)', 'undefined');
// shouldthrow('document.removeeventlistener("foo", undefined)');
// shouldthrow('document.removeeventlistener(undefined, listener)');
shouldBe('document.removeEventListener(undefined, listener)', 'undefined');
shouldBe('document.removeEventListener("foo", undefined)', 'undefined');
shouldNotThrow('document.removeEventListener(undefined, listener)');
shouldNotThrow('document.removeEventListener("foo", undefined)');
shouldNotThrow('document.removeEventListener("zork", listener, undefined)');
</script>
......@@ -3,10 +3,10 @@ Test addEventListener() and removeEventListener() fail silently if arguments are
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS document.addEventListener("foo") is undefined
PASS document.removeEventListener("bar") is undefined
PASS document.addEventListener() is undefined
PASS document.removeEventListener() is undefined
PASS document.addEventListener("foo") threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS document.removeEventListener("bar") threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 1 present..
PASS document.addEventListener() threw exception TypeError: Failed to execute 'addEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS document.removeEventListener() threw exception TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 0 present..
PASS successfullyParsed is true
TEST COMPLETE
......
......@@ -7,11 +7,11 @@
<script>
description("Test addEventListener() and removeEventListener() fail silently if arguments are missing.");
shouldBe('document.addEventListener("foo")', 'undefined');
shouldBe('document.removeEventListener("bar")', 'undefined');
shouldThrow('document.addEventListener("foo")');
shouldThrow('document.removeEventListener("bar")');
shouldBe('document.addEventListener()', 'undefined');
shouldBe('document.removeEventListener()', 'undefined');
shouldThrow('document.addEventListener()');
shouldThrow('document.removeEventListener()');
</script>
</body>
</html>
......@@ -7,7 +7,8 @@ PASS window.confirm.length is 0
PASS window.open.length is 2
PASS window.setTimeout.length is 1
PASS window.clearTimeout.length is 0
PASS window.addEventListener.length is 0
PASS window.addEventListener.length is 2
PASS window.removeEventListener.length is 2
PASS window.postMessage.length is 2
PASS window.dispatchEvent.length is 1
PASS window.openDatabase.length is 4
......
......@@ -11,9 +11,8 @@ shouldBe('window.confirm.length', '0');
shouldBe('window.open.length', '2');
shouldBe('window.setTimeout.length', '1');
shouldBe('window.clearTimeout.length', '0');
// addEventListener.length should be 2, but legacy content calls with 0 or 1
// argument. See http://crbug.com/249598
shouldBe('window.addEventListener.length', '0');
shouldBe('window.addEventListener.length', '2');
shouldBe('window.removeEventListener.length', '2');
shouldBe('window.postMessage.length', '2');
shouldBe('window.dispatchEvent.length', '1');
shouldBe('window.openDatabase.length', '4');
......
......@@ -41,10 +41,6 @@ namespace {
void addEventListenerMethodPrologueCustom(const v8::FunctionCallbackInfo<v8::Value>& info, EventTarget*)
{
if (info.Length() < 2) {
UseCounter::countIfNotPrivateScript(info.GetIsolate(), callingExecutionContext(info.GetIsolate()),
info.Length() == 0 ? UseCounter::AddEventListenerNoArguments : UseCounter::AddEventListenerOneArgument);
}
if (info.Length() >= 3 && info[2]->IsObject()) {
UseCounter::countIfNotPrivateScript(info.GetIsolate(), callingExecutionContext(info.GetIsolate()),
UseCounter::AddEventListenerThirdArgumentIsObject);
......@@ -59,10 +55,6 @@ void addEventListenerMethodEpilogueCustom(const v8::FunctionCallbackInfo<v8::Val
void removeEventListenerMethodPrologueCustom(const v8::FunctionCallbackInfo<v8::Value>& info, EventTarget*)
{
if (info.Length() < 2) {
UseCounter::countIfNotPrivateScript(info.GetIsolate(), callingExecutionContext(info.GetIsolate()),
info.Length() == 0 ? UseCounter::RemoveEventListenerNoArguments : UseCounter::RemoveEventListenerOneArgument);
}
if (info.Length() >= 3 && info[2]->IsObject()) {
UseCounter::countIfNotPrivateScript(info.GetIsolate(), callingExecutionContext(info.GetIsolate()),
UseCounter::RemoveEventListenerThirdArgumentIsObject);
......@@ -80,6 +72,11 @@ void removeEventListenerMethodEpilogueCustom(const v8::FunctionCallbackInfo<v8::
void V8EventTarget::addEventListenerMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
{
ExceptionState exceptionState(ExceptionState::ExecutionContext, "addEventListener", "EventTarget", info.Holder(), info.GetIsolate());
if (UNLIKELY(info.Length() < 2)) {
setMinimumArityTypeError(exceptionState, 2, info.Length());
exceptionState.throwIfNeeded();
return;
}
EventTarget* impl = V8EventTarget::toImpl(info.Holder());
if (LocalDOMWindow* window = impl->toDOMWindow()) {
if (!BindingSecurity::shouldAllowAccessToFrame(info.GetIsolate(), callingDOMWindow(info.GetIsolate()), window->frame(), exceptionState)) {
......@@ -89,22 +86,14 @@ void V8EventTarget::addEventListenerMethodCustom(const v8::FunctionCallbackInfo<
if (!window->document())
return;
}
V8StringResource<TreatNullAsNullString> type;
V8StringResource<> type;
RefPtrWillBeRawPtr<EventListener> listener;
EventListenerOptionsOrBoolean options;
{
if (!info[0]->IsUndefined()) {
type = info[0];
if (!type.prepare())
return;
} else {
type = nullptr;
}
if (!info[1]->IsUndefined()) {
listener = V8EventListenerList::getEventListener(ScriptState::current(info.GetIsolate()), info[1], false, ListenerFindOrCreate);
} else {
listener = nullptr;
}
// TODO(dtapuska): This custom binding code can be eliminated once
// EventListenerOptions runtime enabled feature is removed.
// http://crbug.com/545163
......@@ -126,6 +115,11 @@ void V8EventTarget::addEventListenerMethodCustom(const v8::FunctionCallbackInfo<
void V8EventTarget::removeEventListenerMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
{
ExceptionState exceptionState(ExceptionState::ExecutionContext, "removeEventListener", "EventTarget", info.Holder(), info.GetIsolate());
if (UNLIKELY(info.Length() < 2)) {
setMinimumArityTypeError(exceptionState, 2, info.Length());
exceptionState.throwIfNeeded();
return;
}
EventTarget* impl = V8EventTarget::toImpl(info.Holder());
if (LocalDOMWindow* window = impl->toDOMWindow()) {
if (!BindingSecurity::shouldAllowAccessToFrame(info.GetIsolate(), callingDOMWindow(info.GetIsolate()), window->frame(), exceptionState)) {
......@@ -135,22 +129,14 @@ void V8EventTarget::removeEventListenerMethodCustom(const v8::FunctionCallbackIn
if (!window->document())
return;
}
V8StringResource<TreatNullAsNullString> type;
V8StringResource<> type;
RefPtrWillBeRawPtr<EventListener> listener;
EventListenerOptionsOrBoolean options;
{
if (!info[0]->IsUndefined()) {
type = info[0];
if (!type.prepare())
return;
} else {
type = nullptr;
}
if (!info[1]->IsUndefined()) {
listener = V8EventListenerList::getEventListener(ScriptState::current(info.GetIsolate()), info[1], false, ListenerFindOnly);
} else {
listener = nullptr;
}
// TODO(dtapuska): This custom binding code can be eliminated once
// EventListenerOptions runtime enabled feature is removed.
// http://crbug.com/545163
......
......@@ -25,10 +25,7 @@
WillBeGarbageCollected,
Exposed=(Window,Worker)
] interface EventTarget {
// FIXME: first 2 args should be required, but throwing TypeError breaks
// legacy content. http://crbug.com/353484
// FIXME: type should not be nullable.
[Custom] void addEventListener(optional DOMString? type = null, optional EventListener? listener = null, optional (EventListenerOptions or boolean) options);
[Custom] void removeEventListener(optional DOMString? type = null, optional EventListener? listener = null, optional (EventListenerOptions or boolean) options);
[Custom] void addEventListener(DOMString type, EventListener? listener, optional (EventListenerOptions or boolean) options);
[Custom] void removeEventListener(DOMString type, EventListener? listener, optional (EventListenerOptions or boolean) options);
[ImplementedAs=dispatchEventForBindings, RaisesException] boolean dispatchEvent(Event event);
};
......@@ -542,10 +542,6 @@ public:
OfflineAudioContext = 653,
PrefixedAudioContext = 654,
PrefixedOfflineAudioContext = 655,
AddEventListenerNoArguments = 656,
AddEventListenerOneArgument = 657,
RemoveEventListenerNoArguments = 658,
RemoveEventListenerOneArgument = 659,
MixedContentInNonHTTPSFrameThatRestrictsMixedContent = 661,
MixedContentInSecureFrameThatDoesNotRestrictMixedContent = 662,
MixedContentWebSocket = 663,
......
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