Comment indexing does not include the fully rendered comment

The core Search module does not search comments at all. The "content" tab of core Search only searches node content, which includes CCK fields if you are using CCK. The "users" tab only searches registered user names, and not any other information about the user (such as profile fields, email address, etc.). That is how the core Search module is designed, and in my opinion it is not a critical bug if Search is behaving the way it was designed to behave.

You can also write your own module to search comments using the Search API (search for hook_search() on api.drupal.org for more information). (Or maybe there already is such a module on drupal.org? I have no idea.)

Another alternative is to embed Google or some other 3rd-party search into your site (there are integration Drupal modules that will let you do this), as those search engines index the entire page as rendered. Or you could possibly use my Search by Page contrib Drupal module, which indexes pages as rendered within Drupal.

Anyway, I am converting this into a "support request", since Search is behaving the way it was designed to behave.

Also, I've closed #523680: Anonymous comment posters who used their names to post a message cannot find the name on search as a duplicate of this issue.

The usual procedure if an issue applies to more than one version:
a) File the issue against 7.x (or whatever the highest version number is that the issue applies to).
b) Note that it also applies to 6.x, 5.x, etc. (whatever other versions it applies to).
c) Monitor the issue, and if a fix is applied to 7.x, request that it be back-ported to 6.x (for example).

The reason for this is that we don't want to have discussion of the issue going on in two places, and usually the solution for one version of Drupal is very similar to the solution for another version.

Yes, maybe this should be changed to a Documentation issue.

It looks like the Handbook page for Search doesn't make clear that "content" includes just nodes. It probably should. That page can be edited by anyone: http://drupal.org/handbook/modules/search -- or you can file an issue under project "Documentation" asking for a correction to that page (include the URL).

For help text inside Drupal, probably the right place ito explain how Search works would be in some help text in the search module. So you could file an issue about that. It is unlikely to be changed for Drupal 6, because it would add text to Drupal that would need to be translated.

You could also file a Feature Request for the Search module and ask that the module be able to search comment text and comment authors as well as node content. This may have already been filed by someone.

I've updated the Search module's handbook page to make this clearer.

Comments *are* indexed with nodes, but they do not contain the name of poster. I agree with kaakuu that they should.

The core of the issue is that the comment_nodeapi() function only adds the following to the node:

$text .= '<h2>'. check_plain($comment->subject) .'</h2>'. check_markup($comment->comment, $comment->format, FALSE);

I see you fixed the Search handbook page accordingly - thanks!

Just as a note: you can see in http://api.drupal.org/api/file/modules/comment/comment.tpl.php/6/source what is rendered by default for a comment: submitted, content, and signature.

In http://api.drupal.org/api/function/template_preprocess_comment/6 you can see that
content is $comment->comment
signature is $comment->signature -- so this is also missing from the search index
submitted comes from http://api.drupal.org/api/function/theme_comment_submitted/6, which is (by default) user name (or the name an anonymous commenter entered) and a date stamp.

So it looks like the missing components are user name and signature (I don't think we'd want to index the date).

But isn't that policy consistent with how nodes are indexed? I don't think the "submitted" line is being indexed in nodes (whether or not the theme is displaying the "submitted" line for that content type), so if you search for a particular author's user name in the "Content" tab of Search, you won't find the nodes that person wrote. Correct?

Just some more food for thought...

But even non-anonymous posters of node content do not have their user names indexed with the node.

Now we have fieldable users, we need to look at indexing user account pages similar to how we do nodes, rather than a direct query of the {user} table for username (and e-mail if you're an admin). All the sites which had searchable profiles due to content_profile.module are going to lose that if they upgrade to fields on users.

catch: there are reasons besides wanting searchable profiles to use Content Profile, such as generally wanting the "profile" content to be nodes on the site. Unless users-with-fields behave exactly like nodes, then probably many Content Profile module users would want to stick with that module.

E.g.: Can the new users-with-fields have Taxonomy? Can they be used in Node-type views, Faceted Search, and other modules that expect to deal with Nodes? Do they theme using node.tpl.php or node-conten_type.tpl.php? Can you set up different profile types for different roles of users? These are all reasons I have used Content Profile in Drupal 6.x.

That said, indexing users like nodes are currently indexed does sound like a good idea.

E.g.: Can the new users-with-fields have Taxonomy?
Yes.

Can they be used in Node-type views, Faceted Search, and other modules that expect to deal with Nodes?
Views supports users as a base table in D6. It's unlikely the others work with this yet because they haven't been ported to D7.

Do they theme using node.tpl.php or node-conten_type.tpl.php?
No, but user profile page theming has been improved afaik.

Can you set up different profile types for different roles of users?
Not yet, but someone should file a patch for different bundles for different kinds of users, it'd also be possible to do from contrib I think.

I'm sure there'll be a content_profile for D7 rather than just a data upgrade path - at least since no-one updated profile.module to fields yet, but it'd still be nice to make it as small/redundant as possible.

Let's try this, is necessary for fieldable comments / comment body as field anyway.

Is there a proposed fix that actually needs review here? I don't see a patch or a link to another issue with a patch.

I also agree with kaakuu that it isn't obvious that just having fields for comments will fix this issue, which is that if an anonymous poster posts a comment with contact information, this contact information isn't indexed for search in the core Search module.

Agreed, only the poster's name should be indexed, as the email address is supposed to be private.

Sorry, didn't actually attach the patch. Haven't tested this yet.

That sure looks like a sensible approach: do the same thing currently being done for node_view() to render the comments.

The only exception is that comment_node_page_additions() also prints out the "add a new comment" form, which should probably not be indexed for searching. There's also some logic in that function to print something extra the current user can administer comments, which seems potentially dangerous (could it index unpublished comments? not sure what the permissions are during cron, but I think it's permissive). So I think you'd want an arg to comment_node_page_additions that could modify the output a bit if it's being used for search indexing.

Thanks for the review.

Now forcing anonymous user in drupal_cron_run() and added a $form parameter to comment_node_page_additions().

The anonymous part is a duplicate of #431776: Cron should run as anonymous when invoked via the run-cron link on the status report page. And of course, we want to avoid saving the session data in that case ;)

Moved that hunk to the other issue.

Almost there. I think in the last line of your patch, you need to pass FALSE in for the new second argument to comment_node_page_additions(). :)

One more issue with the patch. If you add a parameter to a function, you need to put a new @param section in its doc header. Sorry I missed that in the previous reviews. Other than that, it looks good, from a looking-at-the-code perspective anyway.

Have you tested it to see that it solves the problem? It might be good to have a test for this added to comment.test or search.test as well.

@jhogdon, good point. When writing the phpdoc, I realised the parameter only hides the form, it won't necessarily show it since the default is still dependent on comment settings, so changed it to $hide_form.

I haven't tested this and probably won't have time to get to that, just wanted to get the ball rolling in this case.

OK, from an examine-the-code perspective, I like your current patch. Now if someone wants to test it and see if it solves the problem... and better yet write a SimpleTest test for it... :)

#537746: fields on comments are not indexed marked as duplicate.

On that issue, yched pointed out the performance issues with large numbers of comments (no paging here obviously) which this patch would make worse. The only trade off we might get is if comment body is converted to a field - then we save the check_markup() for each comment in due to field caching (but lose some PHP efficiency), but I've seen a cron run cause 25,000 queries or more in D6 and we have a higher number of words to index too, so it's not going to help much if at all.

We should still try to get this into Drupal 7.
Bump.

So.

Things have "somewhat" changed since this issue was filed, and comment_node_view() doesn't call comment_node_page_additions() during search indexing at all any more. Instead, the comments are added to the search index by comment_node_update_index().

It looks like this is just calling comment_view_multiple() to prepare the comments, so it's probably rendering the user name.

So I bet this issue is not an issue any more... anyone care to re-test?

I just tested this on D7.

I have verified that the name the person enters is indexed along with the comment.

So I am going to close this issue at this point, as it appears to be fixed.