CODEWITHSUNDEEP
javascriptjqueryhtmlfunctionnative
Joel Murphy
Joel Murphy

Reputation: 2512

Why is my smiley parsing code calling native javascript functions?

I'm writing a smiley parsing function for my website. What I'm trying to accomplish is to transform certain strings, e.g. ":)" into an image like this: enter image description here

Or here's the actual html as an example:

":)" ===> <img src="images/smilies/smile.png" />

My function does what it's meant to do, but it is also parsing native javascript function names! What I mean by this, is if I type a comment containing the strings "push" , "pop" , or "some" (there are probably loads others) my function will parse those strings into invalid images like this:

enter image description here

Here is an html string showing this:

<img src="images/smilies/function some() { [native code] }" alt="">

This results in a 404 not found error in the browser console.

Failed to load resource: the server responded with a status of 404 (Not Found)

Why is this happening? I'm not doing anything too unusual in my code as you can see here:

        function parse_new_comment(commentElem){
            $(commentElem).html(parse_comment($(commentElem).text()));
        }


        function parse_comment(comment){
            var formatted_comment = "";

            var smilies = new Array();
            smilies[":)"] = "smile.png";
            smilies[":D"] = "smile-big.png";
            smilies[":p"] = "tongue.png";
            smilies["[sheep]"] = "sheep.png";
            smilies["<3"] = "love.png";
            smilies["[love]"] = "love.png";

            var words = comment.split(" ");

            for (var i = 0; i < words.length; i++) {
                if(smilies[words[i]] !== undefined){
                    formatted_comment += ' <img src="images/smilies/'+smilies[words[i]]+'" alt="" />';
                }else{
                    formatted_comment += ' ' + words[i];
                }
            }
            return formatted_comment;
        }

I have a feeling that this line of the code is causing the problem if(smilies[words[i]] !== undefined){, as push and pop are array functions, i'm not too sure though... I would appreciate if someone could suggest any ideas on why my function is failing.

Oh I forgot to mention, my page uses ajax to do everything, so new comments are parsed by calling the function like this:

parse_new_comment($("#comment_343"));

Thank you.

Upvotes: 3

Views: 1071

Answers (5)

adeneo
adeneo

Reputation: 318272

I'd just do :

function parse_new_comment(commentElem) {
    commentElem.html(parse_comment(commentElem.text()));
}

function parse_comment(comment) {
    return comment.replace(/(\:\)|\:D|\:p|\[sheep\]|\<3|\[love\])/gi, function (match) {
        var smilies = {
                ":)": "smile.png",
                ":D": "smile-big.png",
                ":p": "tongue.png",
                "[sheep]": "sheep.png",
                "<3": "love.png",
                "[love]": "love.png"
        }
        return '<img src="images/smilies/' + smilies[match] + '" alt="" />';
    });
}

FIDDLE

Upvotes: 4

Francis Avila
Francis Avila

Reputation: 31631

I would use an implementation which dynamically creates a regular expression (which is also likely to be much faster):

RegExp.escape = function(text) {
  return text.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
};

function SmileyParser(smiles) {
    var smilepatterns = [], k;
    for (k in smiles) {
        if (smiles.hasOwnProperty(k)) {
            smilepatterns.push(RegExp.escape(k));
        }
    }
    this.smiles = smiles;
    this.re_smiles = RegExp("(^|\\s)("+smilepatterns.join("|")+")($|\\s)", 'g');
}
SmileyParser.prototype.replace = function (text) {
    var smiles = this.smiles;
    function replacer(m, p1, p2, p3) {
        console.log(m);
        return p1
        +"<img src='/images/smiles/"+smiles[p2]+"'>"
        +p3;
    }
    return text.replace(this.re_smiles, replacer);
};

Then use like this:

var smiles = {
                ":)": "smile.png",
                ":D": "smile-big.png",
                ":p": "tongue.png",
                "[sheep]": "sheep.png",
                "<3": "love.png",
                "[love]": "love.png"
        };

var sp = new SmileyParser(smiles);
var text = 'This comment parses my smiley fine :)';
var newtext = sp.replace(text);

newtext will be:

This comment parses my smiley fine <img src='/images/smiles/smile.png'>

Upvotes: 2

Benjamin Gruenbaum
Benjamin Gruenbaum

Reputation: 276356

Check that the object has the property itself and not that the object's property is undefined. This can be done using hasOwnProperty.

if(smilies.hasOwnProperty(words[i])){

Instead of 

if(smilies[words[i]] !== undefined){

Also, since you are not using smilies as an array I agree with Pointy's comment that it should be declared as an object. It is worth mentioning that when you attach keys to an Array in JavaScript they are only considered array indexes if they can be seen as unsigned integers. 

var smilies = {};//short for new Object();
smilies[":)"] = "smile.png";
smilies[":D"] = "smile-big.png";
smilies[":p"] = "tongue.png";
smilies["[sheep]"] = "sheep.png";
smilies["<3"] = "love.png";
smilies["[love]"] = "love.png";

You might be interested in using literal syntax as suggested by the answer Explosion Pills suggested. Just to clarify, using hasOwnProperty is still required.

Upvotes: 7

alexcasalboni
alexcasalboni

Reputation: 1724

Yes you are right, your code should become:

if(smilies[words[i]] !== undefined && (typeof smilies[words[i]]) == 'string'){

or

if(words[i] in smilies && (typeof smilies[words[i]]) == 'string'){

Upvotes: 1

Explosion Pills
Explosion Pills

Reputation: 191769

Don't use an Array and access it like an object. Use an object. What you are seeing is behavior from accessing the Array methods using object-access syntax. You should only use numeric keys to access arrays.

var smilies = {
    ":)": "smile.png",
    ":D": "smile-big.png",
};

...and so on.

You should be able to keep the rest of your code the same.

Upvotes: 2

Related Questions