#697 enhancement
Radoslav Stankov

Event.fire to support all events, not only custom ones

Reported by Radoslav Stankov | May 31st, 2009 @ 10:08 PM | in 2.0

Today I have been trying to create tests for Event.delegate, and come to the conclusion that prototype needs Event.fire to fire not only custom events but custom too.
I digg a lot today for this issue, I created a Event.fire (looking mainly on old Event.fire and YUI's one)

http://github.com/RStankov/javascript-playground/tree/550554caf705a...

http://gist.github.com/121011

This version now pass all prototype.js tests in - event_test.js + basic tests I created. Tested in FF3 / Safary / IE. But I think there are not enough and I'm not very good at testing.

Comments and changes to this ticket

  • Tobie Langel

    Tobie Langel June 1st, 2009 @ 01:00 AM

    • Milestone set to 2.0
    • State changed from “new” to “enhancement”
  • Juriy Zaytsev

    Juriy Zaytsev June 1st, 2009 @ 06:23 AM

    I've been using Event.simulate (http://github.com/kangax/protolicious/blob/889a60d6d22cd59e93664ae0...
    ) for some time now and it's been working fine so far. I know few other people are using it in their applications as well.

    It's probably too simplistic though (e.g. supports only HTMLEvents and MouseEvents)

  • Juriy Zaytsev

    Juriy Zaytsev June 1st, 2009 @ 06:50 AM

    A couple of comments about Event.fire:

    isKeyEvent = Enumerable.include.bind($w('keydown keyup keypress'));

    This looks cool, but should probably be replaced with regex :) It's much faster that way.

    if (options.relatedTarget && !event.relatedTarget){ if (evenName == 'mouseout'){ event.toElement = options.relatedTarget;

    evenName looks like a typo, doesn't it? What is the purpose of this assignment? Shouldn't there be a check for existence of toElement before trying to assign to it? Is there a guarantee that toElement can be assigned to (it's a host object we are talking about, after all)? I would rather avoid event augmentation.

    } catch(e) {

       try {
    

    // if initKeyEvent() is not , to create generic event - // will fail in Safari 2.x

         return createEvent('Events', eventName, bubble, options);
       } catch(e){
    

    Why so many try/catch? try/catch are slow and should be avoided when possible. Instead, why not detect any deficiencies at load time and use boolean checks?

    For example:

    var IS_KEY_EVENTS_CREATION_SUPPORTED = (function(){
      if (!document.createEvent) return false;
      try {
        var e = document.createEvent('KeyEvents');
        return (typeof e != 'undefined');
      } catch(e) {
        return false;
      }
    })();
    

    This snippet gives me true in FF 3.0.10 and false in nightly WebKit.

    Also, what if I want to fire a non-standard proprietary event, such as "mouseenter"? Do we want to allow that?

  • Radoslav Stankov

    Radoslav Stankov June 1st, 2009 @ 11:47 AM

    10x for the helpful comments,

    This is an very early draft. I wanted to make the basic test cases first than to refactor.

    I will try to skip all ifs and try your approach with IS_KEY_EVENTS_CREATION_SUPPORTED + something like this at load time or make it lazi loaded:

    if (document.createEvent){
       function createMouseEvent(){}
       function createKeyEvent(){}
       // ...
    } else if (document.createEventObject){
       function createMouseEvent(){}
       function createKeyEvent(){}
       // ...
    }
    

    I'm not very experienced with cross-browsers behaviors so, please excuse if some parts of this are not perfect.

    The main reason I want this functionally is for testing ( especially Event.delegate )

    p.s. I think that mouseenter / mouseleave should be in.

  • Samuel Lebeau

    Samuel Lebeau June 1st, 2009 @ 01:45 PM

    @Radoslav : due to how JS scoping works, the construct you just used in your comment wouldn't work as expected, and you'd end up with last version of functions whatever host methods available.

  • Radoslav Stankov

    Radoslav Stankov June 1st, 2009 @ 01:55 PM

    @Samuel Lebeau :10x, I know that, but every time forget about it. I generally overcome this with just variable functions :)

    I have make some basic changes http://gist.github.com/121011 but still have work to do, there are several code duplication patterns (+ toElement/fromElement setting / try catch) and more tests.

    The version http://github.com/RStankov/javascript-playground/tree/4f653f2a84e77... works in FF3/Safary on MacOsX ( latter today will test against IE )

  • Radoslav Stankov

    Radoslav Stankov June 4th, 2009 @ 02:24 PM

    I think I'm near to the final version

    http://github.com/RStankov/javascript-playground/blob/28a1f4a0abc4b...

    This version pass all test on Firefox 2/3, Safari 3, IE 6/7/8, Chrome, Opera 9.2/9.6

    The problems I still have are
    1) mouseenter/mouseleave 2) document.fireEvent on IE have some problems on load, unload, abort, error, select, change, submit, reset,... events 3) event.cancelBubble = true, on IE 3) focus, blur bubbling on IE 4) more sophisticated tests

    Possible solutions:
    1) for mouseenter/mouseleave I will probably use getDOMEventName, but on browsers where the isn't mouseenter/mouseleave, mouseover/mouseout event will be fired 2 and 3) I used a little work around, but probably there is more elegant solution 3) https://prototype.lighthouseapp.com/projects/8886/tickets/666-make-... this will be very useful solution + getDOMEventName (probably will make patch for this too) I tend to use _getDOMEventName to turn focus/blur in IE to focusin/focusout on the background 4) on this I have more work, since I'm not very experienced with js tests

    Code reviews and suggestions are welcomed :)

    In the next couple of days I will try to make a patch in the next couple of day

  • Juriy Zaytsev

    Juriy Zaytsev June 4th, 2009 @ 04:25 PM

    Radoslav, don't forget to test Safari 2.x and Opera 8.54+

  • Radoslav Stankov

    Radoslav Stankov June 4th, 2009 @ 06:13 PM

    Juriy, I always forgot those. When I expand the test suite I'll definitely test in them too

  • Radoslav Stankov

    Radoslav Stankov July 19th, 2009 @ 10:37 PM

    I think I'm ready, finally :)

    Here is the patch it includes:
    - Event.fire - 100% backward compatible to current Event.fire.
    - Unit tests for Event.fire - All current Prototype event functional tests are now rewritten as unit tests via Event.fire - 2 minior changes to prototype event sistem - fix for ticket #731 ( https://prototype.lighthouseapp.com/projects/8886-prototype/tickets... ) - and check for Event.relatedTarget on IE - all test pass on IE6, IE7, IE8, FF3, FF3.5, Safary3, Safary4, Opera8.5, Opera9.25, Opera9.6, Chrome

    All test expect the testMouseEnterMouseLeave pass on FF2. The problem there is that FF2 event.initMouseEvent do not set relatedTarget. I couldn't think of anything that could fix this.

    And I didn't test on Safari 2

    I really hope this is good enough for the Core :)

    p.s. I'm thinking to use Event.fire for the test of Event.delegate ( https://prototype.lighthouseapp.com/projects/8886/tickets/435-event... )

  • Radoslav Stankov
  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:26 AM

    • Tag changed from element.fireevent, event, events, feature, fire, needs_patch, needs_tests to event, events, missing:tests, needs_patch

    [not-tagged:"needs_tests" tagged:"missing:tests" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 02:28 AM

    • Tag changed from event, events, missing:tests, needs_patch to event, events, missing:patch, missing:tests

    [not-tagged:"needs_patch" tagged:"missing:patch" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:36 AM

    • Tag changed from event, events, missing:patch, missing:tests to event, events, missing:patch, needs:tests

    [not-tagged:"missing:tests" tagged:"needs:tests" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:37 AM

    • Tag changed from event, events, missing:patch, needs:tests to event, events, needs:patch, needs:tests

    [not-tagged:"missing:patch" tagged:"needs:patch" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:42 AM

    • Tag changed from event, events, needs:patch, needs:tests to events, needs:patch, needs:tests, section:dom

    [not-tagged:"event" tagged:"section:dom" bulk edit command]

  • Tobie Langel

    Tobie Langel July 24th, 2009 @ 03:43 AM

    • Tag changed from events, needs:patch, needs:tests, section:dom to needs:patch, needs:tests, section:dom

    [not-tagged:"events" bulk edit command]

  • T.J. Crowder

    T.J. Crowder November 16th, 2009 @ 04:50 PM

    [responsible:none bulk edit command]

  • Radoslav Stankov

    Radoslav Stankov March 1st, 2010 @ 07:15 PM

    • Tag changed from needs:patch, needs:tests, section:dom to needs:review, patched, section:dom
  • Jason Westbrook

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

The Prototype JavaScript library.

Shared Ticket Bins

Attachments

Referenced by

Pages