2

This seems to work 75% of the time and the modal function executes with the parameters available, but every few buttons in the table I'll get a Uncaught SyntaxError: Unexpected identifier. Does this have to do with inproper closure? My google searches landed on that as a potential issue but I was unable to implement any of the solutions into my current method here.

html += '<thead><th>Question</th><th>Answer 1</th><th>Answer 2</th><th>Answer 3</th><th>Answer 4</th></thead>';

        for (var i = 0; i < questions.length; i++) {

            question = questions[i].question;
            questionTitle = question.q;
            answer1title = question.a1;
            answer2title = question.a2;

            html += '<tr><td class="question"><b>'
             + question.q
             + '</b></td><td class="answer1">'
             + question.a1
             + '</td><td class="answer2">'
             + question.a2
             + '</td><td class="answer3">'
             + question.a3
             + '</td><td class="answer4">'
             + question.a4
             + '</td><td class="edit">'
             + '<button onclick="openQuestionModal(\''+questionTitle+'\', \''+answer1title+'\', \''+answer2title+'\')" class="btn btn-small btn-primary" id="questionEdit" type="button">Edit</button>'
             + '</td></tr>';   

        }

        $('#questionsTable').append(html); 
Jon Adams
  • 24,464
  • 18
  • 82
  • 120
  • Where are you getting your `questions` from and how does it look? – Henrik Andersson Dec 17 '12 at 08:01
  • Would/could any of `questionTitle`, `answer1title`, or `answer2title` have any single/double quotes in them? – Ian Dec 17 '12 at 08:01
  • I think some or more of the tags are not getting closed, hence the syntax error . – The Dark Knight Dec 17 '12 at 08:02
  • Uncaught SyntaxError: Unexpected identifier - because, the contents of question might have literals which is causing this issue. – DDK Dec 17 '12 at 08:02
  • `questionTitle` or `answer1title` or `answer2title` contains a single quote `'`. – Felix Kling Dec 17 '12 at 08:05
  • questions is a JSON parsed response from an API, I have verified that it is valid JSON and being parsed correctly. – user1909325 Dec 17 '12 at 08:06
  • @adeneo http://jsfiddle.net/ZYnfc/1/ – Ian Dec 17 '12 at 08:07
  • I had done this in the past and never had any issues. Thanks for pointing out that its not the correct way to go about it. What do you recommend. I have changed the variables to be contained within the scope of the for loop and it hasnt helped. – user1909325 Dec 17 '12 at 08:09
  • One of the node's in the JSON array in question contains the word "I'm" but does not warn that its invalid JSON. Could that be the issue? – user1909325 Dec 17 '12 at 08:11
  • @user1909325 It's not invalid JSON, it's the fact that you're concatenating strings and not taking care of escaping quotes – Ian Dec 17 '12 at 08:13
  • Ok, what is the best practice here? – user1909325 Dec 17 '12 at 08:14
  • Just apply `htmlspecialchars` equivalent: http://stackoverflow.com/questions/1787322/htmlspecialchars-equivalent-in-javascript – Ja͢ck Dec 17 '12 at 08:28

2 Answers2

1

The problem is that any of questionTitle, answer1title, or answer2title may have a single quote in their values, which breaks the string concatenation you are attempting. You need to replace all occurrences of a single quote with an escaped single quote. So something like this:

http://jsfiddle.net/ZYnfc/2/

$(document).ready(function () {
    var par = "I'm the problem";
    var replacer = new RegExp("'", "g");
    var new_btn = $('<button onclick="clicker(\'' + par.replace(replacer, "\\'") + '\');">ASDF</button>');
    $("#main").append(new_btn);
});

function clicker(param) {
    alert(param);
}

You'll have to take my example and apply it to your actual code, but the main point is the use of the replace method and the replacer regex for each of your variables you're concatenating in the function call.

There are better ways to append HTML to an area, especially when it comes to a table. And I would suggest binding events with jQuery, not inline. That's a different topic and is unrelated to your problem, although it could've helped avoid it in the first place.

Ian
  • 50,146
  • 13
  • 101
  • 111
  • Can I just run this on the entire array to stamp out the problem before I even start looping? I'd also like to see better examples on howto append HTML. I'm definitely open to learning better practices. – user1909325 Dec 17 '12 at 08:33
0

May be you should escape the string before adding to onclick attribute:

+ '<button onclick="openQuestionModal(\''+ escape(questionTitle)+'\', \''+ escape(answer1title)+'\', \''+escape(answer2title)+'\')" class="btn btn-small btn-primary" id="questionEdit" type="button">Edit</button>'

And Use :

unescape('str_to_revert')// to get the data back inside your function.
Akhil Sekharan
  • 12,467
  • 7
  • 40
  • 57
  • I didnt downvote, but I did try it. It fixed the error but now all the data im passing is loaded with special characters (%20, etc) – user1909325 Dec 17 '12 at 08:20
  • This isn't the escaping you think it is. This `escape` function is for escaping URLs, and is really unnecessary for escaping a single quote, which is the main problem – Ian Dec 17 '12 at 08:29