Uncaught ReferenceError: function is not defined with onclick

ECMAScript picture ECMAScript · Jun 29, 2013 · Viewed 381.5k times · Source

I'm trying to make a userscript for a website to add custom emotes. However, I've been getting a lot of errors.

Here is the function:

function saveEmotes() {
    removeLineBreaks();
    EmoteNameLines = EmoteName.value.split("\n");
    EmoteURLLines = EmoteURL.value.split("\n");
    EmoteUsageLines = EmoteUsage.value.split("\n");

    if (EmoteNameLines.length == EmoteURLLines.length && EmoteURLLines.length == EmoteUsageLines.length) {
        for (i = 0; i < EmoteURLLines.length; i++) {
            if (checkIMG(EmoteURLLines[i])) {
                localStorage.setItem("nameEmotes", JSON.stringify(EmoteNameLines));
                localStorage.setItem("urlEmotes", JSON.stringify(EmoteURLLines));
                localStorage.setItem("usageEmotes", JSON.stringify(EmoteUsageLines));
                if (i == 0) {
                    console.log(resetSlot());
                }
                emoteTab[2].innerHTML += '<span style="cursor:pointer;" onclick="appendEmote(\'' + EmoteUsageLines[i] + '\')"><img src="' + EmoteURLLines[i] + '" /></span>';
            } else {
                alert("The maximum emote(" + EmoteNameLines[i] + ") size is (36x36)");
            }
        }
    } else {
        alert("You have an unbalanced amount of emote parameters.");
    }
}

The span tag's onclick calls this function:

function appendEmote(em) {
    shoutdata.value += em;
}

Every time I click a button that has an onclick attribute, I get this error:

Uncaught ReferenceError: function is not defined.

Any help would be appreciated.

Thank you!

Update

I tried using:

emoteTab[2].innerHTML += '<span style="cursor:pointer;" id="'+ EmoteNameLines[i] +'"><img src="' + EmoteURLLines[i] + '" /></span>';
document.getElementById(EmoteNameLines[i]).addEventListener("click", appendEmote(EmoteUsageLines[i]), false);

But I got an undefined error.

Here is the script.

I tried doing this to test if listeners work and they don't for me:

emoteTab[2].innerHTML = '<td class="trow1" width="12%" align="center"><a id="togglemenu" style="cursor: pointer;">Custom Icons</a></br><a style="cursor: pointer;" id="smilies" onclick=\'window.open("misc.php?action=smilies&amp;popup=true&amp;editor=clickableEditor","Smilies","scrollbars=yes, menubar=no,width=460,height=360,toolbar=no");\' original-title="">Smilies</a><br><a style="cursor: pointer;" onclick=\'window.open("shoutbox.php","Shoutbox","scrollbars=yes, menubar=no,width=825,height=449,toolbar=no");\' original-title="">Popup</a></td></br>';
document.getElementById("togglemenu").addEventListener("click", changedisplay,false);

Answer

Brock Adams picture Brock Adams · Jun 29, 2013

Never use .onclick(), or similar attributes from a userscript! (It's also poor practice in a regular web page).

The reason is that userscripts operate in a sandbox ("isolated world"), and onclick operates in the target-page scope and cannot see any functions your script creates.

Always use addEventListener()Doc (or an equivalent library function, like jQuery .on()).

So instead of code like:

something.outerHTML += '<input onclick="resetEmotes()" id="btnsave" ...>'


You would use:

something.outerHTML += '<input id="btnsave" ...>'

document.getElementById ("btnsave").addEventListener ("click", resetEmotes, false);

For the loop, you can't pass data to an event listener like that See the doc. Plus every time you change innerHTML like that, you destroy the previous event listeners!

Without refactoring your code much, you can pass data with data attributes. So use code like this:

for (i = 0; i < EmoteURLLines.length; i++) {
    if (checkIMG (EmoteURLLines[i])) {
        localStorage.setItem ("nameEmotes", JSON.stringify (EmoteNameLines));
        localStorage.setItem ("urlEmotes", JSON.stringify (EmoteURLLines));
        localStorage.setItem ("usageEmotes", JSON.stringify (EmoteUsageLines));
        if (i == 0) {
            console.log (resetSlot ());
        }
        emoteTab[2].innerHTML  += '<span style="cursor:pointer;" id="' 
                                + EmoteNameLines[i] 
                                + '" data-usage="' + EmoteUsageLines[i] + '">'
                                + '<img src="' + EmoteURLLines[i] + '" /></span>'
                                ;
    } else {
        alert ("The maximum emote (" + EmoteNameLines[i] + ") size is (36x36)");
    }
}
//-- Only add events when innerHTML overwrites are done.
var targetSpans = emoteTab[2].querySelectorAll ("span[data-usage]");
for (var J in targetSpans) {
    targetSpans[J].addEventListener ("click", appendEmote, false);
}

Where appendEmote is like:

function appendEmote (zEvent) {
    //-- this and the parameter are special in event handlers.  see the linked doc.
    var emoteUsage  = this.getAttribute ("data-usage");
    shoutdata.value += emoteUsage;
}


WARNINGS:

  • Your code reuses the same id for several elements. Don't do this, it's invalid. A given ID should occur only once per page.
  • Every time you use .outerHTML or .innerHTML, you trash any event handlers on the affected nodes. If you use this method beware of that fact.