New issue
Advanced search Search tips

Issue 712778 link

Starred by 2 users
Status: Fixed
Owner: ----
Closed: Oct 2017
Cc:
jbroman@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

addEventListener accepts craziness for the listener

Project Member Reported by rdevlin....@chromium.org, Apr 18 2017 
Chrome Version: (copy from chrome://version)
OS: (e.g. Win7, OSX 10.9.5, etc...)

What steps will reproduce the problem?
(1) open a developer console
(2) `addEventListener('something', 1);

What is the expected result?
Some kind of type error

What happens instead?
Nothing

Please use labels and text to provide additional information.
The spec for addEventListener specifies that listener must be either a function that implements the EventListener interface (void handleEvent) or a function.  `1` is neither of those.  It seems like we should be able to throw an error on this unless we want to support

Number.prototype.handleEvent = ...;
addEventListener('something', 1);

(Which I'm hoping we don't.)

Comment 1 by jbroman@chromium.org, Apr 18 2017 

This seems to be because V8EventListenerHelper::GetEventListener doesn't throw an exception in this case (but just returns nullptr, which is passed to addEventListener).

My reading of the WebIDL spec suggests we should throw a TypeError in this case (because the type is not object), and this is what Firefox nightly, at least, does.

Because addEventListener is a super important API, it might be worth talking to API owners before changing this (we might want a quick UseCounter to check).

Comment 2 by yukishiino@chromium.org, Apr 19 2017

Labels: -Pri-3 Pri-2
Status: Available (was: Untriaged)

Comment 3 by bhagirat...@samsung.com, Sep 20 2017 

I am working on this issue.
Work is in progress: https://chromium-review.googlesource.com/c/chromium/src/+/672623
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ca8b3be7d04a0fde6519125f212c64839a4212b

commit 1ca8b3be7d04a0fde6519125f212c64839a4212b
Author: Bhagirathi Satpathy <bhagirathi.s@samsung.com>
Date: Thu Oct 05 09:25:14 2017

addEventListener accepts craziness for the listener

Added exception when listener (second parameter in addEventListener)
is not an object to match the specification.

EX: document.getElementById("id").addEventListener('something', 1);

Bug:  712778 
Change-Id: I2b1dca04f08ffe67c3a48283caf781ec8b373975
Reviewed-on: https://chromium-review.googlesource.com/672623
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#506690}
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/chrome/test/data/webui/print_preview/print_preview_tests.js
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/content/test/data/loader/async_resource_handler.html
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/third_party/WebKit/LayoutTests/editing/selection/mouse/mouse_up_focus.html
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/third_party/WebKit/LayoutTests/http/tests/misc/webtiming-cross-origin-and-back.html
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/third_party/WebKit/LayoutTests/images/image-document-remove-listener.html
[modify] https://crrev.com/1ca8b3be7d04a0fde6519125f212c64839a4212b/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl

Comment 5 by sriram...@samsung.com, Oct 13 2017

Status: Fixed (was: Available)
►

Sign in to add a comment