jQuery children a href click does not work

Question

I have this code:

<html>
<head>
<title>site</title>

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.6.1/jquery.min.js" type="text/javascript"></script>
<script type="text/javascript">
$(document).ready(function() {
    $('#wlink a').click(function() {
        $('.box:visible').fadeOut('fast', function() {
            $('#' + (this.id).replace('link', '')).fadeIn('fast');
        });
        $('#wlink a').removeClass('selected');
        $(this).addClass('selected');
    });
    $('#wlink div').click(function() {
        var child = $(this).children();
        child.click();
    });
    $('#linkbox1').addClass('selected');
    $('#box1').fadeIn('fast');
});
</script>
</head>

<style>
a { outline: none; cursor: pointer; }
#wrapper { border:1px solid #cccccc; border:solid 1px #ddd; width:806px; height:255px; overflow: hidden; }
#wrapperBox { width:6000px; }
span.text { font-size:100px; color:#aaa; }
div.box { float:left; width:805px; height:255px; background:#efefef; display: none; }
#wlink div { width: 200px; text-align:center; display: block; float:left; border: solid 1px #ddd; }
a.selected { background: #eee; }
</style>

<body>
<div id="wrapper">
    <div id="wrapperBox">
        <div id="box1" class="box">
            <span class="text">Box 1</span>
        </div>
        <div id="box2" class="box">
            <span class="text">Box 2</span>
        </div>
        <div id="box3" class="box">
            <span class="text">Box 3</span>
        </div>
        <div id="box4" class="box">
            <span class="text">Box 4</span>
        </div>
    </div>
</div>
<div id="wlink">
    <div><a id="linkbox1">Box 1</a></div>
    <div><a id="linkbox2">Box 2</a></div>
    <div><a id="linkbox3">Box 3</a></div>
    <div><a id="linkbox4">Box 4</a></div>
</div>
</body>
</html>

Now what I want to do is when the parent DIV of the A HREF is clicked, I want to simulate an HREF click. But it does not work, and I get this error:

too much recursion
[Break On This Error] )});return}if(e.nodeType===3||e.nodeTy...nt=="undefined"&&(b=b.ownerDocument||

What is wrong with my code?

Thanks, J

Adam Terlson · Accepted Answer · 2011-09-15T14:11:30.240

30

sillyMunky is correct in that your div click handler will also be fired, creating an infinite loop, but his approach to solving this issue is not best practice. What you want to do is explicitly stop event propagation with e.stopPropagation() in your click handler and not return false. Using return false will do more than you need/intend. If you also want to prevent the default click action and stop the page jump, you'll also want to add e.preventDefault().

$('#wlink a').click(function(e) {
    e.stopPropagation();
    e.preventDefault(); //not part of fixing your issue, but you may want it.
    $('.box:visible').fadeOut('fast', function() {
        $('#' + (this.id).replace('link', '')).fadeIn('fast');
    });
    $('#wlink a').removeClass('selected');
    $(this).addClass('selected');
});

For more info: Stop (Mis)using Return False

edited Sep 15 '11 at 14:11

answered Sep 15 '11 at 13:57

Adam Terlson

12,610
4
42
63

1

I assumed he wanted to stopPropagation and preventDefault, the most efficient way to do this is just return false. Furthermore, I have personally experienced browser compatibility issues with those two functions, but never with using return false. I disagree that using return false is bad practice at all, someone deciding to say its better practice on their blog doesn't cut any mustard with me. If he needed the default action but not the bubbling then the correct answer would have been to use e.stopPropagation. I prefer my more elegant answer. – sillyMunky Sep 15 '11 at 14:33
Pointing out these things in the link and providing an alternative is a good call, maybe the OP hasn't met them before. But I think claiming there's a 'best practice' for this which is the long way round is not so helpful. – sillyMunky Sep 15 '11 at 14:36
1

You assumed that he wanted that without explicitly saying it, which I think is the disservice. Newbies need to understand *what* their code is doing and *why* they should write it one way or another. If the OP wanted to `return false` while understanding its implications, fine. What's a disservice is saying "just return false to prevent propagation" when that's not all returning false does and could have other negative impacts. I'm saying "This is the code that does what you asked, and here's what return false actually does. Use wisely." Really though, it's not that big of a deal. – Adam Terlson Sep 15 '11 at 15:25
what e.stopPropagation(); e.preventDefault(); do? – Tech4Wilco Sep 15 '11 at 15:25
1

@Jimmy - See my link above, it explains it well. Check these out too: https://developer.mozilla.org/en/DOM/event.preventDefault and https://developer.mozilla.org/en/DOM/event.stopPropagation – Adam Terlson Sep 15 '11 at 15:27
@Adam Terison, I guess explaining why is a fair point, i've updated my post accordingly. I still definitely prefer my method for the two reasons I stated, but plurality is the spice of life ;) I won't be voting you down despite thinking my method is better, I'm not sure if it was you who voted me down before. meh. – sillyMunky Sep 15 '11 at 19:37
It most certainly was not me, sir! We have different approaches, and that's just fine by me! I also don't see any downvotes next to your post. You're at +1 / 0. – Adam Terlson Sep 15 '11 at 20:26

score 29 · Answer 2 · answered Sep 15 '11 at 22:54

@Adam Terlson has a good solution if you don't want to change your code. Here's my solution:

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>site</title>

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.6.1/jquery.min.js" type="text/javascript"></script>
<script type="text/javascript">
$(document).ready(function() {
$('#wlink a').click(function() {
    var l = (this.id).replace('link', '');
    $('.box:visible').fadeOut('fast', function() {
        $('#' + l).fadeIn('fast');
    });
    $('#wlink a').removeClass('selected');
    $(this).addClass('selected');
});
$('#linkbox1').addClass('selected');
$('#box1').fadeIn('fast');
});
</script>

</head>

<style>
a { outline: none; }
#wrapper { border:1px solid #cccccc; border:solid 1px #ddd; width:806px; height:255px; overflow: hidden; }
#wrapperBox { width:6000px; }
span.text { font-size:100px; color:#aaa; }
div.box { float:left; width:805px; height:255px; background:#efefef; display: none; }
a.linkBox { cursor: pointer; width: 200px; text-align:center; display: block; float:left; border: solid 1px #ddd; }
a.selected { background: #eee; }
</style>

<body>

<div id="wrapper">
   <div id="wrapperBox">
       <div id="box1" class="box">
           <span class="text">Box 1</span>
       </div>
       <div id="box2" class="box">
           <span class="text">Box 2</span>
       </div>
       <div id="box3" class="box">
           <span class="text">Box 3</span>
       </div>
       <div id="box4" class="box">
           <span class="text">Box 4</span>
       </div>
   </div>
</div>
<div id="wlink">
   <a class="linkBox" id="linkbox1">Box 1</a>
   <a class="linkBox" id="linkbox2">Box 2</a>
   <a class="linkBox" id="linkbox3">Box 3</a>
   <a class="linkBox" id="linkbox4">Box 4</a>
</div>

</body>
</html>

It does not invoked any of the previous answered but rather use HREF without DIVs and use CSS to do the magic. This way, you don't have to select childrens, parents etc...

I think this is way easier to understand but I have too much code I need to change. thanks — Tech4Wilco, Sep 16 '11 at 12:42

sillyMunky · Answer 3 · 2011-09-15T19:34:18.163

It is enough to add return false; to the end of the anchor's click handler. The problem seems to be that the click handler being fired is then bubbling up to the div which contains it making an infinite recursive loop. Adding the return false will prevent both the event propagating (reaching up the hierarchy to parent elements) and the default action being performed (following the link if it was clicked).

You could do this using the individual functions of the event object (e.stopPropagation and e.preventDefault respectively) if you prefer, however you are more likely (in my experience) to have problems in your target browsers doing this than doing both at once with the return false; technique.

 $('#wlink a').click(function() {
    $('.box:visible').fadeOut('fast', function() {
        $('#' + (this.id).replace('link', '')).fadeIn('fast');
    });
    $('#wlink a').removeClass('selected');
    $(this).addClass('selected');
    return false;
 })

score 0 · Answer 4 · answered Sep 15 '11 at 13:57

0

I think your problem lies with the fact that you are binding a click event to both the div and the anchor (which is wrapped by the same div). So when someone clicks the div or any link inside it, both click events will fire.

answered Sep 15 '11 at 13:57

Bas Slagter

9,831
7
47
78

jQuery children a href click does not work

4 Answers4