New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 759461 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 651762



Sign in to add a comment

|onerror| event handler function should have fixed name arguments

Project Member Reported by tkent@chromium.org, Aug 28 2017

Issue description

Chrome Version: 62 canary
OS: All

What steps will reproduce the problem?
(1) open http://w3c-test.org/html/webappapis/scripting/processing-model-2/body-onerror-runtime-error.html

What is the expected result?
No FAIL tests

What happens instead?
Two FAIL tests

Please use labels and text to provide additional information.
Spec:
https://html.spec.whatwg.org/multipage/webappapis.html#getting-the-current-value-of-the-event-handler

> 10. Let function be the result of calling FunctionCreate, with arguments:

> If H is an onerror event handler of a Window object
>   Let the function have five arguments, named event, source, lineno, colno, and error.

V8ErrorHandler.cpp prepares five arguments correctly when calling a handler.
Maybe we should do something when we compile error event handler?


Edge and Firefox work correctly.  Safari has the same issue.

 
Status: Available (was: Untriaged)
Looks like we need a counterpart of V8LazyEventListener for V8ErrorHandler.
Cc: yukishiino@chromium.org
Labels: Hotlist-GoodFirstBug
cc yukishiino: if you want to look at this as an interop issue

Hotlist-GoodFirstBug as this doesn't seem terribly complicated to implement

Comment 3 by ratsu...@gmail.com, Sep 14 2017

Hi everyone, I'm trying work on this issue.
Re: #3, Good news.  Feel free to send us patches.

Comment 5 by ratsu...@gmail.com, Dec 4 2017

To the people who may want to work on this issue:

I've tried to make a rough patch for this issue, you can see it at
https://chromium-review.googlesource.com/c/chromium/src/+/801191

But as yukishiino@ said in comment, it's not good enough for merge, please read the comment and continue your good work

Cheers
Cc: yukiy@google.com
Cc: -yukiy@google.com
Owner: yukiy@google.com
Status: Started (was: Available)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 5

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

commit 1b2f95835607dbb9c6021df8893fb18f4c7aee9e
Author: Yuki Yamada <yukiy@google.com>
Date: Fri Oct 05 09:20:48 2018

Create new EventHandler and base class for EventListener/EventHandler

This CL implements these classes in Blink:
JSBasedEventListener                       (The base class for
 |                                          EventLisntener/EventHandler)
 +- JSEventListener                        (Implements EventListener)
 +- JSEventHandler                         (Implements EventHandler and
     |                                      other special types)
     +- JSEventHandlerForContentAttribute  (Supports lazy compilation
                                            for content attribute)
EventHandlers start to support incumbent realm using
blink::V8EventHandlerNonNull that inherits blink::CallbackFunctionBase.
(EventListener already supports incumbent realm.)

These four classes are no longer referenced from anywhere:
  blink::V8AbstractEventListener
  blink::V8EventListenerOrEventHandler
  blink::V8ErrorHandler
  blink::V8LazyEventListener
Although these are no longer necessary, this CL does not remove them
so that we can easily switch which classes to use in case new classes
contains some problems.

regarding  crbug.com/759461  :
OnErrorEventHandler for content attribute, which has different number
of arguments, gets supported by blink::JSEventHandler and
blink::JSEventHandlerForContentAttribute.

regarding  crbug.com/664676  :
|js_event| in the relevant realm of |event|'s target is created in
blink::JSBasedEventListener::handleEvent().

Bug:  872138 ,  849236 ,  759461 ,  664676 ,  606900 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I356734fc16611277ad9f3f279a676e7dc8083408
Reviewed-on: https://chromium-review.googlesource.com/c/1215512
Commit-Queue: Yuki Yamada <yukiy@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597054}
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/events/onerroreventhandler-expected.txt
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/processing-model-2/body-onerror-compile-error-data-url-expected.txt
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/processing-model-2/body-onerror-compile-error-expected.txt
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/processing-model-2/body-onerror-runtime-error-expected.txt
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/WebKit/LayoutTests/fast/events/before-unload-return-bad-value.html
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/WebKit/LayoutTests/fast/events/window-onerror-11-expected.txt
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/WebKit/LayoutTests/fast/performance/detached-event-timestamp.html
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/inline-event-handler-blocked-after-injecting-meta-expected.txt
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/bindings.gni
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/BUILD.gn
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_based_event_listener.cc
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_based_event_listener.h
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_handler.cc
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_handler.h
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_handler_for_content_attribute.cc
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_handler_for_content_attribute.h
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_listener.cc
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/js_event_listener.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/script_event_listener.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/script_event_listener.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/v8_abstract_event_handler.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/v8_event_listener_helper.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/v8_event_listener_helper.h
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/blink/renderer/bindings/core/v8/v8_event_listener_impl.cc
[delete] https://crrev.com/2555486fd96872f6b98f9ad54c541d7ee2b6fcb1/third_party/blink/renderer/bindings/core/v8/v8_event_listener_impl.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/v8_event_listener_or_event_handler.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/core/v8/v8_lazy_event_listener.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/scripts/v8_types.py
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_node.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/core_idl_files.gni
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/dom/events/event_listener.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/dom/events/event_listener_map.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/dom/events/event_target.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/dom/events/event_target.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/frame/dom_window.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/frame/dom_window.h
[add] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/html/event_handler.idl
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/html/html_body_element.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/html/html_frame_element_base.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/html/html_frame_set_element.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/inspector/thread_debugger.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/svg/svg_script_element.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/svg/svg_svg_element.cc
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/1b2f95835607dbb9c6021df8893fb18f4c7aee9e/third_party/blink/renderer/platform/bindings/v8_private_property.h

Status: Fixed (was: Started)

Sign in to add a comment