Firefox add-on stacked on the loops :(

Question

This is my main.js:

var data = require("self").data;
var window = require("window-utils").activeBrowserWindow;
var tabs = require("tabs");
var Request = require("request").Request;

var team = require("page-mod").PageMod({
include: ["http://www.example.com/team/*"],
contentScriptFile: [data.url("jquery.js"), data.url("item.js")],
contentScriptWhen: 'ready',
onAttach: function(worker) {

        worker.port.on('got-id', function(id) {
        var requesturl = "http://www.othersite.com/item/" + id;   

        Request({
            url: requesturl,
            onComplete: function(response) {
                worker.port.emit('got-request', response.text);
            }
        }).get();

    });
}
});

and my item.js:

// Getting number of total items in the page
var totalitems = document.getElementById("NumberOfItems").textContent;
totalitems = parseInt(totalitems);
// Creating array for items position
positions = [];
for ( z = 0; z <= totalplayers-1 ; z++ ) { positions.push(z < 10 ? ("0" + z.toString()) : z.toString()); }
$.each( positions, function(players, position) {
// Getting item id for the request
var player_id = $("#item-position" + position).attr("href").match(RegExp('items/([^/]+)/details.aspx$'))[1];
self.port.emit('got-player-id-team', player_id);

self.port.on('got-request-team', function(data) {
var columns = $('div.columns',data);
replacediv = $("div#itembox").eq(position).find('td').eq(3);
replacediv.append(columns);
});
});

Its seems that is working fine, BUT all the divs get appended by every item! How i can mod it to place every item's fetch data only in this items and not in every item?

What i am doing wrong? I thinks its running twice! Its like runs = totalitems*2.

If i force it to run for the first item only, everything is good!

Sorry for my bad english! I know that is hard to understand it so i am waiting your comments

Is my question bad or something? – Alex Bogias Dec 06 '12 at 20:30 — Alex Bogias, Dec 06 '12 at 20:30

score 0 · Accepted Answer · edited May 23 '17 at 12:03

as far as I can see, the problem in your code is here:

$.each( positions, function(players, position) {
    // ...
    self.port.on('got-request-team', function(data) {
        var columns = $('div.columns',data);
        replacediv = $("div#itembox").eq(position).find('td').eq(3);
        replacediv.append(columns);
    });
});

Basically for each cycle of the loop, you're adding a new listener to the 'got-request-team' event. Let's say you have only one item in positions: then, you associate to this event only one function, and when you emit the 'got-request-team', only that function is executed. But, if you have, let's say, 20 items in the positions array, you associate 20 different function to the same event, so when 'got-request-team' is emitted, all of them are executed.

Also, because you created a closure, the variable position inside the event listener will always have the last value set, and I believe it's not that your intention. It's a common mistake, and you can find additional details here, for example: Closure needed for binding event handlers within a loop?

There are also some other small things (for example, I don't see replacediv declared anywhere), but they're not related to your issue.

Hope it helps!

Update: about your comment, it's hard to answer without could test the code, but I believe your error is still in the piece above: first, more than positions, it's position that you have to "bind". Because you're in content script, you can use let, therefore you can try something like that:

$.each(positions, function(players, position) {
    // ...

    let pos = position; // let is block scope
    self.port.on('got-request-team', function(data) {
        var columns = $('div.columns',data);
        replacediv = $("div#itembox").eq(pos).find('td').eq(3); // use pos here
        replacediv.append(columns);
    });
});

Note that here you still have the problem of adding new listener to the same event for each item of positions (that you said you have fixed), I just reuse this code to showing the usage of let in this context.

First i would like to thank you for your help. I think i solved the multi-loop problem and now i only have the weird error of columns div appended to all positions and not on the specific one. I tried adding the closure `})(positios);` but i get the exactly same output! — Alex Bogias, Dec 12 '12 at 16:18

Firefox add-on stacked on the loops :(

1 Answers1