Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Updated: Comment #11
Problem/Motivation
Under some circumstances, comment reply links, other comment-related links, and comment forms are rendered into the search index and search results. They shouldn't be.
To reproduce:
1. Grant permission to anonymous users to add and view comments.
2. Add a node with comments enabled.
3. Add a comment to the node.
4. Run cron to index.
5. Search for 'reply' or 'permalink'. Ideally, you should not find your node.
6. Search for something that is in the node. You should not see any reply/edit/delete links in the results or text from the comment form.
Test results
Bug 1: In step 5, you can search for "reply" or "permalink" and find the node. So, these are being added to the search index. (You cannot search for "delete" or "edit". The search index is built based on what an anonymous user would see, and they do not have comment admin.)
Bug 2: In step 6, if you search for something near the end of the node body, you will see "permalink" in the results. If you search for something near the end of the comment body, you will see "reply edit delete" in the results. [As of August 20, 2014 this part is not a problem any more in Drupal 8; still a problem in 7]
This is a problem in Drupal 8 and Drupal 7.
Proposed resolution
Make sure these links and text are not included when the Comment module renders comments for search indexing and search results.
Remaining tasks
Patch, with tests.
User interface changes
API changes
Better searching, without "reply" and "permalink".
Original report by @pwolanin
Seems that comment module does a poor job of implementing hook_node_update_index(). I see that the text "Log in or register to post comments" and "Permalink" are rendered into the text for the node as sent to the search index.
This is really a comment module bug, but it's only relevant to search, so marking it for search.module
Perhpas comment module needs an addition module (not just full/teaser) for comment_view_multiple()?
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | 1113832-test-only-25.patch | 3.04 KB | jhodgdon |
Comments
Comment #1
jhodgdonThis issue seems to be related to both
#903392: "Add new comment" text appears in search result snippet
and
#721374: "Add new comment" is put into the search index along with each node
Both of these are marked closed/fixed. I thought this issue would also have been fixed along with those, but maybe not.
Comment #2
pwolanin CreditAttribution: pwolanin commentedTrying now with core node search I see the text "delete edit reply" at the end of search results, which seems to be the same problem (comment links are rendered). Probably comment module re-renders the results.
For apachesolr we currently build a snippet using just what's in the search index.
I can see evidence of the same bug since core node search matches all nodes with comments when I search for the the words "register" or "register post", but the words don't appear in the snippet.
The call chain causing the problem seems to be:
comment_node_update_index()
comment_view_multiple()
comment_view()
comment_build_content()
comment_links()
So the bug is multi-fold:
$node->rendered .= ' ' . module_invoke('comment', 'node_update_index', $node);without being able to set the $view_mode to 'search_result'.Comment #3
pwolanin CreditAttribution: pwolanin commentedHere's a minimal patch - it fixes #1 and #2 in the list above. #3/#4 might be considered a feature request and are not essential for resolving the bug.
Comment #4
pwolanin CreditAttribution: pwolanin commentedfor fun, here's a git formatted patch.
Comment #5
jhodgdonThe patch looks like it should fix the problem. Perhaps a test?
The test should:
a) add a node with comments enabled
b) make sure if you search for 'register' that you don't see the node in the results
c) make sure if you search for something that is in the node, you don't see the reply/edit/delete links in the results (if you have comment admin privs).
Comment #6
jhodgdonI just tested this in a somewhat recent Drupal 8.x and also in 7.23.
a) If you have content without any comments, there is nothing bad happening. You don't see anything about adding comments when you search.
b) If you add a comment to the content (and run cron to index), however, you do see the "reply delete" from the comment links in the search results page. This is true in 7 and 8. So we need to port this patch to 8, add a test for it, and then backport the test and patch to 7.
c) As a note, you cannot search for "reply" or "delete". So this is only a problem in the rendering of search results, and these are not getting into the index.
Comment #7
jhodgdonComment #8
jhodgdonI also noticed that if you have the "count page views" turned on in the Statistics module, and you somehow manage to get it to count your page views (that feature of statistics.module is currently broken but you can do a database insert to simulate it), then you get "10 views" at the start of your search result snippet. I'm not sure whether this gets into the search index or not.
I've also noticed that the node title is shown at the end of the search result snippet, which is odd to say the least.
So this all seems related but we could file a separate issue I guess... Really we need a generic way to deal with modules that add stuff to the node view array that would be silly to show in search results snippets.
Comment #9
jhodgdonI had a thought this morning and revisited the testing I did in #6. It's worse than I thought...
If anonymous users have permission to add comments, then "Reply" is actually added to the search index, along with "permalink". If anonymous users do not have the ability to add comments, then you don't get "Reply" in the search index, but assuming they have the ability to view comments, "permalink" is added to the index.
This isn't very good.
Comment #10
jhodgdonThis is most likely going to be fixed for 8.x in
#2141929: Comment link or form is added to print view mode.
hopefully anyway! When that gets in we should retest this issue.
Comment #11
jhodgdon#2141929: Comment link or form is added to print view mode. went in and I retested, updated the issue summary.
This is no longer a problem in Drupal 8.x, since that fix went in.
Probably it will vanish from 7.x when/if the patch is ported, but I'll leave this issue open in 7.x until then.
Comment #12
jhodgdonWhoops. I forgot a crucial step in testing: adding a comment to the node. So, I retested. Still a problem in 8.x. Updated summary.
Comment #13
jhodgdonHuh. It looks like this will most likely be fixed in 8.x by:
#1901110: Improve the UX for comment bundle pages and comment field settings
and/or
#1920044: Move comment field settings that relate to rendering to formatter options
which I think should add settings that will allow us to turn off the links when we're in the search index/results view modes.
So I'm postponing this again... I've asked on those issues what the plan is. If they are not planning to put in settings for this, we'll need to put in a kludgy fix instead.
Comment #14
jhodgdonWell, now [#19901110] has been closed as a duplicate of #1920044: Move comment field settings that relate to rendering to formatter options, and the latter issue is not looking like it will get fixed in anything resembling a timely manner.
So I guess we need to kludge this instead. Sigh.
Comment #15
andypostComment #16
andypostComment #17
jhodgdonI wrote a test for this today. It looks like not all of the bugs in the issue summary are still there in 8.x [updated summary], but this test should ensure that they don't come back (assuming we fix the remaining bugs and commit this test and the patch).
For fixing this, we may have a lesser kludge to do if we wait for #2271349: Node and Comment ops links should be display components (which can be disabled) to land. This adds the ability for comments to have a display mode that would exclude links. However, we'd still have to do something of a kludge, to make it so that if you are rendering a node in one of the search-related display modes, you'd somehow make sure the comments were rendered in a similar display mode. This association is not present in the current patch on that issue.
Comment #19
jhodgdonGood, the test bot agrees. This test fails in the expected way. Setting back to Active since this patch is not fixing the problem, only presenting a viable test.
Comment #20
jhodgdonHopefully we can fix this along with #2324719: Node indexing - should use view mode for comments, not hook
Comment #21
moshe weitzman CreditAttribution: moshe weitzman at Acquia commentedPretty sure this is fixed. See \Drupal\comment\CommentLinkBuilder::buildCommentedEntityLinks
Comment #22
jhodgdonOK, let's run the test above again and see if it passes.
And then if it does, let's DEFINITELY add this test so it doesn't break again! This has been a long-standing bug.
And then we still need to backport something to D7 if possible...
Comment #25
jhodgdonTest fail due to reroll needed... straight reroll...
Comment #28
jhodgdonAccording to the test results, "reply" and "permalink" are still being added to the index: the test tries searching for these terms, and the search is returning a node result (it shouldn't).
So it appears that this issue was not actually fixed yet.
Comment #29
jhodgdonThe function cited in #21, \Drupal\comment\CommentLinkBuilder::buildCommentedEntityLinks, is for building the entity-level links (e.g. node-level) -- like "jump to first comment" and "add new comment". It's not about building the links on each comment, like "permalink" and "reply" (the subject of this bug).
So this function is (rightfully) not outputting the node-level links if the node is being rendered in search_index view mode, which is good, but it doesn't fix this bug.
Comment #36
apadernoComment #40
catchPostponing this on #1920044: Move comment field settings that relate to rendering to formatter options again, we should fix that, then add test coverage (and possibly default display settings) for the specific bug here. We didn't get the kludge fix done six years later, so concentrating efforts on the longer term fix won't necessarily be slower.