jquery wrong event handler

Question

I have two event handlers:

    $('body').on('click','.saveField',function(){
                    var id = $(this).attr('id');
                    alert($(this).attr('class'));
                    id = id.split("_");
                    id = id[0];
                    $('#'+id).trigger('click');
    });

    $('body').on('click','.nav',function(){
            var id = $(this).attr('id');
            alert('I triggered');
            $.ajax({
                    type:"POST",
                    data:"id="+id,
                    url:"getServiceData.php",
                    success:function(result){
                            $('#pageBody').html(result);
                    }

            });
    });

Much deeper into the page I have:

<img src='../images/saveIcon.png' width='25px' id='".$id."_save_".$field."' class='saveField'>

The .nav event handler is being fired by clicking on the above image. My question is where should I be looking to find out why?

This is what I know:

1) The class given by the first is only saveField

2) I have searched through pages to look for a missing quote somewhere, I think this is the cause, but I am not sure.

3) Here is the odd thing. The img src is 1 of many. Only the first instances fires the .nav class event. img src's 2-10 fire the saveField event as expected...

Is this likely some syntax missing a quote/comma/etc. somewhere?

Why do you do `$('body').on('click','.nav',function(){` and not `$('.nav').click(function(){`? That feels like a source of possible bug. — sjahan, Jul 18 '18 at 14:21
Is the image a child of an element with the nav class? Click events bubble (otherwise the delegate would not work). — Taplar, Jul 18 '18 at 14:22
Is `saveField` under `nav` div ? Why are you targeting body ? replace `$('body') to $(document)`. — Bhavin, Jul 18 '18 at 14:23
@ChrisBeckett side note, you don't need the quotes. $(document) or $(document.body) saves a lookup. They are already global variables. — Taplar, Jul 18 '18 at 14:23
the nav element doesnt exist on page load. no. the nav class opens an ajax call, with results containing the imgs yes i have the nav div is across the top of page. saveField is in pages returned by ajax calls — bart2puck, Jul 18 '18 at 14:24
Have you checked the actual DOM structure using your browser dev tools? — CBroe, Jul 18 '18 at 14:25
yes, i have fine-toothed all elements and doms. my eyes are crossed at this point ...I am certain I am missing a . or a ' somewhere. just hoping someone here has experienced this before and can give some insight. — bart2puck, Jul 18 '18 at 14:26
@sjahan, "on" support dynamic doms, but "click" works only on elements that are loaded before before the event is registered. — Dongdong, Jul 18 '18 at 14:28
Why do you suspect you're missing a `.` or a `'`? The symptoms of missing quotes would result in either syntax errors that you'd see in the console output, or in the case of CSS, selectors that simply don't work. — Bucket, Jul 18 '18 at 14:29
my only thought why is because the 1st img src fails. all the rest work fine. img src's are save icons on a list of users returned by some ajax calls. im assuming whatever error causing 1 to fail, may get closed when 2 through n are created. — bart2puck, Jul 18 '18 at 14:30
May be some division closing bracket etc. is missing after ajax call. Are you changing HTML elements after ajax call ? — Bhavin, Jul 18 '18 at 14:31
yes. the ajax call returns a list of users, and some data about each user. this is where i am focused... — bart2puck, Jul 18 '18 at 14:32
Please update your question with HTML code which you get in `result` of success function. I think the problem is that when you are changing `HTML` of division with id `pageBody` after ajax call then there may be you are missing some brackets etc. that's why it's considering division with `saveField ` class. — Bhavin, Jul 18 '18 at 14:33

score 0 · Answer 1 · answered Jul 18 '18 at 14:36

You're likely hitting this because your <img class='saveField' ... > is a descendant of a <div class='nav'>...</div> element. Therefore whenever you click on your image, you're also clicking on all the elements that sit beneath it.

I suggest you look into the DOM structure to see if this is indeed the case. If you don't expect your image to be a descendant of an element with the nav class, you may simply have an unclosed tag somewhere that is placing your image inside this element. Without seeing your DOM structure, it's hard to say. I doubt you're missing a . or a ' in your code, because this would result in your application not functioning, as opposed to functioning incorrectly.

Furthermore, I'd advise using the $('.saveField').on('click', function(){ ... } in place of $('body').on('click','.saveField',function(){ ... }, if nothing more than for readability. The way you're doing it, jQuery has to filter through elements to figure out which ones to fire at the time of the click event, as opposed to registering the click event with these elements initially.

Just to be clear, are you advocating for users to never use delegate event bindings, because of readability? — Taplar, Jul 18 '18 at 14:41
Certainly not, there's a time and place for event delegation, but it's not apparent that it's needed for this solution. It would at least be better practice to bind the event to a specific element as opposed to ``. — Bucket, Jul 18 '18 at 14:45
Good deal, yeah just checking as your last paragraph I kinda read as possibly saying to not do delegate event bindings in general. I definitely agree that delegate event bindings should be bound as low as possible in the dom. — Taplar, Jul 18 '18 at 14:47

score 0 · Answer 2 · answered Jul 18 '18 at 14:40

I guess your html looks like this:

<div class='nav'>

...
<img ... class='saveField' />

...
</div>

so when you click img, but parent it fire

Just add this check to your code:

if(e.target !== e.currentTarget) return;

more details here: jquery stop child triggering parent event

jquery wrong event handler

2 Answers2