jQuery loop, only last ajax success executes

Question

So this is the heart of my Chrome Extension [here], [jq is jQuery.noConflict()] the jq('.pam div .fbCurrentActionLink') returns each "Poke" link on Facebook. It uses .each() to loop through each person [aka, each person's "poke" link], and on success, it replaces the text "Poke" with a bold & green "Poked" text.

function execute()
{
        jq('.pam div .fbCurrentActionLink').each(function () {

        anc=jq(this)
        uid=anc.attr('ajaxify').match(/(\d+)/)[0]

        //ajax
                var post_form_id = jq('#post_form_id').val();
                var fb_dtsg = jq('input[name=fb_dtsg]').val();
                //use AJAX to submit poke, via their fb id
                jq.ajax({
                        type: 'POST',
                        url: 'ajax/poke.php?__a=1',
                        data: 'uid=' + uid + '&pokeback=1&post_form_id=' + post_form_id + '&fb_dtsg=' + fb_dtsg + '&post_form_id_source=AsyncRequest',
                        beforeSend: function(xhr){
                                xhr.setRequestHeader("Content-type", "application/x-www-form-urlencoded; charset=UTF-8");
                        },
                        success: function(data, textStatus){
                                anc.html('<b style="color:green !important">Poked</b>');
                        }
                });

        //ajax
        });

}

Now, the problem is, let's say there are 3 pokes to return. It will only execute the .html() on the last one. I can't figure out why.

I'm not 100% certain, but I believe the anc variable gets re-assigned (even in the success closure) on each loop iteration. If you declare the variable using var anc; in each loop cycle it may fix your problem. — robbrit, Nov 08 '11 at 19:38
FYI: Never use ``, it's deprecated in XHTML and strongly avoided in HTML5. Use `` for semantic emphasis or `` otherwise. Never use inline styling, it's evil. Use classes or IDs and an external style sheet for styling. Oh, and you're missing semicolons and the `var` keyword in your variable declarations. — Adam Terlson, Nov 08 '11 at 19:39
My guess is that the variable "anc" appears to be global, and since anc.html() replaces the html rather than appending to the html, it's being overwritten. I suspect your code is running all three times, but the HTML is being overwritten each time so that only the last one shows. — Jemaclus, Nov 08 '11 at 19:39
You can pull var post_form_id = jq('#post_form_id').val(); var fb_dtsg = jq('input[name=fb_dtsg]').val(); out of the loop. No need to redeclare the variables. — Cory Danielson, Nov 08 '11 at 20:06
You're also missing a semi colon after these 2 declarations anc=jq(this) & uid=anc.attr('ajaxify').match(/(\d+)/)[0] — Cory Danielson, Nov 08 '11 at 20:06
I helped somebody last week with a similar problem involving looping with jQuery and multiple object... I can't seem to wrap my head around how to apply his fix to your problem. Here's the link http://stackoverflow.com/questions/7999668/while-loop-in-jquery-of-dynamic-id-and-class/8000007#8000007 ... I'm still 'investigating' — Cory Danielson, Nov 08 '11 at 20:09
@CoryDanielson, how so? post_form_id is a **object**, see screenshot: http://img401.imageshack.us/img401/9056/screenshot2ph.png . As far as missing semi-colons, I wasn't aware semi-colons were **required**, I thought they were optional. — Matt, Nov 08 '11 at 21:25
Ohh are they? I'm just so accustomed to using semi colons that I never write a line without them. To pull the 2 variable declarations out of the loop just cut and paste the above the `.each()` loop. As you have it now, each time the loop runs it recreates those variables and I'm assuming that the value of them never changes. It's just an efficiency thing. No need to recreate/dedeclare the variables during each loop if the value remains constant (which i'm assuming they do) — Cory Danielson, Nov 08 '11 at 21:51
Right! I forgot about that, thanks! And yes, W3schools says it is optional here: http://www.w3schools.com/js/js_statements.asp [But I supposed it's preferred to have semi-colons. sometimes I use them, sometimes not. BASH also has optional semi-colons] — Matt, Nov 08 '11 at 21:54

Cory Danielson · Accepted Answer · 2015-04-09T02:34:51.480

5

I think the problem you're running into is that by the time the first ajax call is done, the value of anc has been overridden. Once the success event is triggered, anc is equal to it's value at the last iteration. To solve this, you must stop the values from being overridden or, during the loop, you must tie each ajax calls to the element where the results will be placed.

To tie an ajax call to a dom element, you need to supply define the context of the ajax call.

Give this a try, I'm hopeful it should work...

      jq.ajax({
              type: 'POST',
              context: this,        //added line
              url: 'ajax/poke.php?__a=1',
              data: 'uid=' + uid + '&pokeback=1&post_form_id=' + post_form_id + '&fb_dtsg=' + fb_dtsg + '&post_form_id_source=AsyncRequest',
              beforeSend: function(xhr){
                    xhr.setRequestHeader("Content-type", "application/x-www-form-urlencoded; charset=UTF-8");
              },
              success: function(data, textStatus){
                    $(this).html('<b style="color:green !important">Poked</b>');  
                    //changed to $(this) instead of anc
              }
      });

edited Apr 09 '15 at 02:34

answered Nov 08 '11 at 20:19

Cory Danielson

14,314
3
44
51

+1, It did work. But what's this `context: this,` mean? Also, let's say I wanted to only have one AJAX request at a time? – Matt Nov 08 '11 at 21:21
Context relates the AJAX call to an HTML element, so that inside all functions related to that ajax call (success, beforeSend, error, etc) when you use the `this` pointer, it points to the ajax call's `context` definition. – Cory Danielson Nov 08 '11 at 21:40
In order to have 1 ajax call at a time, you would set `async: "false",` within the `jq.ajax({ async: "false", /*url,context,data...*/ });` That will disable the `asynchronous` functionality of AJAX, meaning that only 1 ajax call will occur at a time and once it finishes the other will start. – Cory Danielson Nov 08 '11 at 21:42
To be more specific: When I used `context: this,` the `this` is a reference to the `this` pointer supplied by the `.each()` loop. A more typical `context` definition would be something like `context: document.getElementById("results"),` but since the ajax call was inside of an `.each()` loop, I just grabbed the `this` from that loop and sort of related it to the ajax call using the `context`. That's the problem you were having, the ajax calls weren't being properly tied to the HTML elements which they were modifying. – Cory Danielson Nov 08 '11 at 21:48
Well, I'm not sure I understand your second comment fully.. oh well :P So is it bad to use non-asynchronous? Does it freeze the browser? Also, LOL! I thought you and the person I was talking to about the semi-colons were different people. XD – Matt Nov 08 '11 at 21:58
When you run a loop, you have a collection of elements that you are looping through. In order to reference the current element that is being looped through, you can simply type `this` or `$(this)` to add jQuery functionality. Since you ajax call is inside of the loop, I was able to use `this` to define the `context`. Just like you did with `anc=jq(this)` – Cory Danielson Nov 08 '11 at 22:00
And yeah, a non asynchronous call will 'freeze' the browser. Asynchronous calls run in the background. So when you run ajax (here comes a long ugly word) non-asynchronously, the call is pushed to the front and nothing else will execute until the call is done. The problem with that, is that if the called url gets stuck in an infinity loop or takes a long time to load you'll be sitting there for a while waiting for the ajax to load... in order to prevent that, you can set the `timeout` limit for the call. If the call times out, the error event will be triggered – Cory Danielson Nov 08 '11 at 22:04
do I just add `timeout: 5000,` to the $.ajax() ? for 5 seconds? or no – Matt Nov 08 '11 at 23:09
Yep, that's it. 5 seconds is usually more than people are willing to wait though. If the timeout fails, the error code will execute. – Cory Danielson Nov 09 '11 at 01:11
Alright I set it to 2000. Is that fine? So by freezing, it doesn't mean the browser locks up, it means it can't send ajax requests? For facebook, I guess that means you can't do a thing unless you reload. lol – Matt Nov 09 '11 at 02:15
No "freezing" is worse than that. It pretty much does mean that the browser locks up. Most of the time, when people set `async: 'false',` they have a 'spinner' graphic to be displayed so that it is implied to the user that they cannot do anything and must wait. Something like this: http://www.loadinfo.net/ – Cory Danielson Nov 09 '11 at 02:19
Aw.. but why does the browser lock up? I don't get why it would, if anything, maybe just the tab, that would make sense – Matt Nov 09 '11 at 02:44
Idk.. people could make up reasons all day... I guess it's just so you don't navigate away from the page during the ajax call... not sure... – Cory Danielson Nov 09 '11 at 05:04

score 3 · Answer 2 · answered Nov 08 '11 at 20:22

You have to prefix your variables by var. Otherwise, they're not locally (ie, in each) declared. If this is the case, the variable is overwritten at each iteration.

function execute() {
        jq('.pam div .fbCurrentActionLink').each(function () {
            var anc = jq(this);                              // Added `var`
            var uid = anc.attr('ajaxify').match(/(\d+)/)[0]; // Added `var`
            ....

jQuery loop, only last ajax success executes

2 Answers2