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.
What are the steps required to reproduce the bug?
- Create a node with only few words in the body.
- Run cron so that the page is indexed for the search.
- Search for the node with any of the words you entered in the body.
What behavior were you expecting?
To see the node body text and possible taxonomy terms in the snippet. This is the behavior in D6.
What happened instead?
I see also an "Add new comment" text in the snippet. See the attached screenshot. This doesn't seem to serve the purpose of a snippet.
Comment | File | Size | Author |
---|---|---|---|
#16 | 903392-withfix.patch | 2.9 KB | jhodgdon |
#14 | 903392-failtest.patch | 1.95 KB | jhodgdon |
#8 | 903392.patch | 1.35 KB | rayasa |
#1 | 903392.patch | 1.29 KB | rayasa |
bug - d7 - search result snippet - add new comment text.png | 20.13 KB | apaatsio |
Comments
Comment #1
rayasa CreditAttribution: rayasa commentedNot only "Add new comment" link but you will also get the read count prefixed in the excerpt if you are counting page reads.
The patch disables sorting of $build during drupal_render so that if there are links added by other modules they will only show up (if they have to) after the body. And also, it disallows comment module to add the "add new comment" link if view mode is 'search result'. Not sure if page read count be allowed in the excerpt.
Comment #2
tanoshimi CreditAttribution: tanoshimi commentedThe attached patch seems to resolve the issue - thanks rayasa - but I'm wondering whether it's the best way/place to implement it...
When I encountered the same problem as the OP, I expected (but failed) to fix the "Add Comment" link being displayed in search snippets by visiting the "Content Type" configuration page at admin/structure/types/manage/page/display.
On the "Manage Display" tab here, you can define custom display settings for how the labels and formats of fields should appear in the "Search index" and "Search result".
However, on the "Comment Display" tab, you don't seem to get equivalent options for how comments (or, the "Add new comment" link) appear in Custom Display modes. This would seem to be a natural place for a checkbox to allow users to choose how comment-related links should be styled in search results and indexes, making them consistent with the handling of other fields attached to nodes
Just my thoughts, anyway!
Comment #3
tanoshimi CreditAttribution: tanoshimi commentedActually, scrap that last comment - I was thinking about how the comment body itself is included in search indexing/results, which is a separate issue.
I can't think of any reason why you'd ever want to display the "Add Comment" link in a search result snippet or index it!
So, I'm all for approving the patch in #1. I'll let someone more experienced than me determine if it should be RTBC or not though. And should this be in the Comment issue queue?
Comment #5
jhodgdonThis is related to an earlier issue that I can't find now, where this line was put in:
It seems perfectly reasonable to me to amend it like this patch does:
However, the next hunk of this patch is missing a period at the end of the comment:
And what is this doing -- is this bit left over from another patch?
???
Comment #6
jhodgdonHere's the earlier issue that removed "add new comment" from search indexing, if anyone's interested...
#721374: "Add new comment" is put into the search index along with each node
Comment #7
jhodgdonAlso we should add a test to this patch, so it doesn't break again.
Comment #8
rayasa CreditAttribution: rayasa commentedI don't see how this issue is related to the one in #6. Here, the string "add new comment" is never added to the search index ( i.e. The fix for the issue in #6 is not broken yet. ). The issue here is with the construction of the search excerpt and this patch removes "add new comment" string from getting into the search result snippet.
The second hunk of the patch ($build['#sorted'] = TRUE;) is well intended and not by mistake. I see no reason why we should allow drupal_render to sort the build meant for 'search result' view. The body of the result should always be put before the links and these get switched if we sort.
Resubmitting the patch against head...
Comment #9
jhodgdonThat sorting is a separate issue -- please file it as a separate issue rather than combining it here with this unrelated problem.
And this patch still needs a test.
Comment #10
rayasa CreditAttribution: rayasa commentedThank you... will file a separate issue. Test coming up (my first one).
Comment #14
jhodgdonHere's a patch containing just a test, which fails. I will also attach a patch with the fix, in the next comment.
Comment #15
jhodgdonComment #16
jhodgdonHere's the above fix from #8 (without the unrelated issue), and the test from #14, which should now pass (at least, it does for me).
Comment #17
jhodgdonThis is a D6 regression. I think it should be fixed before release.
Comment #18
pwolanin CreditAttribution: pwolanin commentedIt might be better to flip the logic and only add the link if we are in a normal/full display.
Comment #19
jhodgdonIt might be better -- I'm not sure. Here's the current function:
http://api.drupal.org/api/drupal/modules--comment--comment.module/functi...
It's going through several special-case view modes, to decide whether to print out the "add new comment" and "N comments so far" links:
- RSS
- teaser
- default -- which we patched in #721374: "Add new comment" is put into the search index along with each node to be "everything but search indexing", and the proposed fix here is to make it "everything but search indexing and search snippeting".
I'm not the maintainer of the node or comment modules, and I'm not sure what other "view modes" there might be -- should the default for unknown view modes be that they act like the default view mode (which is to say, print out the links), or should the default be to omit the links? I'm inclined to say the default should be to print out the links (as is done in this patch), but I'm not sure....
Comment #22
pwolanin CreditAttribution: pwolanin commentedI'd tend to think we should go for a no-link default for unknown modes.
Comment #23
jhodgdonI'm OK with that idea, but I think we should get a philosophical OK from webchick/Dries before we bother with a patch to make that change at this point in D7.
Comment #24
jhodgdonpwolanin and I investigated this in IRC today. We discovered that in D6 the add new comment link is added by comment_link(), i.e. hook_link(), which is invoked independent of build mode from node_view().
In D6, however, search indexing and search results do not use node_view(), which is why they don't show the add new comment link (hook_link is not invoked for either one, and taxonomy and comments are invoked in a special way, and there are some special nodeapi hooks as well).
So in D7 this is a regression that it is showing, but only for the case of search results, and we should probably leave the default as it was in D6 -- show comment links generically, and make exceptions for search results/indexing.
We took care of indexing previously, and this patch takes care of results... so I think it's OK as it is?
Comment #25
pwolanin CreditAttribution: pwolanin commentedYes, seems like this is the most reasonable fix at this point.
Comment #26
webchickCommitted to HEAD. Thanks!