126

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.

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);
Web Developer
  • 333
  • 4
  • 17
ECMAScript
  • 4,439
  • 4
  • 21
  • 29
  • 1
    The link to your complete script is always appropriate and appreciated, *in addition* to relevant code snippets which should be in your post. – Brock Adams Jun 29 '13 at 09:10

8 Answers8

185

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.
Community
  • 1
  • 1
Brock Adams
  • 90,639
  • 22
  • 233
  • 295
  • 1
    I receive this error: Uncaught TypeError: Object 3 has no method 'addEventListener' – ECMAScript Jun 29 '13 at 10:49
  • You're doing it wrong, then. Link to the full script that gave that error, AND link to the target page it runs on. – Brock Adams Jun 29 '13 at 12:02
  • 2
    Fixed it I used: btnreset.onclick = function(){ resetEmotes(); }; Thanks! – ECMAScript Jun 29 '13 at 13:12
  • Glad you got it working, but that technique will only work in some conditions and is not portable. – Brock Adams Jun 29 '13 at 21:17
  • My code is really poorly coded so I'm re coding the whole thing and using jQuery. Thanks! – ECMAScript Jun 30 '13 at 23:48
  • what do I do, if I want to have parameters for my function? Is Best practice to lookup an attribute from the element now? Before I tried onclick="toggle('setting1')". How do get this done with an EventListener? – kruben Mar 03 '20 at 20:53
  • 1
    "never" is a _little_ extreme. Seems a bit like Evan You (creator of vue) proclaiming that no web developer should ever use input type=hidden. "Best practice" is one thing... but a hard and fast rule stating no one should ever under any set of circumstances use [insertPossibleTechniqueOrTechnology] is rarely going to be true. – MemeDeveloper Dec 10 '20 at 17:39
  • something.outerHTML += '' document.getElementById ("btnsave").addEventListener ("click", resetEmotes, false); // It's working well. – Bang Andre Feb 27 '23 at 07:10
24

Make sure you are using Javascript module or not?! if using js6 modules your html events attributes won't work. in that case you must bring your function from global scope to module scope. Just add this to your javascript file: window.functionName= functionName;

example:

<h1 onClick="functionName">some thing</h1>
6

I think you put the function in the $(document).ready....... The functions are always provided out the $(document).ready.......

Mouhcine Mtg
  • 69
  • 1
  • 1
5

I got this resolved in angular with (click) = "someFuncionName()" in the .html file for the specific component.

Alferd Nobel
  • 3,185
  • 2
  • 30
  • 35
0

Check the casing of your functions.

onclick="sillyLongFunctionName" 

and

function sillylongFunctionName() { ...

Are not identical. Hard to spot sometimes!

bendecko
  • 2,643
  • 1
  • 23
  • 33
0

for me I've just put the function out of pageReady scope and it works jQuery(document).ready(function(){ don't the function here }

put your function here with script tag

  • While this link may answer the question, it is better to include the essential parts of the answer here and provide the link for reference. Link-only answers can become invalid if the linked page changes. - [From Review](/review/late-answers/34790985) – Andrew Louw Aug 07 '23 at 16:15
-2

If the function is not defined when using that function in html, such as onclick = ‘function () ', it means function is in a callback, in my case is 'DOMContentLoaded'.

  • 1
    This is not an answer. You can post it as a comment(by building up some proper reputation). Please read [How do I write a good answer?](https://stackoverflow.com/help/how-to-answer) – EhsanT Jun 01 '20 at 13:44
-2

See that your function is not in a callback function if you are using an external js file. Removing the callback function would do the trick

(function() {  //comment this out

//your code

})(); //comment this out
I_love_vegetables
  • 1,575
  • 5
  • 12
  • 26
Anna
  • 47
  • 6