159953 - Setting the same event handler with both addEventListener() and event property produces duplicate

Status: VERIFIED INVALID

(Core :: DOM: Events, defect)

Description

[tfriesen]

If you set the same event handler with both addEventListener() and the event
property,the handler gets called twice.
I believe this should be considered a duplicate and one instance should be
discarded.

Konqueror seems to handle this correctly.

Comment 1

[tfriesen]

Attachment: testcase:click document to see duplicate event call

Comment 2

[Vladimir Ermakov]

This seems like correct behaviour to me. You assign listeners twice, it gets
called twice. (.onclick is equivalent of addEventListener() ) If you want to
remove one of the listeners use the removeEventListener() method.

Comment 3

[tfriesen]

According to the specs this is not correct behavior.
Under the addEventListener() entry it says:

"If multiple identical EventListeners are registered on the same EventTarget
with the same parameters the duplicate instances are discarded. They do not
cause the EventListener to be called twice and since they are discarded they do
not need to be removed with the removeEventListener method."

I think is a valid bug.

Comment 4

[tfriesen]

Unless these two ways of listener registration are to be kept separate.
You can't remove a listener set with the event property using
removeEventListener() either.
But the specs make it sound like they are all one big happy family.

Comment 5

[tfriesen]

reopening this bug

Comment 6

[Boris Zbarsky [:bzbarsky]]

> But the specs make it sound like they are all one big happy family.

They do?  Where exactly do the specs describe what setting |.onclick| does?

In particular, what should the result be of running the following?

Case 1:

document.onclick = foo;
document.onclick = bar;

(I think we can agree that there should be one click listener on the document
now and it should be "bar").

Case 2:

document.onclick = foo;
document.addEventListener("click", foo, false);
document.onclick = bar;

What should happen here?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase setting an onsubmit handler via addEventListener

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase setting the same handler via .onsubmit

Comment 9

[Boris Zbarsky [:bzbarsky]]

These two testcases show that the the two methods of setting a handler are in
fact _different_.  The key here is that in the ECMAScript binding for DOM
Events, an EventListener is described as:

  This is an ECMAScript function reference. This method has no return value. The
  parameter is a Event object.

Whereas returning "false" from a function set as the onclick (or unsubmit)
attribute must cancel the event in DOM0.  So to make this work, setting
|.onclick = foo| must synthesize a function along the lines of:

function realListener (event) {
  if (!foo(event)) {
    event.stopPropagation();
  }
}

and attach it as an event listener to the node.  This listener is obviously
different from just using "foo" as a listener.

I would be interested in knowing how Konqueror handles the two testcases I
attached.....

Marking this invalid, since the event listeners involved are actually different.

Comment 10

[tfriesen]

Boy,you guys sure want to invalid this bug is a hurry,don't ya :-)

The specs don't speak of event properties per sec but in section 1.3.2
they do talk about the HTML attributes.
One sentence from that says:

"This EventListener behaves in the same manner as any other EventListeners which
may be registered on the EventTarget."

To me,this line says that all listeners (including properties) are the same and
they should all
behave as DOM2 listeners (as outlined in the specs).....is there something I
missed here?

If you used attributes instead of properties in Boris' testcases,the
bahavior would be the same (returning false works) and it should not be....event
attributes
should act like DOM2 listeners (according to the specs).

So either the bahavior in the browser need to change or the specs
need to be changed.
Because we have different browsers doing different things and that ain't good. 

OK,I'am reopening this sucker once more,if someone invalids it again Iam outta
here,this little transitional bug ain't worth it..but maybe someone has some
better answers out there

Comment 11

[tfriesen]

Attachment: test: setting onsubmit via attribute

Comment 12

[Boris Zbarsky [:bzbarsky]]

Ah... but the spec does not say that doing

window.onload = foo;

means that "foo" is registered as an event listener.   It just says that this
registers an event listener. If we take the view that the event listener object
should actually be "foo" in this case, then there is a fundamental
incompatibility between existing practice and the quoted section of the
specification.  Since the section exists solely to legitimize existing practice,
that seems like either a mistake in the spec or an incorrect interpretation of
the spec...

Essentially, a browser must fail one of the three testcases in this bug.  If it
fails yours, that is Mozilla's behavior Compliant with current practice and the
events specification's ECMA binding, contrary to an interpretation of vague
wording in section 1.3.2.  If it fails the first of my testcases, that is
Konqueror's behavior (I just installed it to test).  Compliant with current
practice and your interpretation of 1.3.2, but violating the ECMA bindings in a
big way.  If it fails the second of my testcases, then it will not work with any
real-world pages that actually set event handlers via attributes.  That would be
poor...

So essentially we have chosen to follow existing practice on event handler
attributes and follow the events spec.  This requires that the event listener
being attached when an attribute is set not be the function that is set as the
attribute.  But the spec allows that.

Does that count as a better answer?  ;)  

Comment 13

[Boris Zbarsky [:bzbarsky]]

In the case of the attribute thing, the listener is actually:

function listener(event) {
  function foo(event) {
     return lala(event);
  }
  
  if (!foo(event)) {
    event.stopPropagation();
    event.preventDefault();
  }
}

This is the only listener that will have the desired backwards-compatible
behavior that section 1.3.2 is striving to legitimize.

If you are saying that returning "false" in that testcase should _not_ prevent
the form submit, then you're basically saying that the DOM Events spec is
blindly ignoring how _every_ _single_ browser implements event attributes and
"standardizing" them a different way?  I doubt the spec authors are that stupid
(certainly those of them I know are not).

Comment 14

[tfriesen]

If it is all suppose to kept separate,I don't understand why they just
don't say that in the specs.
Trying to explain the relationship between the two like in section 1.3.2
is just confusing.
It would be better to just leave it out completely and say the it part of
DOM 0 and let implementors work it out for themselves. 
But whatever.....marking this bug invalid

Comment 15

[Vladimir Ermakov]

verifying