Stack Exchange userscript for showing spoilers on questions with particular tags

Question

Stack Exchange sites support hiding text as a spoiler with >!; for example:

This text will be hidden until you hover over it/tap the "show spoiler" button.

This gets a lot of use on the Sci-Fi & Fantasy stack, usually to cover up key plot elements of a particular work (like the recent Star Wars film).

That’s a good way to avoid spoilers if you haven’t seen the work in question, but it can be annoying if you already know the plot. Over on Meta SFF, somebody asked whether it would be possible to unhide spoilers on questions with particular tags.

I wrote this UserScript, which allows the user to specify an array of tags, and will unhide spoilers on questions with those tags:

// ==UserScript==
// @name         SFF.SE Show spoilers on questions with particular tags
// @namespace    http://tampermonkey.net/
// @version      1.0
// @description  Show all spoilers on SFF.SE questions with specified tags
// @author       alexwlchan
// @match         *://scifi.stackexchange.com/questions/*
// @match         *://meta.scifi.stackexchange.com/questions/*
// @grant        none
// ==/UserScript==
/* jshint -W097 */
'use strict';


// If every tag on a question is in this array, then the spoilers on this
// question will be shown.
var SHOW_ME_SPOILERS_IF_TAGGED = [
    "harry-p0tter",
    "star-w4rs",
];


// Returns an array of tags on a question
var getTagsOnQuestion = function() {
    var taglist = document.getElementsByClassName("post-taglist")[0];
    var hrefs = taglist.getElementsByClassName("post-tag");

    var tags = [];
    for (var ii = 0; ii < hrefs.length; ii++) {
        tags.push(hrefs[ii].innerText);
    }
    return tags;
}


// Returns true/false depending on whether the spoilers on this question
// should be shown -- that is, whether every tag on the question is in
// SHOW_ME_SPOILERS_IF_TAGGED.
var shouldShowSpoilersOnQuestion = function() {

    // Get the tags on a question.  All questions have at least one tag.
    var tags = getTagsOnQuestion();

    for (var jj = 0; jj < tags.length; jj++) {
        // If we don't find the tag in the array of spoiler-able tags,
        // then we have to reject the question.
        if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tags[jj]) == -1) {
            return false;
        }
    }
    return true;
}


// Modify all the spoilers on the page to show their spoiler text
var showSpoilers = function() {
    var spoilers = document.getElementsByClassName("spoiler");
    for (var kk = 0; kk < spoilers.length; kk++) {
        spoilers[kk].transition = "none";
        spoilers[kk].style.color = "#222426";
    }
}


if (shouldShowSpoilersOnQuestion()) {
    showSpoilers();
}

Any feedback is welcome, but particularly:

• Is this good/idiomatic JavaScript?
• Is the intent of the code clear?
• Are there any edge cases that I’ve missed?
• Is there a better way to check if (tags on the question) is a subset of (tags the user wants spoilers to be shown for)? I feel like my current approach is quite inefficient.

Answer 1

For Loop

The style of the for loop you're using could be improved.

for (var jj = 0; jj < tags.length; jj++) {
    // If we don't find the tag in the array of spoiler-able tags,
    // then we have to reject the question.
    if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tags[jj]) == -1) {
        return false;
    }
}

should probably be changed to a for of loop

for (tag of tags) {
    // If we don't find the tag in the array of spoiler-able tags,
    // then we have to reject the question.
    if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tag) == -1) {
        return false;
    }
}

map

This code is essentially doing the same thing as a map function:

var tags = [];
for (var ii = 0; ii < hrefs.length; ii++) {
    tags.push(hrefs[ii].innerText);
}

You could rewrite it to this (which uses an arrow function as well):

var tags = hrefs.map(href => href.innerText);

Answer 2

He's actually his father

Jokes aside, your code is good, but there's a few things it could use to make it great:

---

Using the right levels of abstraction and user control:

var SHOW_ME_SPOILERS_IF_TAGGED = [
    "harry-p0tter",
    "star-w4rs",
];

I'm not trying to insult, but I would think that some of the SF&F community aren't actually programmers, and as such, they may have issues or reservations modifying this.

I would suggest implementing a dialog box that pops up the first time you run the script, and using a drag-and-drop list (so they can choose tags). For this, you could use a plugin like jQuery UI's Sortable.

---

Using map:

You don't need to build an array using another variable, you can just convert and use Array.prototype.map:

var taglist = document.getElementsByClassName("post-taglist")[0];
var hrefs = taglist.getElementsByClassName("post-tag");
var tags = [];
for (var ii = 0; ii < hrefs.length; ii++) {
    tags.push(hrefs[ii].innerText);
}
return tags;

into:

var tags = document.getElementsByClassName("post-taglist")[0]
           .getElementsByClassName("post-tag");
return Array.prototype.slice.apply(tags).map(function(e){
    return e.innerText;
});

or using arrow notation:

var tags = document.getElementsByClassName("post-taglist")[0]
           .getElementsByClassName("post-tag");
return Array.prototype.slice.apply(tags).map(e => e.innerText);

---

Naming:

Index variables:

Your choice of naming on your index variables is strange:

• jj, ii, kk

They can never exist at the same time, so it's pretty pointless to have them named differently. You should just stick with the standard index variable name: i.

SHOW_ME_SPOILERS_IF_TAGGED:

This is a bit long and icky, I would consider smaller names like SPOILABLE, SPOILABLE_TAGS, TAGS_TO_SPOIL or something similar instead.

---

spoiler class:

Instead of modifying the style properties on each spoiler, you can just take away the spoiler class.

var spoilers = document.getElementsByClassName("spoiler");
for (var kk = 0; kk < spoilers.length; kk++) {
    spoilers[kk].transition = "none";
    spoilers[kk].style.color = "#222426";
}

into:

var spoilers = document.getElementsByClassName("spoiler");
for (var i = 0; i < spoilers.length; i++) {
    spoilers[i].classList.remove('spoiler');
}

However, this code would have an issue as you're accessing a live-list and removing it from that would effective mean that you'd skip every second data item, up to the last item would be skipped entirely.

Instead, select all <blockquotes> tags, like:

var spoilers = document.getElementsByTagName("blockquote");
for (var i = 0; i < spoilers.length; i++) {
    spoilers[i].classList.remove('spoiler');
}

---

L33T:

I'm not sure why you guys over at SF&F would have tags like:

• harry-p0tter, or
• star-w4rs.

Either this a bug, or you should just be commenting out those lines instead of confusing the reader.