1

In the simplified code below the href is linked to a custom javascript function (MyFunction). When clicking on the link ('link text') nothing happens (while I'd expect an alert message 'Hello world') and the console shows an error 'Uncaught SyntaxError: Unexpected token !' at the start of the document, which I (also) dont understand.

Just wondering what is wrong in the code below (?)

<script type="text/javascript">
function MyFunction(message){
    alert(message);
}

$(document).ready(function(){
    var message="Hello world";

    $('#myDiv').html('<a id="myLink" href="#" onclick="MyFunction('+message+');">link text</a>'); //looked at the example solution at http://stackoverflow.com/questions/1070760/javascript-function-in-href-vs-onclick 
}); //$(document).ready
</script>
</head>
<body>
    <div id="myDiv">  </div>   
</body>
</html>
Joppo
  • 715
  • 2
  • 12
  • 31

4 Answers4

2

You string for the creating of the a tag will evaluate to this:

<a id="myLink" href="#" onclick="MyFunction(Hello world);">link text</a>

So it will be thrown an exception. You can quote the parameter like many people said:

onclick="MyFunction(\'' + message + '\');" // onclick="MyFunction('Hello World!');"

Or you can do it the right way:

$('#myDiv').append('<a id="myLink" href="#">link text</a>').on('click', 'a', function(e)
{
    alert(message);
});

In this case, message must be declared in the global scope in order to work. Fiddle.

UPDATE: Now, why I think the second one is better:

The html() method espects the following parameter:

A string of HTML to set as the content of each matched element.

And the append() method accepts as the first parameter:

DOM element, array of elements, HTML string, or jQuery object to insert at the end of each element in the set of matched elements.

So you can see that append() espects a DOM element/tree to be appended to the element while html() espects only an string, like innerHTML for pure JavaScript. You just don't add elements to a element with innerHTML, you have to create them, that is what append() does.

Then, as the element is now added to the dom tree, you can set it's events with on() replacing your inline event on the HTML tag. See here or here why.

Community
  • 1
  • 1
DontVoteMeDown
  • 21,122
  • 10
  • 69
  • 105
  • thnx all for your prompt comments (!). @DontVoteMeDown: your solutions work, but why do you prefer your second solution? What's 'wrong' with your first solution? (as you mention 'or you can do it the right way') – Joppo Mar 06 '14 at 20:01
2

String literal should be quoted, even inside quoted code. The problem is you need it to be quoted too much: String literal (1) inside attribute (2) inside string with HTML (3).

With such deep nesting different problems arise. I faced them literally yesterday. I didn't finish with exact test case, but it seems, that even using &quot; leads to bugs in Chrome.

Hence I'd suggest a workaround: put a message into the attribute:

$('#myDiv').html(
     '<a id="myLink" href="#" ' +
     'data-message="' + message.replace(/"/g, "&quot;") + '"' + 
     'onclick="MyFunction(this.getAttribute(\'data-message\'));">' +
     'link text</a>'
);

Solution by @DontVoteMeDown is even better, and not only because it "attacks" the problem from completely different direction, eliminating the need of escaping/quoting.

My technique is useful, when one need to have different context for different elements even with attaching event handlers via DOM level 2 / jQuery.

kirilloid
  • 14,011
  • 6
  • 38
  • 52
  • Nice but, you know, I don't like to mix pure js with jquery, I avoid it(when possible). So in your case you could still use append and then `$(selector).data()` to set/get *data* values. You know, just my opinion. – DontVoteMeDown Mar 06 '14 at 20:16
  • Yes, that's why I mentioned your solution is better *not only* =) – kirilloid Mar 06 '14 at 20:18
  • Nice.. +1 because good answers are made by explanation, not only *do this* like high-rep user some times do. – DontVoteMeDown Mar 06 '14 at 20:20
0

Quote the parameter:

onclick="MyFunction('"+message+"');

Else it looks for a variable with the text from message

tymeJV
  • 103,943
  • 14
  • 161
  • 157
  • What? This is correct... They could be swapped, but either way.. @Ani -- You're answer is actually incorrect, you concatenate the string to early... – tymeJV Mar 06 '14 at 19:54
  • @Ani no, **this** is the correct way. Single quotes should be escaped inside single-quoted string, though. – kirilloid Mar 06 '14 at 19:55
0

You're running into two separate quoting issues at the same time. Currently your code will output this:

<a id="myLink" href="#" onclick="MyFunction(Hello world);">link text</a>

Which isn't correct because JavaScript is going to try to interpret Hello world as objects (notice it has no quotes):

MyFunction(Hello world);

Thus, you need to surround it in quotes. However, if you try that:

'<a id="myLink" href="#" onclick="MyFunction("'+message+'");">link text</a>'

Then the resulting markup won't be parseable:

<a id="myLink" href="#" onclick="MyFunction("Hello world");">link text</a>

Because the double-quotes will confuse it. It'll try to end the onclick attribute at the first parenthesis and then encounter an unexpected Hello token.

(Note the syntax highlighting here on Stack Overflow identifies the problems in both examples.)

So you also need to use single-quotes, and in order to do so in an already single-quoted string, you need to escape them:

'<a id="myLink" href="#" onclick="MyFunction(\''+message+'\');">link text</a>'
David
  • 208,112
  • 36
  • 198
  • 279
  • 1
    One more to go: it still can fail if `message` contains double quotes. `message.replace(/"/g, """)` should work. – kirilloid Mar 06 '14 at 20:00
  • 1
    And for single quotes as well - even more probable, since single quote is often used as apostrophe. – kirilloid Mar 06 '14 at 20:07