archon810

Hey Tyler,

I’m guessing they did it because (and I’m making an assumption here) the comment counts displayed are coming from WP and not Disqus, which has the potential of being severely out of sync.

Ideally, Disqus would apply the same logic (if it doesn’t already) as it does on the main site and replace the comment counts with the JS calls to the Disqus API – is it doing it if the code you mentioned is removed?

I am an active member of the WPTouch forums and have communicated with their team in the past, so I’ll try to get them to take a look at this issue.

Thanks, Tyler.

Pull request submitted: https://github.com/moskis/fancybox-for-wordpress/pull/1

Hi Tyler, I was looking at pull requests earlier today actually and while I have a github account, I never actually dealt with pull requests before, and it wasn’t immediately obvious how to create one. I do want to figure it out though, so let me try to submit one for you.

As for the fork, I saw that some functions were exactly the same, including the one that got patched by WP, so the way I would have done it is only clone the necessary functions that you changed. Although it does make it safer if WP touches one of the functions that you call, it really isn’t any different than relying on using any other core functions – anything can change. As I said, I’m not sure Disqus is following all changes to the original export.php, so by cloning only the functions you need would have netted you not having to do anything at all here.

Alternatively, maybe not forking at all and using filters instead would have been even better and cleaner.

Tyler, I’m not sure that solution in the comment on that request would be ideal either – I’ve just replied there with my thoughts. Here they are, for posterity:

Tyler, what you’re saying is better, but if the close action is fired from WordPress upon closing comments, that means existing closed comments wouldn’t be supported by this if you’ve migrated to Disqus after having an established comment base.

Disqus should perhaps try to detect whether a given post has comments closed in WP but not in Disqus, and if they’re not closed on the remote side, issue a close action. Not sure what the best way is, but simply setting a flag that gets passed to the JS form would be a good start. The problem is, spammers can figure this out and unset the flag, unless you have a security mechanism in place that uses hashed API secret/key to pass parameters to the form.

Tyler, I looked through the code, and I’m not sure I follow its logic.

What is the expected result here? I would expect to see a Disqusified comment list with the exact same words “Comments are closed” as you’d get if you closed comments from within Disqus.

Looking through the code, I have a suspicion that instead the intention is to not load Disqus at all and rather show the theme’s comment template instead. I may be missing some logic, but the functions I was looking at are dsq_comments_template() and dsq_can_replace().

Specifically, I verified that $post->comment_status in dsq_comments_template() returns “closed” correctly. The logic then doesn’t enter either if statement and proceeds to set $EMBED=true (see below, I added a few debug statements, out of which only dsq1 was printed).

The code enters dsq_can_replace() in the 2nd if clause and returns true at else if ( 'all' == $replace ) { return true; } meaning Disqus can indeed replace and the form is embedded.

This is confusing behavior, but then again, I’m not sure what the end result should be – in my opinion the correct logic should be to pass an additional parameter to the JS form that embeds comments which would then disallow showing of the comment entry field but otherwise show all the comments from Disqus and *not* the local db.

If the idea behind the logic is indeed what I think it is now, the theme comments would be shown which means comments from the local database, which can be vastly out of sync compared to Disqus due to moderation, editing, spamming, etc. Either way, neither behavior is happening, and the Disqus form shows itself with an open comment box.

If you need to take a look at all the variables (including disqus_replace = all), I pasted them in the original report.

// ugly global hack for comments closing
$EMBED = false;
function dsq_comments_template($value) {
    global $EMBED;
    global $post;
    global $comments;
error_log("dsq1: " . $post->post_title . " " . $value . " " . $post->comment_status);
    if ( !( is_singular() && ( have_comments() || 'open' == $post->comment_status ) ) ) {
error_log("dsq2: " . $post->post_title . " " . $post->comment_status);
        return;
    }

    if ( !dsq_is_installed() || !dsq_can_replace() ) {
error_log("dsq3: " . $post->post_title . " " . $post->comment_status);
        return $value;
    }

    // TODO: If a disqus-comments.php is found in the current template's
    // path, use that instead of the default bundled comments.php
    //return TEMPLATEPATH . '/disqus-comments.php';
    $EMBED = true;
    return dirname(__FILE__) . '/comments.php';
}