Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #6497 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

[PATCH] bindAsEventListener duplicated argument

Reported by: wiktor Assigned to: sam
Priority: high Milestone: 1.x
Component: Prototype Version:
Severity: minor Keywords: event
Cc: mislav

Description

The event object will be passed two times to the bound function if you use bindAsEventListener.

I made an example page. http://spacergif.net/2006/10/26/bindasevenetlistener-arguments.html

I am going to write a detailed post about this issue, but I have to translate it. I hope it will be ready today. :)

Attachments

duplicated_event_object_arguments.diff (481 bytes) - added by wiktor on 10/26/06 11:44:20.

Change History

10/26/06 11:44:20 changed by wiktor

  • attachment duplicated_event_object_arguments.diff added.

(in reply to: ↑ description ) 10/26/06 22:16:23 changed by wiktor

Replying to wiktor:

I am going to write a detailed post about this issue, but I have to translate it. I hope it will be ready today. :)

Here it is! http://spacergif.net/2006/10/26/duplicated-event-object-arguments/

(follow-up: ↓ 3 ) 11/27/06 11:24:59 changed by mislav

  • cc set to mislav.
  • keywords changed from bindaseventlistener to event.
  • severity changed from normal to minor.
  • summary changed from [PATCH] duplicated event object arguments when using bindAsEventListener to [PATCH] bindAsEventListener duplicated argument.

I've never noticed it, but you're right! bindAsEventListener() is broken indeed. Your patch does the trick, but it replaces "event window.event" with just "event" - not IE friendly in some cases.

(in reply to: ↑ 2 ) 11/27/06 12:17:09 changed by wiktor

Replying to mislav:

I've never noticed it, but you're right! bindAsEventListener() is broken indeed. Your patch does the trick, but it replaces "event window.event" with just "event" - not IE friendly in some cases.

Yes, I replaced it with just "event", because as I mentioned in my post - and as the example shows - the addEvent passes the event object to the function as we expect.

Unfortunately, I don't know the cases which you think, but my experiences - and it seems experieces of others - shows that this trick works reliable.

Quote from blog of David Flanagan:

"Update 2: In comments, Alistair Potts presents test results that indicate that attachEvent has always passed a copy of the window.event object to event handlers. His test code is at: http://www.partyark.co.uk/html/ieeventtest.htm"

Can you drop me an url where I can read about these "some cases"?

11/29/06 09:30:26 changed by mislav

Yeah, attachEvent() seemed consistent to me, too. I think maybe IE/Mac was the culprit, or people are just trying to support the DOM level 0 style, too. (Prototype doesn't support it in the Event module.)

In my patch in #5786 I include your fix without the conditional - but I do check for window.event down the road for IE. Thanks for the link, though! I will test some more - if window.event proves unnecessary I'll remove the conditional.

02/07/07 15:58:09 changed by mislav

  • priority changed from normal to high.

The conditional is necessary and shouldn't be removed. We want to support DOM level 0 style of handling events, in which IE doesn't pass the first argument.

You don't have to fix the patch, but the commiter of it should be aware that the conditional is important since we don't know the context in which this method will be used.

End of story, I think

05/01/07 04:00:18 changed by sam

  • status changed from new to closed.
  • resolution set to fixed.

(In [6641]) prototype: Don't duplicate the event argument in Function#bindAsEventListener. Closes #6497.