Forum Replies Created

Viewing 1 replies (of 1 total)
  • Thread Starter flibbles

    (@flibbles)

    Right, yeah. I’ve seen the source.

    It looks like what I’m describing could be easily accomplished by adding the following:

    
    // Remove the subscribe checkbox for non-logged in users.
    remove_filter('comment_form_default_fields', 'cren_comment_fields');
    
    // Remove the existing POST handler and replace it with one that defaults to 'off'
    remove_action('comment_post', 'cren_persist_subscription_opt_in');
    add_action('comment_post', 'cren_persist_subscription_opt_in_alternate');
    
    // Also tests if an email is available at all
    function cren_persist_subscription_opt_in_alternate($commentId) {
        $value = 'on'
        if (isset($_POST['cren_subscribe_to_comment'])) {
            $value = $_POST['cren_subscribe_to_comment'] == 'on') ? 'on' : 'off';
        } else {
            $value = (($comment->comment_author_email != null)  ? 'on' : 'off';
        }
        return add_comment_meta($commentId, 'cren_subscribe_to_comment', $value, true);
    }
    

    And this would work for me, but what I’m saying is I think your plugin is ignorant of the fact that wordpress can be configured so emails are optional.

    If a user has emails configured as optional, and a commenter doesn’t give one, and they don’t bother unchecking the subscribe field, then your cren_comment_notification hook will still try to call wp_mail with an empty address. I think that fails with an email error, or it will email whomever any ‘wp_mail’ filters specify (such as CCing an admin), but not a recipient.

    If cren_comment_notification had the code:

    
    if ($email == null) {
        return false;
    }
    

    …on line 64 of cren_plugin.php, then the comment notification would exit more gracefully. Also, my plugin won’t try to email blank addresses for all the comments made from before I installed your plugin.

    See what I mean?
    -Flibbles

    • This reply was modified 5 years, 9 months ago by flibbles.
Viewing 1 replies (of 1 total)