0

I am using the following excellent userscript (for Chrome, Chrome's Tampermonkey and Firefox Greasemonkey). It's supposed to display movie ratings next to each IMDb movie link, but it stopped working properly.

This is the complete script:

// ==UserScript==
// @name           Add IMDb rating next to all IMDb links (+voter count)
// @author         Ali
// @description    Adds movie ratings and number of voters to any IMDb link. Modified version of http://userscripts.org/scripts/show/9174
// @include        *
// @version        2013-05-12
// @namespace      http://userscripts.org/scripts/show/96884
// @grant          GM_xmlhttpRequest
// @downloadURL    http://www.alibakir.com/upload/addimdbratings.js
// @updateURL      http://www.alibakir.com/upload/addimdbratings.js
// ==/UserScript==
var IMDBpluginlinks = document.links;
var IMDBcontinueelement=document.createElement("button");
IMDBcontinueelement.innerHTML="Get rating";

function processIMDBLinks(s){
    IMDBcontinueelement.style.display = 'none';
    var r=0;
    for (IMDBi = s; IMDBi < IMDBpluginlinks.length; IMDBi++) {
        if (IMDBpluginlinks[IMDBi].href.indexOf("/title/") != -1 && IMDBpluginlinks[IMDBi].href.indexOf("imdb.") != -1){
            if(r>300){
                IMDBcontinueelement.onclick=function(){ processIMDBLinks(IMDBi); };
                IMDBcontinueelement.style.display='inline';
                IMDBpluginlinks[IMDBi].parentNode.insertBefore(IMDBcontinueelement, IMDBpluginlinks[IMDBi]);
                break;
            }
            r++;
            GM_xmlhttpRequest({
                method: 'get',
                headers: {
                },
                url: IMDBpluginlinks[IMDBi].href,
                onload: function (IMDBi){return function(result) {
                     rating = result.responseText.match(/Users rated this (.*) \(/);
                     votes = result.responseText.match(/\((.*) votes\) -/);
                     IMDBpluginlinks[IMDBi].parentNode.insertBefore(document.createElement("span"), IMDBpluginlinks[IMDBi]).innerHTML = (rating ? "<b> [" + rating[1] + " - "+votes[1]+"] </b>" : "<b style='color: red;'>[NA] </b>&nbsp;");
                }}(IMDBi)
            });
        }
    }
}
processIMDBLinks(0);


And this is how an example page looks at the moment:

Results being added to the wrong links


As you can see, the script displays the results in the wrong places.

Why does it present them in the wrong place and how can it be fixed?

Brock Adams
  • 90,639
  • 22
  • 233
  • 295
Nikita 웃
  • 2,042
  • 20
  • 45
  • 1
    S.O. is a Q&A website to ask programming related questions and help each other resolve programming related problems. I did not request that anyone will write a code, I presented the code and am merely pointing out a problem in a script that shows the results in the wrong place and asking why it does that and how this can be fixed. This is well within the "what's allowed" rules of S.O. I believe. I think the question is excellent and answering it will certainly help other script writers in resolving similar "bad-output" issues, not to mention that this will help the userscript.org community... – Nikita 웃 Jun 01 '14 at 04:51
  • 5
    I tend to agree with the OP here. There's nothing inherently wrong with asking a question about a userscript. The only issue I can see is that the problem with the userscript appears to be so catastrophic that it would be extremely difficult to determine what the underlying problem is and how to fix it. But even then, that does not make this a bad question *per se*. – BoltClock Jun 01 '14 at 05:33
  • Thanks BoltClock. @BrockAdams the `@include *` is just telling the script it can run on any website, how this is a problem or related to my question? As for the `document.querySelectorAll`, the script is using `document.links` which seems to be the correct approach in what it is trying to do. How is `document.querySelectorAll` any better? – Nikita 웃 Jun 01 '14 at 05:35
  • @BoltClock, by the way, exactly `the problem with the userscript appears to be so catastrophic that it would be extremely difficult to determine what the underlying problem is and how to fix it` - is what makes this an excellent question. Again, thanks for your comment. – Nikita 웃 Jun 01 '14 at 05:38
  • 1
    @Brock Adams: At the bare minimum, it benefits anyone else who might happen to have the same userscript and therefore the same problem (at least for as long as the userscript author does not update it anyway). And more broadly, it could benefit other userscript authors, or even other JavaScript developers. There is no need to make this question more generally applicable - the script itself is already generally available at userscripts.org. Maybe the OP could show what they have tried - but calling it "gimme teh codez" is quite a stretch. – BoltClock Jun 01 '14 at 06:31
  • @BoltClock, that's correct, and there are many other userscripts based on the same or similar approach, thus the answers here will certainly help them as well. – Nikita 웃 Jun 01 '14 at 06:34
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/54865/discussion-between-brock-adams-and-creativemind). – Brock Adams Jun 01 '14 at 07:30

1 Answers1

2

The issue causing your main complaint (results in the wrong places) is that GM_xmlhttpRequest operates asynchronously (which is good) and the onload was improperly constructed.

You either need to wrap the call to GM_xmlhttpRequest in a proper closure or provide a context in the GM_xmlhttpRequest call. (See the code below.)

For more information on why closures are needed, see this answer to the same type of problem.


Other big problems include AJAX-fetching dozens of improper links and firing on every page and iframe. Both of these slow the browser down quite a bit.

Don't use @include *. Even if you don't mind, other users of the script will. Add @include or @match lines for just the sites that you know have IMDB links.

I thought that I might want to use this script myself, so I started cleaning it up. You can read the inline comments and compare to the original script for an idea of some of the lesser problems. (Don't use onclick and always check match returns, etc.)

// ==UserScript==
// @name         add IMDb rating next to all IMDb links (+voter count)
// @description  Adds movie ratings and number of voters to any imdb link. Modified version of http://userscripts.org/scripts/show/96884
// @match        *://www.imdb.com/*
// @grant        GM_xmlhttpRequest
// ==/UserScript==

var maxLinksAtATime     = 50; //-- pages can have 100's of links to fetch. Don't spam server or browser.
var fetchedLinkCnt      = 0;

function processIMDB_Links () {
    //--- Get only links that could be to IMBD movie/TV pages.
    var linksToIMBD_Shows   = document.querySelectorAll ("a[href*='/title/']");

    for (var J = 0, L = linksToIMBD_Shows.length;  J < L;  J++) {
        var currentLink = linksToIMBD_Shows[J];

        /*--- Strict tests for the correct IMDB link to keep from spamming the page
            with erroneous results.
        */
        if (    ! /^(?:www\.)?IMDB\.com$/i.test (currentLink.hostname)
            ||  ! /^\/title\/tt\d+\/?$/i.test (currentLink.pathname)
        )
            continue;

        if (! currentLink.getAttribute ("data-gm-fetched") ){
            if (fetchedLinkCnt >= maxLinksAtATime){
                //--- Position the "continue" button.
                continueBttn.style.display = 'inline';
                currentLink.parentNode.insertBefore (continueBttn, currentLink);
                break;
            }

            fetchTargetLink (currentLink); //-- AJAX-in the ratings for a given link.

            //---Mark the link with a data attribute, so we know it's been fetched.
            currentLink.setAttribute ("data-gm-fetched", "true");
            fetchedLinkCnt++;
        }
    }
}

function fetchTargetLink (linkNode) {
    //--- This function provides a closure so that the callbacks can work correctly.

    /*--- Must either call AJAX in a closure or pass a context.
        But Tampermonkey does not implement context correctly!
        (Tries to JSON serialize a DOM node.)
    */
    GM_xmlhttpRequest ( {
        method:     'get',
        url:        linkNode.href,
        //context:    linkNode,
        onload:     function (response) {
            prependIMDB_Rating (response, linkNode);
        },
        onload:     function (response) {
            prependIMDB_Rating (response, linkNode);
        },
        onabort:     function (response) {
            prependIMDB_Rating (response, linkNode);
        }
    } );
}

function prependIMDB_Rating (resp, targetLink) {
    var isError     = true;
    var ratingTxt   = "** Unknown Error!";

    if (resp.status != 200  &&  resp.status != 304) {
        ratingTxt   = '** ' + resp.status + ' Error!';
    }
    else {
        if (/\(awaiting \d+ votes\)|\(voting begins after release\)|in development/i.test (resp.responseText) ) {
            ratingTxt   = "NR";
            isError     = false;
        }
        else {
            var ratingM = resp.responseText.match (/Users rated this (.*) \(/);
            var votesM  = resp.responseText.match (/\((.*) votes\) -/);

            if (ratingM  &&  ratingM.length > 1  &&  votesM  &&  votesM.length > 1) {
                isError     = false;
                ratingTxt   = ratingM[1] + " - " + votesM[1];
            }
        }
    }

    var resltSpan       = document.createElement ("span");
    resltSpan.innerHTML = '<b> [' + ratingTxt + '] </b>&nbsp;';

    if (isError)
        resltSpan.style.color = 'red';

    //var targetLink      = resp.context;
    //console.log ("targetLink: ", targetLink);

    targetLink.parentNode.insertBefore (resltSpan, targetLink);
}

//--- Create the continue button
var continueBttn        = document.createElement ("button");
continueBttn.innerHTML  = "Get ratings";

continueBttn.addEventListener ("click", function (){
        fetchedLinkCnt              = 0;
        continueBttn.style.display  = 'none';
        processIMDB_Links ();
    },
    false
);

processIMDB_Links ();
Community
  • 1
  • 1
Brock Adams
  • 90,639
  • 22
  • 233
  • 295
  • Thanks Brock. I tried your script on Chrome TemperMonkey, just for testing, and it doesn't work. Does it supposed to be working? – Nikita 웃 Jun 02 '14 at 11:37
  • Oops. Forgot that [Tampermonkey does not implement `context` correctly](http://stackoverflow.com/a/18435730/331508). Any chance you 'll switch to Firefox? ;) I'll rewrite the answer to use a closure in a bit. – Brock Adams Jun 02 '14 at 20:54
  • Switched the answer to using closures. But beware that this makes the script much more memory and CPU intensive. Do not set `maxLinksAtATime` too high (20 to 50 seems to cover almost all pages). – Brock Adams Jun 02 '14 at 21:24
  • Nice! :) It does work now obviously. I also headed to TM forum to file a report/request, but saw you have beaten me to it, thanks for that too! That said, your script does still show some bogus results in the wrong place, example: http://grabilla.com/04603-4c508f4b-a246-475b-bf1e-2ee806922d9a.html# Any chance you can fix that? – Nikita 웃 Jun 03 '14 at 01:17
  • I saw your image and spot-checked several of those "bogus" results. They aren't. They're how that script was *always* (poorly) designed to operate. For example, take [this movie](http://www.imdb.com/title/tt3315342/). Now, view the source of that webpage and search for `Users rated this`. That's what your script does, but it's the wrong approach and doesn't always yield the right results. ... **This is a separate problem from your question** -- which is answered. Redesigning the script to use a more correct page-scraping technique is for a new question or paid programmer. – Brock Adams Jun 03 '14 at 01:45
  • I fixed it by simply switching the IFs and adding `in development` to the or condition, so would be great if you edit your answer with this: http://grabilla.com/04603-e57de669-c145-495a-9d7b-9192240a6182.html# and here is the result: http://grabilla.com/04603-d6ed8a34-94f8-45f7-ac5e-eac977de2fd5.html# – Nikita 웃 Jun 03 '14 at 02:32
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/54954/discussion-between-brock-adams-and-creativemind). – Brock Adams Jun 03 '14 at 02:50
  • Thanks Brock, I have replied you in the chat. – Nikita 웃 Jun 03 '14 at 07:55