Smoke the SmokeDetector

Question

In Tavern on the Meta, our spam detector is a bot named SmokeDetector.

While you can ignore the bot, you can't ignore specific replies to Smokey without ignoring users, so I wrote a UserScript to hide SmokeDetector messages and replies/calls to it by other users.

Click Here to install. It's also available on GitHub and StackApps.

I used the startsWith and includes polyfills, because I was too lazy to use indexOf.

I tried to get as much duplicate logic out of it as possible, hence the innerText function, but it just feels icky at points. I'm not particularly sure how to improve it beyond extracting logic out of the main function a bit more.

// ==UserScript==
// @name SmokeTheSmokeDetector
// @description No more smokey spam
// @version 0.0.1
// @match *://chat.stackexchange.com/rooms/*
// @match *://chat.stackoverflow.com/rooms/*
// @match *://chat.meta.stackexchange.com/rooms/*
// @author The-Quill
// @downloadURL  https://github.com/The-Quill/SmokeTheSmokeDetector/raw/master/SmokeTheSmokeDetector.user.js
// @updateURL https://github.com/The-Quill/SmokeTheSmokeDetector/raw/master/SmokeTheSmokeDetector.user.js
// @run-at document-end
// ==/UserScript==

if (!String.prototype.startsWith) {
    String.prototype.startsWith = function(searchString, position) {
        position = position || 0;
        return this.substr(position || 0, searchString.length) === searchString;
    };
}
if (!String.prototype.includes) {
    String.prototype.includes = function(search, start) {
        'use strict';
        if (typeof start !== 'number') {
            start = 0;
        }
        if (start + search.length > this.length) {
            return false;
        }
        return this.indexOf(search, start) !== -1;
    };
}
var SmokeyIdsBasedOnSite = {
    "chat.meta.stackexchange.com": 266345,
    "chat.stackexchange.com": 120914,
    "chat.stackoverflow.com": 3735529
};
var SmokeyUserId = SmokeyIdsBasedOnSite[document.location.host];
function innerText(element, selector){
    if (!selector) return "";
    var foundElement = element.querySelector(selector);
    if (!foundElement || !foundElement.innerText) return "";
    return foundElement.innerText;
}
function hideSmokeyMessage(message){
    var messageBlock = message.parentNode.parentNode;
    if (messageBlock.classList.contains('user-' + SmokeyUserId)){
        var text = innerText(message, '.content');
        if (!text.includes('fault.') && !text.includes('privileged user')){
            var parentNode = message.parentNode;
            parentNode.removeChild(message);
            if (parentNode.children.length === 0){
                messageBlock.style.display = "none";
            }
        }
        return;
    }
    var messageText = innerText(message, '.content');
    var isSmokeyResponse = innerText(message, '.mention') === '@SmokeDetector';
    if (messageText.startsWith('sd') || (messageText.startsWith('!!/') && messageText !== '!!/blame' && messageText !== '!!/amiprivileged') || isSmokeyResponse){
        var parentNode = message.parentNode;
        parentNode.removeChild(message);
        if (parentNode.children.length === 0){
            messageBlock.style.display = "none";
        }
        return;
    }
}
function nodeInsertionListener(){
    var messages = document.querySelectorAll(".monologue .message");
    Array.prototype.slice.apply(messages).forEach(hideSmokeyMessage);
}
window.addEventListener("DOMNodeInserted", nodeInsertionListener);

Answer 1

I'm sorry, but my review is going to be short; your code looks pretty good.

---

Refactor node deletion

You did a very good job of refactoring your code for the most part. The only repetition that I can find is here:

var parentNode = message.parentNode;
parentNode.removeChild(message);
if (parentNode.children.length === 0){
    messageBlock.style.display = "none";
}

Perhaps it would be best to split this off into a deleteMessage function to keep things simple.

---

Refactoring message content checking

This conditional is fairly lengthy:

if (messageText.startsWith('sd') || (messageText.startsWith('!!/') && messageText !== '!!/blame' && messageText !== '!!/amiprivileged') || isSmokeyResponse)

Perhaps it would be cleaner to split this off into a separate function called isSDCommand.

---

Variable confusion

In the first part of hideSmokeyMessage, you have this:

var text = innerText(message, '.content');

In the second part, after the conditional, you have this:

var messageText = innerText(message, '.content');

Two variables, same exact content but with different names and in different scopes. That's a little confusing.

You should define the text variable above the first conditional so both parts of the function can use it, since they both need to use it anyway.

---

Simplifying argument checking

This is your innerText function:

function innerText(element, selector){
    if (!selector) return "";
    var foundElement = element.querySelector(selector);
    if (!foundElement || !foundElement.innerText) return "";
    return foundElement.innerText;
}

First off, if you are going to check for the existence of selector, why not also check for the existence of element? I personally think you could just drop the checking all together because

• This isn't some standard function that you are reinventing
• You always provide the second parameter

Another simplification you can make is removing the second part of the remaining conditional (the part after the ||). If there is no text, it's going to be "" anyway.

Now your function looks like this:

function innerText(element, selector){
    var foundElement = element.querySelector(selector);
    if (!foundElement) return "";
    return foundElement.innerText;
}

This can be even further simplified with a ternary operator now:

return foundElement ? foundElement.innerText : "";

---

Nitpick

You can merge the SmokeyIdsBasedOnSite map and the SmokeyUserId together to make it a bit cleaner in that section:

var SmokeyUserId = ({
    "chat.meta.stackexchange.com": 266345,
    "chat.stackexchange.com": 120914,
    "chat.stackoverflow.com": 3735529
})[document.location.host];