Stack Exchange Chat Caret Pathfinder

In The Nineteenth Byte, we have an excessive amount of carets (carrots, if you will) that direct one's attention to another message. For example:

Amidst the confusion, I made a userscript that highlights in yellow the pointed-at message when the message in question is clicked.

(function(){
    var elements = {}, messages;

    function findCarrotMessage(){
        messages = Array.prototype.slice.call(document.querySelectorAll("[id^='message-']"));
        messages.forEach(function(e){
            if(!elements[e.id] && e.className==="message"){
                elements[e.id] = true;
                e.addEventListener("click", function(s){
                    // count the number of ^'s
                    var num = this.textContent.match(/\^+/g), self = this;
                    if(num){
                        var maxHash = -1;
                        num.forEach(function(k){
                            var destinationMessage = messages[Array.from(messages).indexOf(self)-k.length];
                            destinationMessage.style.background = "yellow";
                            setTimeout(function(e){e.style.background = "";}, 3500, destinationMessage);
                            maxHash = Math.max(destinationMessage.id.slice(destinationMessage.id.indexOf("-")+1), maxHash);
                            elements[self.id] = false;
                        });
                        window.location.hash = "#message-" + maxHash;
                    }
                    this.removeEventListener("click", arguments.callee, false);
                }, false);
            }
        });
    }

    setInterval(findCarrotMessage,50);
})();

How can I improve my code? Thanks!

There is still code-golf

Here you have some golfed code:

function(e){e.style.background = "";}

ungolf it:

function(e) {
    e.style.background = ""
}

Run away from arguments.callee


The 5th edition of ECMAScript (ES5) forbids use of arguments.callee() in strict mode. Avoid using arguments.callee() by either giving function expressions a name or use a function declaration where a function must call itself.

 - arguments.callee on MDN

here you have:

e.addEventListener("click", function(s){

instead do...

var MyFunction = function(s) {

and then...

e.addEventListener("click", MyFunction);

this allows you to then do:

this.removeEventListener("click", MyFunction, false);

Don't forget whitespace:

e.className==="message"

add whitespace around the ===. There a lot more places where spaces could be added

Conflicting things

In one part you're using:

Array.prototype.slice.call

in another you are using:

Array.from

you should probably decide on one

Don't to a ton of things in one line

maxHash = Math.max(destinationMessage.id.slice(destinationMessage.id.indexOf("-")+1), maxHash)

that is waaaay to much in one line for a code reviewer like me ;) to be supporting. This:

destinationMessage.id.slice(destinationMessage.id.indexOf("-")+1)

looks like it can become a regex:

( /-(\d+)/.exec(destinationMessage.id) || [])[1]

or even better:

destinationMessage.id.split("-")[1]

use .getAttribute

rather than something like:

e.id

prefer:

e.getAttribute("id")

Remove all code-golf

var destinationMessage = messages[Array.from(messages).indexOf(self)-k.length]

you should split this into multiple variables. What if indexOf returns -1, you should handle that because otherwise your code will error and completely breakdown

use .bind rather than self = this

You're having to introduce self because the forEach's function creates a new scope. Instead use .bind and keep your old scope

num.forEach(function(k) {
    // code
}.bind(this))

Even better, use e.target to get the element that's been clicked. I highly recommending avoiding using this in lambdas to store data.

StackExchange Code Review Q#122962, answer score: 11