New issue
Advanced search Search tips

Issue 759449 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 651762



Sign in to add a comment

Updating attribute event handler should replace the existing attribute event listener

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

Issue description

Chrome Version: 62 canary
OS: All but iOS

What steps will reproduce the problem?
(1) Open http://w3c-test.org/html/webappapis/scripting/events/event-handler-spec-example.html

What is the expected result?
No "FAIL" tests

What happens instead?
All tests fail.

Please use labels and text to provide additional information.

https://html.spec.whatwg.org/multipage/webappapis.html#event-handler-attributes:event-handlers-12

> When an event handler H of an element or object T ... is first set to a non-null value, the user agent must append an event listener to the list of event listeners associated with T ...

"first set to a non-null" is important.  If we change non-null value to another non-null value, we should not *append*, but should "update".

I think this is a bug of EventTarget::SetAttributeEventListener(), which deletes the existing event listener for the handler, and then append new event listener for the handler.

Edge, Firefox, and Safari work correctly.

 

Comment 1 by tkent@chromium.org, Aug 28 2017

Summary: Updating attribute event handler should replace the existing attribute event listener (was: Updating attribute event listener should replace the existing attribute event listener)
I shall work on this issue. I ll analyze more on EventTarget::SetAttributeEventListener() and update.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14 2017

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

commit 575e50d9534272dfa5ac835ba11a349e30001ec2
Author: Bhagirathi Satpathy <bhagirathi.s@samsung.com>
Date: Thu Sep 14 01:07:45 2017

Updating attribute event handler should replace the existing attribute event listener

We should not remove the old existing attribute event listener
and then appending the new listener.Instead actually we should
replace the existing attribute event listener with new one.

Bug:  759449 
Change-Id: I4bdbd8935dc47dc2fc9f735dee92b1310868312d
Reviewed-on: https://chromium-review.googlesource.com/662087
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501820}
[delete] https://crrev.com/f1dc344fa6a3730b9872a50bf24e8f4a849cbd9a/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/events/event-handler-spec-example-expected.txt
[delete] https://crrev.com/f1dc344fa6a3730b9872a50bf24e8f4a849cbd9a/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/events/inline-event-handler-ordering-expected.txt
[modify] https://crrev.com/575e50d9534272dfa5ac835ba11a349e30001ec2/third_party/WebKit/Source/core/dom/events/EventTarget.cpp
[modify] https://crrev.com/575e50d9534272dfa5ac835ba11a349e30001ec2/third_party/WebKit/Source/core/dom/events/EventTarget.h
[modify] https://crrev.com/575e50d9534272dfa5ac835ba11a349e30001ec2/third_party/WebKit/Source/core/events/RegisteredEventListener.h

Comment 4 by tkent@chromium.org, Sep 14 2017

Labels: M-63
Status: Fixed (was: Available)

Sign in to add a comment