I get the following WSOD-inducing error when I try previewing a comment as an anonymous user in Drupal 8 dev (as of Oct. 13, 2013):

Fatal error: Call to a member function id() on a non-object in core/modules/comment/comment.module on line 1371

It looks like comment.module is still using global $user;, which won't work properly (I don't think) for anonymous users with methods like ->id().

This is interfering with my ability to write tests for the Honeypot module; see: #2036501: Don't run Honeypot checks on form previews.

Files: 
CommentFileSizeAuthor
#8 comment-preview-anon-2111163-8.patch1.91 KBgeerlingguy
PASSED: [[SimpleTest]]: [MySQL] 59,203 pass(es).
[ View ]
#3 comment-preview-anon-2111163.fail_.patch1.25 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] 58,519 pass(es), 7 fail(s), and 4 exception(s).
[ View ]
#3 comment-preview-anon-2111163.pass_.patch1.88 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 58,700 pass(es).
[ View ]

Comments

Yes, it does. It's $account, not global $user that is accessed here, and there are multiple things that could be assigned to it.

It's more likely that the user_load_by_name() fails and returns NULL.

Assigned:Unassigned» larowlan

fixamakating

Assigned:larowlan» Unassigned
Status:Active» Needs review
StatusFileSize
new1.88 KB
PASSED: [[SimpleTest]]: [MySQL] 58,700 pass(es).
[ View ]
new1.25 KB
FAILED: [[SimpleTest]]: [MySQL] 58,519 pass(es), 7 fail(s), and 4 exception(s).
[ View ]

Should be fail/pass.
Thanks for reporting!

Status:Needs review» Reviewed & tested by the community

Great!

Is it possible to also add a test of an anonymous user posting a preview comment, so this doesn't sneak in again?

Nevermind, I noticed you have it post a comment by the anonymous user with preview required. That suffices! RTBC +1.

So the resulting code in comment_preview() with the patch is:

<?php
   
// Attach the user and time information.
   
if (!empty($comment->name->value)) {
     
$account = user_load_by_name($comment->name->value);
    }
    elseif (
$user->isAuthenticated() && empty($comment->is_anonymous)) {
     
$account = $user;
    }
    else {
     
$account = drupal_anonymous_user();
    }
    if (
$account->isAuthenticated()) {
     
$comment->uid->target_id = $account->id();
     
$comment->name->value = check_plain($account->getUsername());
    }
    elseif (empty(
$comment->name->value)) {
     
$comment->name->value = \Drupal::config('user.settings')->get('anonymous');
    }
?>

It *feels* like this could be simplified somehow, but I couldn't actually think of a way how, so just posting here for reference.

Patch totally looks good.

Status:Reviewed & tested by the community» Needs review

We never actually use anything from drupal_anonymous_user(), so it looks like we could change this to something like:

<?php
   
// Attach the user and time information.
   
if (!empty($comment->name->value)) {
     
$account = user_load_by_name($comment->name->value);
    }
    elseif (
$user->isAuthenticated() && empty($comment->is_anonymous)) {
     
$account = $user;
    }
    if (!empty(
$account) && $account->isAuthenticated()) {
     
$comment->uid->target_id = $account->id();
     
$comment->name->value = check_plain($account->getUsername());
    }
    else {
     
$comment->name->value = \Drupal::config('user.settings')->get('anonymous');
    }
?>

StatusFileSize
new1.91 KB
PASSED: [[SimpleTest]]: [MySQL] 59,203 pass(es).
[ View ]

That makes sense.

Status:Needs review» Reviewed & tested by the community

Yep, looks great! Thanks.

Status:Reviewed & tested by the community» Fixed

Great work!

Committed and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

Fixed formatting.