madrobby / zepto

Zepto.js is a minimalist JavaScript library for modern browsers, with a jQuery-compatible API
http://zeptojs.com
Other
15k stars 3.91k forks source link

Multiple Live Events Firing Accidentally #198

Open eberon opened 13 years ago

eberon commented 13 years ago

I believe this issue wasn't present in the .1 series, as I tried upgrading a different project many weeks ago and found some events weren't working as I expected. I didn't have the time to analyze why at the time, I just assumed something had annoyingly changed in Zepto. Take the following example, run against the current release of Zepto:

        $(document).ready(function() { 
            $("ul.menu").hide();
            $("h3.closed").live('click', function(e) {
                $(this).removeClass('closed').addClass('open').next().show();
            });
            $("h3.open").live('click', function(e) {
                // uncomment this for crazy times
                //$(this).removeClass('open').addClass('closed').next().hide();
            });
        });

and the following markup:

<h3 class="closed">xxx</h3>
<ul class="menu">
<li>Item</li>
</ul>

Unexpectedly, when clicking the H3, both of its live bindings fire and this means the sibling UL is shown, then hidden immediately. But why would Zepto be doing this? Is it iterating over each live()-registered event and evaluating it regardless of the state at the time of click which it should be using?

7fe commented 13 years ago

Could you add an example on jsbin.com or jsfiddle.net, otherwise github will actually convert your code.

Also jsbin and jsfiddle are great for version tracking and debugging.

Thanks :)

madrobby commented 13 years ago

please reopen with a proper test

eberon commented 13 years ago

Here then: http://jsfiddle.net/swCrg/ This is the simplest example I could distill.

The expected behavior is that the 2nd handler is not fired because the h3's class attribute is "closed" at the time of the click event.

arextar commented 13 years ago

To me it looks as if the "open" class being added makes it test positive for the second handler and that executes.

eberon commented 13 years ago

Well, that is the correct observation to make, but is that the correct behavior? Should the DOM be examined at the time of the event or should it be examined progressively as any event handlers are addressed? I should think the latter could create serious problems in complex applications. And it's also not the same behavior jQuery exhibits.

mislav commented 13 years ago

@arexkun is right about what happens here and @eberon is right that jQuery doesn't suffer from this behavior.

We could try to examine the DOM only once when first event fires and avoid doing it during event propagation. But until then, I suggest you refactor your code to have only 1 "click" handler and switch classnames within it.

mislav commented 13 years ago

I gave this a long thought and I don't see how this issue could be solved in a manner that wouldn't overly complicate our codebase.

But this is a real problem, and I'll keep this open. Patches welcome

zlatanvasovic commented 10 years ago

@mislav This doesn't happen on v1 (try changing zepto.min.js URL to latest version on jsFiddle). Could you close the issue?

mislav commented 10 years ago

Nope, this is still a problem since we haven't changed the way we handle event delegation.

zlatanvasovic commented 10 years ago

Urgh. :(

2014-02-24 18:28 GMT+01:00 Mislav Marohnić notifications@github.com:

Nope, this is still a problem since we haven't changed the way we handle event delegation.

— Reply to this email directly or view it on GitHubhttps://github.com/madrobby/zepto/issues/198#issuecomment-35911514 .

Zlatan Vasović - ZDroid