Problem/Motivation
Users have an expectation that when they go to Manage Displays for a content type, if they customize the "Search Results" view mode, it would behave similarly to what would happen on other Manage Displays view modes: i.e., it would customize how search results items are displayed. But it doesn't. (See the Original Report section below for a narrative illustrating this expectation.)
What happens instead currently (in D8 and D7) is that the Search Results node view mode is used to decide what is rendered from the node that is found as a search result. That rendered content is then passed to the search_excerpt() function to derive a highlighted snippet for the node. And that snippet is the main body of what is passed to the search-result.html.twig template (or search-result.tpl.php in D7), and displayed on the search results page.
Proposed Resolution
Rename the Search Results view mode "Search Result Highlighting Snippet Input" or something like that (more concise!), to be less confusing.
Another thing that was discussed was exposing the snippet to Views as a "field" so that people can better manage the display of search results by making a Views-based search page instead of using the default. We can do this in a follow-up issue:
#2212321: Add highlighted search result snippet as "field" for Views
And while it would be great to actually let people add fields to the search results, that is also a follow-up issue.
#2212323: Make a way to better control search result display
A rejected resolution
a) There is actually not a strong need to have different content rendered into the search index and used to generate the highlighted result snippet (and you could make an argument that it's much better if they're the same). So, we can/should use the Search Index view mode to generate the snippet instead of having a separate Search Results view mode.Actually there is, see comment #117
b) We can then get rid of the Search Results view mode completely. (In Drupal 8, this definition is in core/modules/search/config/entity.view_mode.node_search_result.yml)
c) We need to expose the highlighted search snippet to Views. This will allow people who want full control over how search results are displayed to either:
- Use Display Suite (which apparently has this functionality) to govern how search results are displayed.
- Use Views to make their own search page with results formatted however they want with whatever fields they want.
- Do some programming in theme preprocess functions in a contrib module so that additional fields are added to the 'snippet' theme variable, and hence displayed in the output.
Other rejected resolutions
Ideally, it would be nice if the display of node search results was governed by a Manage Displays setting, so you could display fields with your search results (such as a thumbnail image). However, this might be difficult to accomplish, because much of what is currently being displayed by the result template file (or at least has the potential to be displayed) is not node fields at all, but "pseudo-fields" like the excerpt with keywords highlighted, the URL, the formatted user name of the author, the displayed name of the content type, the search score, etc. So there are a couple of ways this might be resolved:
1. The "pseudo-fields" could be added somehow to the Manage Display configuration screen. This doesn't seem all that feasible though. The way Manage Display works is that for each entity type (e.g., "node" in this case), you can define "extra fields" using hook_field_extra_fields(). But they apply to all display types -- there doesn't seem to be any way to define extra fields that are specific to one particular display type, and also the keys of the returned "extra fields" have to correspond to things that are present in the render array for the entity. So if we did this, we would I think lose the ability to have the existing search results stuff.
2. There could be a checkbox somewhere that gives you a choice, when a search result is being displayed, of either displaying the keyword-highlighted excerpt or the node rendered with your choice of display type. This output would then be passed to the search-result.tpl.php in the "snippet" field, as it was before.
3. There could be a way to use a display setting to add additional fields to the search output, which would be passed to and displayed by the search-result.tpl.php file in addition to what is there now. This is probably the most feasible option, but it seems like if we have good Views integration, we can just have people use Views if they want more customization and leave it at that.
Remaining tasks
Make a patch that renames the Search Results view mode so it is less confusing.
User interface changes
- The Search Results view mode will be renamed to something less confusing.
API changes
None.
Original report by RobLoach
There is a "Manage Displays" setting page to customize the Search Results page to show which fields will be shown on your content when someone searches for them. Unfortunately, it does not actually reflect what is displayed to the user.
Steps to reproduce
- User wants to customize the search results page for their node "Test #3":
- They visit the Manage Displays page for their article, use a custom display settings page for the Search Results at admin/structure/types/manage/article/display/search_result, and set up their Articles to have the needed fields on the search results page:
- Satisfied with their changes, they make a search, and see that managing the display of the Search Results doesn't actually manage the display of the search results:
Am I missing something in this workflow, or does this seem like an oversight for the search results page? Is theming search-result.tpl.php still the only way to do this?
Comment | File | Size | Author |
---|---|---|---|
#144 | 1166114-screen-UI-change.png | 77.24 KB | SidneyGijzen |
#144 | rename-search-result-label-1166114-144.patch | 902 bytes | SidneyGijzen |
#135 | drupal8-rename_view_mode-1166114-135.patch | 508 bytes | Rajendar Reddy |
#134 | 1166114-rename-view-mode-134.patch | 496 bytes | Rajendar Reddy |
#128 | 1166114-rename-view-mode-128.patch | 476 bytes | jhodgdon |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedLooking at:
http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...
we have :
So it would seem like node module at least is doing the right thing, so perhaps this is a bug in Field API?
Comment #2
pwolanin CreditAttribution: pwolanin commentedmaking the screenshots into attachments...
Comment #3
Agogo CreditAttribution: Agogo commentedThis bug is in Drupal 7 as well.
Subscribing
Comment #4
Agogo CreditAttribution: Agogo commentedThis issues really seems to be a bug. I cant find the cause from doing a quick scan of the modules involved.
However, for those that want a quick fix and likes Display Suite - there already is a solution.
Check this web page for hints on how to change the display of your display results:
http://blog.musicvm.com/media-image-diplay-suite-and-custom-search-music...
Hopefully someone else has the time in hand that is required to fix this issue in core.
Comment #5
jhodgdonHas this been verified using an unmodified core theme, and if so, which themes? Because if a theme ignores the display settings, I think something like this could happen.
I traced the node_view() code a bit. It eventually calls field_attach_view()
http://api.drupal.org/api/drupal/modules--field--field.attach.inc/functi...
passing in the display mode, which in turn ends up on:
http://api.drupal.org/api/drupal/modules--field--field.default.inc/funct...
which definitely omits rendering the field if the view mode is 'hidden'.
So something else must be happening.
Ummm... Actually, looking at your screen shots, it looks like you've only hidden the labels, not the fields? What are you seeing vs. what are you expecting to see?
Comment #6
tagawa CreditAttribution: tagawa commentedI'm having the same issue.
Summary:
Editing the search result display settings doesn't seem to have any effect.
I'm using Content types » Article » Manage display » Search result.
For articles, the settings are identical to my Teaser view, which works.
What I currently see:
Title, Trimmed content, Publication details (user - date - time)
E.g.: http://www.kyokoskitchen.com/search/node/%E3%82%B9%E3%82%B3%E3%83%BC%E3%...
What I want to see:
Title, Trimmed content, Thumbnail image
E.g. http://www.kyokoskitchen.com/articles/all
Environment:
Drupal 7.4, unmodified Bartik 7.4 theme. Also tried with unmodified Seven 7.4 theme. No caching.
Modules:
Chaos Tool Suite
CSS Injector
Global Redirect
Google Analytics
Pathauto
Redirect
Token
Views
Hope this helps - let me know if you need more details.
Comment #7
jhodgdonOK, looks like this needs more investigation, and it's probably a real bug.
Comment #8
jhodgdonNew duplicate report of this:
#1223384: Search Result Fails to use Custom Display Settings
Comment #9
jhodgdonComment #10
tomogden CreditAttribution: tomogden commentedAfter fiddling around some more, I noticed that if I show all the labels and set them to display "Above" their respective field data, it returns everything in a single text string, sans HTML formatting and surrounded by ellipses. Looks like it's stripping all the HTML tags (including the conspicuous ) and clipping it to size to render it as a $snippet, which is what is called in the search-result.tpl.php template.
It seems logical what we really need is a new template that will render the results fields more fully, right? I'm pretty sure the data is all there.
Comment #11
tomogden CreditAttribution: tomogden commentedSee attached screenshot of my test. The three fields needing to display are:
But as you see they are all run together into a single string.
Comment #12
tomogden CreditAttribution: tomogden commentedSee attached patch I made for D7. It uses the $rendered variable whenever the custom display is activated for the given node type. $rendered contains the HTML formated for the selected fields in the custom display settings.
Hopefully someone can port this to D8? Given the similarity in the bug behaviors, there should be some straight-across translation.
Any other feedback? I welcome criticism.
Comment #14
tomogden CreditAttribution: tomogden commentedFile paths set to website root.
Comment #15
tomogden CreditAttribution: tomogden commentedSet to 'needs review'.
Can anyone tell me what I need to set my file paths to in the patch to get the QA Checker to take it?
Comment #17
tomogden CreditAttribution: tomogden commentedPaths reset once more.
Comment #19
tomogden CreditAttribution: tomogden commentedCorrected missing index.
Comment #21
tomogden CreditAttribution: tomogden commentedCorrected potential empty reference.
Comment #22
xjmTagging issues not yet using summary template.
Comment #23
jhodgdonNot every issue necessarily needs a summary, actually...
Comment #24
xjm@jhodgdon: The tag is just meant to help contributors find issues that might benefit from summaries, especially major/critical RTBC/NR. It's in no way meant to denote a requirement or expectation. :)
Comment #25
xjmEr, and in this case most contributors can't actually write a summary because of the input format, so untagging.
Comment #26
catchI don't think we can or should change the templates like this after release.
What about removing all the code that generates 'snippet', and using the display fields settings for that instead?
Or alternatively check for custom settings for display fields for search results, and change $snippet to mean the same as $rendered if it's set (this would be much less of a shock for existing sites, and visually the same as now, just without the template changes).
This feels like search module got left behind in the Field API / Field UI conversion to me so we might want to consider removing snippet altogether in D8.
Comment #27
douggreen CreditAttribution: douggreen commented@catch, while this problem exists in d7 too, this issue is tagged d8, so it's not "after release" in that sense.
My two cents, I'm not sure the "right" way to do this, but passing "rendered" to the template doesn't feel right. I looked at node.tpl.php, which also uses view_mode's and it actually passes the view_mode to the template (although it doesn't use it). Although there's discussion in the search community on simplifying search, I think we're talking about simplifying search related functionality and not Drupal functionality. Can we just get the right field api for this template done for d8?
Comment #28
swentel CreditAttribution: swentel commentedI never understood why we use a different template for search results in the first place. We could - like catch suggests - just create a snippet field which is available as a field to display on the manage display screens. I'm using this technique in Display Suite as well (I provide an info and snippet field, the latter one currently for apache solr only since that's already available way earlier during the process of searching)
If people want different templates, we should add suggestions like node-#bundle and/or node-#bundle-#view-mode / node-#view-mode templates. That makes it *way* more consistent.
[And Imo, all specific templates should die, but that's another debate :)]
Comment #29
yoroy CreditAttribution: yoroy commented#26 and #28 seem to suggest interface changes. I can't really tell yet what these proposed solutions (snippet field?) would look like. Maybe it even helps clarify the technical solution if someone could clarify this with a mockup of sorts?
Comment #30
swentel CreditAttribution: swentel commented@yoroy, see attached screenshot. It doesn't really change the UI, it just adds new fields which you can drag into Field UI, in this case being the snippet and the search info (labels can be discussed of course). Also, on this screenshot, you see the title as well (this comes from ds in D7, but it's something that could be exposed as well in D8)
Comment #31
yoroy CreditAttribution: yoroy commentedThanks swentel
The 'snippet' label doesn't fit well with me. We already have teaser, summary and trimmed version floating around ( #193680: Consistent use of Teaser or Summary or Trimmed version or Trimmed post ) and well, from that perspective this wouldn't help make things clearer.
I think this issue needs a summary :)
Comment #32
swentel CreditAttribution: swentel commentedWell, teaser, summary are ways of formatting one big textarea field. A snippet isn't necessarily from one field, but a combination (that's how it technically works right now), so that's why it's a valid 'display' field (or whatever we come up with).
Comment #33
yoroy CreditAttribution: yoroy commentedmarked #1267076: search-result.tpl.php Ignores Render from Manage Display a duplicate of this.
Comment #34
jkaine CreditAttribution: jkaine commentedsubscribe
Comment #35
BarisW CreditAttribution: BarisW commentedsubscribe. #1286100: Display: Search Result not working is a duplicate of this as well.
Comment #36
rogical CreditAttribution: rogical commented+1 this feature is very important
Comment #37
kingswoodute CreditAttribution: kingswoodute commentedSubscribing - if anyone comes up with a fix for D7 that would be great.
Comment #38
jkaine CreditAttribution: jkaine commentedKingswoodute,
I've got a partial solution that's working well. It allows one to sync up the "Manage Display" settings for Search Returns with what's actually returned on the page. It doesn't however, solve some of the deeper Drupal issues with search returns.
In short, this should help with the display piece, but does not address the Drupal API questions that still linger.
1) Copy the search-result.tpl.php file from /modules/search/ into the templates directory in your theme.
Note: There is a search-result.tpl.php file and a search-results.tpl.php file. They work together. Copy the one without the ending "s"-- search-result.tpl.php
Note: Copy the tpl file into your theme-- don't edit it in /modules/search or the next time you upgrade Drupal core, you'll loose these changes.
2) In the file you copied to your theme, delete lines 65 to 78.
3) Paste the following code starting at line 65:
Your file should end at line 83 with a closing li tag. Here's the entire file, if it helps:
I've got this running in a semi-production environment for a D7.8 site, and it's working well.
Note: I haven't had much luck creating and posting patches.... and since this isn't really a patch but a theme/template implementation, I'm not sure if a patch is appropriate. If there's a better way to post this kind of code, please let me know.
Note: I expanded out all the print statements onto their own lines as my production version actually makes some site-specific modifications to the output. There's actually a lot of data in $variables['result'] that can be used to further customize your output.
Note: Ideally, I'd do any customization using the template.php file (or a custom module). However, I wasn't able to find any API hooks that affected this display-- and so I did it all through a TPL file. I'm thinking that this is the result of the search piece being left behind in the API upgrade in D7 (as suggested by catch in #26).
My two cents is that this is definitely an omission in the Drupal API (and supporting structures), and should be addressed in D8-- but that's easy for me to say as I have no idea how one might adjust core code to actually fix this.
I hope this helps.
Comment #39
erics14 CreditAttribution: erics14 commentedI thought I followed the edit instructions for search-result.tpl.php above but it still doesn't work - though I have defined the title field as a link once it is displayed twice in different sizes. I'm not sure where the second entry comes from.
How can I log this thread as a bug in D7?
What would it take to get this fixed properly in D7? Can someone name a bounty to implement (quickly) DS modifying the search results?
Comment #40
jkaine CreditAttribution: jkaine commentedEric,
This should work. I'm using it in a D7.10 site right now.
Two questions: 1) what theme (base theme) are you using? 2) Can you post your search-result.tpl.php file here? I'll take a look at it.
JK
Comment #41
erics14 CreditAttribution: erics14 commentedHi jkaine,
Thank you so much for responding. I'm running Bartik. I've attached my search-result.tpl file.
Regards,
Eric
Comment #42
jkaine CreditAttribution: jkaine commentedEric,
I'm not sure what's happening here. I don't use Bartik-- I'm an old-school Zen theme guy-- but I don't think that should make a difference.
Three things you might try:
One, make sure your cache is clear. (might be obvious, and if it is, I apologize).
Two, check your settings in the manage display for "Search Result" for your content type. (again, might be obvious).
Three, you might try commenting out the print render($title_prefix); and print render($title_suffix); lines. It could be that the extra link is getting set in the prefix code.
Sorry I can't help more.
Comment #43
erics14 CreditAttribution: erics14 commentedJkaine,
Your suggestion to look at the display fields got me thinking and after some testing I am pretty sure that with node searching there is no problem - I get a simple list of title links as search results. The problem is when I use SOLR.
Thanks again for your support.
Comment #44
msmithcti CreditAttribution: msmithcti commentedI'm probably overlooking something here but in my D7 theme I've simply added an
if
statement to search-result.tpl.php checking if the $module variable equals node. If so, I'm usingrender(node_view($result['node'], 'search_result'))
to print the node out in the search result view mode. If $module doesn't equal node then I just print the default code that was previously in search-result.tpl.php (for user/any other module).Is there something wrong with doing it this way?
Comment #45
dhayles CreditAttribution: dhayles commentedI get the following error when attempting the fix by jkaine on #38:
Notice: Undefined property: stdClass::$rendered in include() (line 80
The list of results do appear with only the title, but no image, My Nodes 'Managed Display' 'Search Result' has 6 fields including the image but only the title is being rendered.
Comment #46
efc84 CreditAttribution: efc84 commentedAre we any closer to a release fix?
Comment #47
Stephane Bouillet CreditAttribution: Stephane Bouillet commentedHello,
same same...
That's an important bug for a photographer site, when you are searching for ... images
So is there news about this bug ?
Thanks
Comment #48
Stephane Bouillet CreditAttribution: Stephane Bouillet commentedI've just tried the patch of the post #21 in Drupal 7.12, and it works for me ! (especially to display images in search results).
It works also with facetapi search motor instead of node search motor
Thank you !
Comment #49
shenzhuxi CreditAttribution: shenzhuxi commented#21 patch works in Drupal 7.12,
Comment #50
DamienMcKennaQuestion for, well, @webchick & @dries I guess: if a patch could be built for D7 that would a) make the node search results work like normal, b) provided a fail-over to use the existing search-result theming, would that be something that could get added? The alternative is to scrap core search for site development and just use SearchAPI.
Comment #51
Stephane Bouillet CreditAttribution: Stephane Bouillet commentedjust tried the patch #21 in drupal 7.14: still working ! ;)
Comment #52
catchI'm assigning this to David to see if he's got a view on what would make sense for a D7 backport.
For 8.x I think it makes sense to completely remove the search excerpt stuff, and just render nodes normally via node_view_multiple() with the search result view mode.
Comment #53
drupalninja99 CreditAttribution: drupalninja99 commentedI might not have done it right but the patch in #21 worked for everything but the search result tpl for 7.14
Comment #54
drupalninja99 CreditAttribution: drupalninja99 commentedI am getting notice errors when I tried the patch from #21
That property doesn't seem to exist
Comment #55
David_Rothstein CreditAttribution: David_Rothstein commentedIf I understand this issue correctly, my overall take is that we should not guess what people want, but rather provide an explicit setting. So something like this perhaps would make sense to me for a D7 backport:
Would that work? (Other modules which allow entities to be searched could add a similar setting too, of course.)
The reason I don't think we should guess is that I'm not totally convinced this is a bug (although I agree it's close enough to one that it can probably be backported to D7). As I understand it, it's not that the 'search_result' view mode isn't being used currently; it still runs and therefore ultimately controls what content can appear in the snippet, right? So for example, let's say I'm using the 'search_index' view mode to only index the node body; I might also want to set up the 'search_result' view mode to only display the node body (and therefore avoid having the snippet contain random, other unimportant fields that happen to match the search keywords also).
Whether or not I bothered to set up the 'search_result' view mode is therefore an independent question from whether or not I want the fancy search snippet formatting in the end.
So I'd be much happier making a small change to the admin UI to add that checkbox than trying to guess and breaking people's search result pages in unexpected ways.
Rethink it yes, but remove it completely? What if you do want your search results to look like Google's? I think that's a pretty common need.
Comment #56
DamienMcKenna@David_Rothstein: the Search Results display settings are not used to control the results display, i.e. field changes make no difference, thus it's a bug.
Comment #57
David_Rothstein CreditAttribution: David_Rothstein commentedSure they do - see my example above. In particular, you can try the following:
1. Create two text fields on a node, and add the word "elephants" as part of the content of each.
2. Search for "elephants". Both snippets will appear.
3. Now edit the Search Results display settings so that the second text field is hidden.
4. Search for "elephants" again. This time only the first snippet will appear.
Same thing goes for lots of other settings (e.g., field labels); you can use the display settings to control whether they are able to appear in the search snippet and what order they appear in, etc. It's true that some settings won't have an effect, of course (since the snippet code strips out HTML).
This behavior has been in core since Drupal 6, I believe (although in Drupal 6 you needed CCK to take advantage of it). I understand the motivation for wanting it to behave differently, but there are certainly use cases for the current behavior which is why I think it needs to be a setting.
Comment #58
yannickooAfter applying patch from #21 I don't see the entity node rendered with search result view mode. Did I forget something?
Comment #59
Stephane Bouillet CreditAttribution: Stephane Bouillet commentedPatch #21 still works for me in D7.15.
Be sure to change the
search-result.tpl.php
, and not thesearch-resultS.tpl.php
Comment #60
swentel CreditAttribution: swentel commentedDont't change the version.
Comment #61
yannickooI installed a fresh version of drupal 7 and it works OOTB but it doesn't work with Display Suite. swentel mentioned in #30 that he added fields but I cannot see them. This is my configuration of the view mode "Search result" with a Display Suite layout enabled.
Drupal uses the manage display settings from the search result view mode without mind the Display Suite settings.
So I would guess that this is a Display Suite issue only. Wo do I need the patch from #21 at the moment when it works OOTB?
Comment #62
yannickooDunno why the version switched to 7.15 in the previous comment.
Comment #63
yannickooOh cool, the patch from #21 it works and I also created a patch in #1744452: Remove search title and info from the search result which removes the title and the info stuff in a preprocess function so that you can control it completely via Display Suite, yeah. Can we set this issue to RTBC?
Comment #64
jenlampton@yannickoo - No, we can't set this issue to RTBC, see the comments in #55
@David_Rothstein - I understand that the search snippet has been around in Drupal for a long time, but with the addition of field UI to core, and the new 'search result' view mode - the old snippet-like functionality makes the new field UI + view mode appear broken.
People expect that adjusting fields for the search result view mode will affect the display in exactly the same way as adjusting the fields for the teaser view mode. There's nothing in the UI that indicates that this is not the case, and that is super frustrating. Even as someone who's worked with search snippets in previous versions of Drupal - I did not expect the old behavior to still be present after we have these new tools.
So the problem is: if we just 'fix' the UI so that it works the way everyone expects it to work - there may be some who were happy with the old 'Search snippet' style display who would suddenly be without it.
I like the idea of checking a checkbox to to use a snippet-style display instead, and setting that checkbox to checked by default in D7 (in D8, the checkbox should certainly *not* be checked by default). I think the checkbox should live on the field UI for the search result view mode - and should have a very good explanation of exactly what it does (strips excess HTML tags, and makes search results appear similar to search results from Google). If there are things in the Field UI that become 'broken' when this box is checked, those settings should instead become disabled in the field UI - to prevent confusion and frustration from people who are changing things in the UI and not seeing those changes reflected in the search result.
Comment #65
swentel CreditAttribution: swentel commentedThis is a really easy approach: as soon as there are custom settings, do not run search_excerpt(). Thoughts ?
Comment #66
swentel CreditAttribution: swentel commentedLet's see what the bot says
Comment #67
swentel CreditAttribution: swentel commentedNote - I think simply adding more help text at 'admin/structure/types/manage/%/display/search_result' (in node_help) should be sufficient enough for D8 imo. Adding an extra checkbox on field ui seems unfriendly to me.
Comment #68
icanko CreditAttribution: icanko commentedAfter fiddling around for quite some time this is what I did to impact the search results display output:
- in addition to custom display for search results I enabled custom display for search index
- I re-index the site (for default drupal search - admin/config/search/settings reindex and then several cron runs; for apache solr - admin/config/search/apachesolr - delete index and index again)
- Clear all caches.
(Maybe some of these steps aren't needed, I tried to find a consistent pattern in the workings, but even though I was clearing caches after every action the logic in behaviour of the system eluded me.
It could be that I had both search engines active, and while I had node search as default and without any search index Drupal was falling to the non-default apache solr search index, that was full at the time of my testings)
The result of this is a snippet that follows the rules set in the custom search results display. But still there remains the problem of customising the snippet size, as it currently gets cut down at arbitrary positions (probably related to the keyword position in the text).
Comment #69
David_Rothstein CreditAttribution: David_Rothstein commentedThis is an important issue, but I don't believe it's a major bug (see http://drupal.org/node/45111 and http://drupal.org/node/1181250). In fact, as discussed above, it's really unclear that this is a bug at all... it's actually somewhere between a bug and a feature request (I guess what makes it a bug is that people are definitely confused by the current situation). But it's a pretty obscure area of the Drupal admin UI for a usability issue to be considered major, in my opinion.
If you disagree and feel this must still be marked major, please provide a very specific justification and keep in mind that every major issue prevents other features that people are working hard on from getting into Drupal core, due to the issue queue threshold policy (until/unless #1810428: [Policy, no patch] Adjust major and critical task thresholds during code thaw is addressed, at any rate). Note that I am not singling out this issue for particular attention but rather (slowly) trying to go through the issue queue and do this in many places so that other features still have a chance to get into Drupal core soon without the thresholds getting in the way. I am trying my best to be impartial, and of course, just because an issue isn't marked "major" certainly doesn't mean it's unimportant to work on. You can follow my progress via the tag I've just added to this issue :)
Comment #70
David_Rothstein CreditAttribution: David_Rothstein commentedSorry for the noise; I'm told that slashes in tags can break the autocomplete on drupal.org.
Comment #71
Kazanir CreditAttribution: Kazanir commentedThe issue of whether this is a "bug" or not is kind of semantic. The system might be working as "intended" but the design is broken in the context of Drupal 7 and how view modes are supposed to work. There shouldn't be a hidden function that makes a view mode not work -- especially without any possibility of modifying it, and if a view mode is named Search Result then by George people expect it to function the way every other view mode does. With the proliferation of modules that affect view modes (both for content, like Panels and Display Suite, and for entities, like File Entity) then that method of altering "how stuff is displayed" needs to be respected.
If that means splitting the view modes and having Search Index define what is indexed (as Search Result now does) and have Search Result define the appearance of the result -- and default to the "looks like Google" display -- then that is the best solution.
This seems like a wide enough deviation from expected behavior of view modes, with no recourse by the average user, that it should be marked major.
Comment #72
David_Rothstein CreditAttribution: David_Rothstein commentedIsn't this exactly how it already behaves? Search Index and Search Result are already separate view modes that behave as you describe them above.
What this issue is about is that sometimes people don't want the "looks like Google" processing to be added to the search result display (and/or are confused by the fact that it happens without anything in the admin UI that tells you it's going to be).
Comment #73
Kazanir CreditAttribution: Kazanir commentedRight I guess I'm not being clear. What needs to happen is that if we want the "looks like Google" snippet to be available, it should be available and being applied through a view mode, just like the rest of the Drupal entity display model going forward -- not wrapped into one giant function that is spitting out HTML before we even get to the preprocess_search_results hook.
Like I have mentioned I'm still quite new to the Drupal API so I'm sorry if I'm not 100% clear on how EVERYTHING works -- but that is also why it is so clear to me that this doesn't function like Drupal is "meant to" in its current incarnation.
Comment #74
David_Rothstein CreditAttribution: David_Rothstein commentedDo you mean something like @swentel's idea of a separate "snippet" field (see #28 and #30)?
I think that makes the most sense too, going forward in Drupal 8. I can't see how that would be backportable to Drupal 7 though, and it seems a lot of people want to see something done about this in Drupal 7... So we might need a separate issue to split off what's backportable to Drupal 7 from what isn't.
Comment #76
Stephane Bouillet CreditAttribution: Stephane Bouillet commentedPatch #21 still works for me in D7.21.
Be sure to change the search-result.tpl.php, and not the search-resultS.tpl.php
Comment #77
clemens.tolboomThe issue summary really needs a rewrite.
I haven't tested #65 but it seems it replaces the search result excerpt by a rendered node. That means we won't see the search words highlighted when having a custom display.
That feels like a loss. The excerpt should become an extra field on the custom display.
Comment #78
jhodgdonUmmm...
What's really happening here is that the display type called "Search Results" is just misleading... The node module renders the node with the "Search Results" display type, and then asks other modules to modify it via hook_node_search_result, and then after that, it actually displays a search excerpt derived from this.
To alter how the search results are actually displayed, you would need to alter the search-result.html.twig template.
So really all we should probably do here is to change the name of the display type so it better conveys what it is doing. I am not sure where that is set up, or what it should be changed to, but the above patch is not what we really want to do.
Comment #79
jhodgdonOK, I've found the definition.
It's in core/modules/search/config/entity.view_mode.node_search_result.yml
There's also one there for indexing.
They should both be moved to the Node module, and both should be renamed. Perhaps better names would be:
Search index data
Search result display data
or something like that?
Comment #80
tim.plunkettWhy would they be moved to the node module? Then it would be enabled even if search.module is disabled.
It can certainly be renamed, but it should not be moved.
Comment #81
jhodgdonWell... They should actually only be enabled if the node_search plugin is enabled on the search settings page. I doubt that is possible... And really, they should be available if you are using a different search module like Solr too... And also we'd like to not have node-specific stuff in the search module.
Thoughts?
Comment #82
tim.plunkettYou want to provide a view mode for node, that's search's job. Not node's to do it for you.
You can implement hook_entity_view_mode_info_alter() and unset() it based on that setting....
Comment #83
jhodgdonThe node module provides its own search plugin, not the search module, and it is only the node module's own plugin that uses these display modes. So I kind of disagree that it's the search module's job to provide the display modes.
I had suggested during the search module plugin conversion that we should split off the node_search stuff into its own module. That might be the best solution for this issue as well... but if not, I think providing them in node.module and altering them in node_module if the search plugin is not enabled or the search module is not enabled would be best.
Really the philosophy is that Search is the framework, and modules provide plugins that do what they want them to do, and we don't want node-specific stuff in the Search framework if it doesn't need to be there.
Comment #84
tim.plunkettWell the node module could say the same about search :) NIMBY goes both ways.
I think a node_search module is an interesting idea, but I don't know how that will work for a site builder.
Comment #85
jhodgdonThe way it would work for a site builder is that if you want the Core Node Search, you will enable both Search and ... I suppose it would be called "Content Search" in the UI. Same with if you want User search, you would enable the User Search module. Then on the Search settings page you would enable those two plugins as well.
I don't think it's that big of a deal... we're used to having to turn modules on if we want functionality turned on?
Anyway, in the absence of that idea, ... let's leave this issue as "fix the names" and discuss which module things should be on elsewhere.
Comment #86
Valeratal CreditAttribution: Valeratal commentedI used # 38
But I get full node without changes in display/search_index
Comment #87
icanko CreditAttribution: icanko commentedI think the #85 suggested title sets emphasis on renaming the display mode instead of making it work.
Comment #88
dp85 CreditAttribution: dp85 commentedIs there now a solution? The body field is displayed according to the display configuration for search results. But custom fields like image fields still don't appear in the search results list.
I have tried out to manage displays with "Display Suite" module: no success, same fields missing in search results list.
Comment #89
jhodgdonI don't even think we all agree on what the problem is, actually.
But for the moment, if you want to change how search results are displayed, you need to modify search-result.tpl.php in D7 or search-result.html.twig in D8. Changing the Manage Displays settings will not do it.
Comment #90
jhodgdonI've added an issue summary to this issue, which explains the problem and 1 definite solution that needs to be done.
If anyone has ideas on how to accomplish the second part of the solution, please comment here. So far I have not seen anything constructive in that regard.
Comment #91
jhodgdonThought of some options for how to do this and added them to the Proposed Resolution. It's too late to make these changes for Drupal 7...
Comment #92
jhodgdonI just investigated one of the options for managing search results display and rejected it as not feasible without a lot of code changes in the Field UI module (summary updated).
Comment #93
DamienMcKennaWhy not just add all of the "pseudo-fields" as global fields for the entire entity type, but hidden by default? Could we build a list of all of the elements used in the traditional results template and work out what each one is and how it might be possible to map onto the current (node) entity structure? For example, I've seen lots of sites that display the full URL as a "permalink" line, etc.
Comment #94
jhodgdonRE #93, I think that would be confusing. People might try to add some of them to other display types, when they really are not available. Plus, we'd have to make the node module define all of them when rendering nodes in general.
Comment #95
DamienMcKennaOff-hand, the only pseudo field that's overly unique is the one to provide the extract with highlighted words. For that I was trying to think of maybe a new pseudo field that built a textual representative of the text of another view mode? That would then require there be three view modes for dealing with search:
Excluding performance or development effort concerns, is it at least a logical step?
Comment #96
jhodgdonHere's a preliminary patch that does this with a new display mode and renames the existing display mode... It outputs the node title in the "extra rendered" area so that will need to be fixed.
Regarding the other fields, actually there are a lot of them. Look at the NodeSearch::execute() method and see what it is returning to the search module.
Comment #98
jhodgdonSigh. I have been trying for several hours to get this test to work. Here's a completely new patch; interdiff is not really useful.
In the test, at least when I run it, the display modes are not working right, so when I just call node_view() and drupal_render() to look at a node, even in 'default' or 'full' view mode, in the test, I am not getting the field values. So the field information is not getting saved to the search index in the test.
However, it works fine for me in the UI having followed pretty much the same steps as in the test.
Comment #100
larowlanThe fields added just don't exist in the node properties.
If you use $node->getPropertyDefinitions() they don't appear.
See the debug output in this new patch.
Tried a whole heap of different things (all still in this patch)
Comment #101
larowlanScreenshots demonstrating the issue:
During the test
After loading a node of that type
Loading back a node and running
$node->getPropertyDefinitions()
.Comment #102
larowlanFull debug output: http://privatepaste.com/62adc06d8d
Comment #103
jhodgdonOK, I'm looking into this further. I don't think the properties stuff is relevant, because these are fields not properties...
Comment #104
BerdirIt is relevant and they are, that method currently has a name that's too generic but we're working on that.
I'll have a look at this tonight... or I'll try :)
Comment #105
jhodgdonYeah, I see now that "body" is in the properties.
So I did some more debugging and digging. What's actually being called inside the view code is field_invoke_view(), which calls _field_invoke_get_field_definitions() [it isn't really calling getProperties() specifically].
And when I do _field_invoke_get_field_definitions('node', $type) right after I've just defined the fields in the test using the Field UI pages, the new fields the test defines are not there.
So I looked into that function, and when it calls _field_create_entity_from_ids(), it is not getting the fields there (it gets body but not the new fields the test adds). All that is doing is basically calling entity_create().
At this point, I'm kind of out of ideas. It seems like a bug in the entity/field/field_ui system, if you create some fields in the UI and then right afterwards, create an empty node and the node does not have the fields at all. Or if you create a node with values set for those fields, and try to view it in any display mode, and the view code does not recognize the fields.
However, this does work in an actual Drupal install. It's just not working in the tests. So maybe there is some caching going on behind the scenes that makes this stuff not work in a test?
Comment #106
jhodgdonAnd just to clarify...
I just installed a clean install of the latest Drupal 8.x with this patch applied. I followed all of the steps in the test as closely as possible:
- used minimal install profile
- turned on Search, Field UI (text was enabled)
- defined the content type in the UI [technically this is not done in the UI in the test]
- defined fields in the UI
- set up the display modes in the UI
- created a node with the defined values
- ran cron to index search [this is done slightly differently in the test]
- searched, and verified that the "index" field was in the search index, but not the display or excerpt fields, and the fields in the display part are being successfully displayed in search results.
So... the test *should* pass. But it doesn't work in the testing environment, on the exact same machine.
Comment #107
clemens.tolboomThis construct is weird. In #2141929: Comment link or form is added to print view mode. I added 'print' to the plate too.
If the display mode declares 'Show no comments field' the link should not be visible either.
Can we do better?
Comment #108
clemens.tolboomI cannot edit this issue?!?
In #1901110: Improve the UX for comment bundle pages and comment field settings a desired situation is sketched for my remark in #107
Comment #109
jhodgdonI'm not going to address #107 here. That is a bigger issue, and I've commented on that other issue.
Anyway... thanks to larowland and berdir in IRC for figuring out what caches need to be cleared as a testing artifact to get the tests to pass! I also fixed one thing in the NodeSearch class: when it checks to see if the display has been customized, it needs to check to see if the entity exists **and its status is TRUE**.
Now all the tests pass and here's a new patch with an interdiff. The interdiff goes back to the patch in #98 because the debugging stuff in the later patch is not needed (and the debugging stuff in #98 is also removed, so the test is fairly lean).
Comment #110
jhodgdonclemens.tolboom - maybe I was editing it at the same time. Are you still unable to edit?
Comment #111
jhodgdonFixed the text format on this issue so everyone can edit (not sure why it was on Full HTML). And added the issue that needs to be referenced.
Comment #112
jhodgdonI had a discussion about this in IRC today with pwolanin (the co-maintainer of the D8 Core search module, along with me). Our conclusions:
a) This bug is about a cognitive/UI disconnect between people going to Manage Display and seeing "hey, I can mange the display of search results" (due to the view mode called "Search Results" for Nodes) and then finding out "Wait, this doesn't actually change how search results are output" (because that view mode actually just governs what fields are rendered in order to build the search "snippet" with highlighted search terms).
b) There really isn't a good reason to justify having separate view modes for indexing and snippet generation. If someone searches for a keyword and finds a match in the index, you'd really want the same information to be used to generate a highlighted snippet. So we should eliminate the view mode that is currently called "Search Results" in the Manage Display UI, and just use the one called "Search Indexing" for both purposes.
c) The Search module output is meant to be generic, not specifically for Node search (it also supports User search and contrib module plugins). So, the html.twig template used to generate search results output is pretty generic -- it basically just outputs a linked item title, a "snippet" if provided by the plugin's search results output, and an "info" section if provided (meta-data, which for a node is things like the node type, authoring info, and updated date). It wouldn't be easy to add fields to it or turn it into a proper View Mode view of an Entity -- in particular, for a general Search plugin, you cannot assume that each search result item must correspond to an entity.
d) Therefore... We think we should:
1. Keep the html.twig template simple as it is currently.
2. Get rid of the Search Results view mode and use Index for building the snippet for NodeSearch.
3. Let people use Display Suite or theme preprocessing functions in a custom module if they want to make the "snippet" portion of the search item output have fields or other magic.
4. Expose the node search snippet to Views as a "field" (if the search keyword filter is being used), so they can also use Views to make completely custom output.
So we need a patch that gets rid of the Search Results view mode, and that exposes the snippet to Views.
I've also updated the issue summary and assigned this issue to myself.
Comment #113
rooby CreditAttribution: rooby commentedThat sounds pretty good to me.
Like you say, contrib modules can provide fancier solutions, we just need to make the default UI less confusing.
Comment #114
clemens.tolboom+1 for #112
b)
I prefer the term 'Search Result' over 'Search Indexing'. The latter suggests how it is indexed not presented (as a result).
Comment #115
jhodgdonRE #114 - the problem with the view mode being called "Search Result" is that it leads to misunderstanding in the UI, which is the entire reason this bug was filed in the first place. We can't call it that.
Also, just to be clear, the view mode currently called "indexing" or "index" or whatever it is, *is* being used to determine what content from the node is indexed for searching. The proposal here is also to use that same view mode to make the snippet, which (since the snippet is for highlighting found search terms) seems very appropriate so that the search terms that matched the index are highlighted in the snippet (otherwise the content that had the found keywords might not be there).
Comment #116
jhodgdonHere's a patch for the first part: removing the Search Result view mode and using search_index instead.
The second part (Views integration) is still TBD but let's see if this much passes the tests.
Comment #117
David_Rothstein CreditAttribution: David_Rothstein commentedI've actually found the difference to be useful sometimes. The reason is that a snippet doesn't look good when multiple fields are smushed together in it (particularly if they aren't longtext fields). But you still might want those other fields to be searchable.
For example, consider a node with two fields:
Document ID: 5642432533
Body: Chickens are wonderful animals. etc blah blah blah
You want the document ID in the search index so that if someone pastes it in, the document pops up. But you don't necessarily want it highlighted in the snippet. Even worse, if someone searches for "chickens" you really don't want the snippet to look like this:
Excluding the document ID from the search results is how you avoid that.
----
That said, your earlier idea of renaming the "Search Result" view mode would probably work... Could it just be called "Search Snippet"?
I still really like @swentel's idea of a separate "snippet" field also (see #28 and #30) as it neatly turns the problem on its head: puts the snippet into fields rather than trying to stuff fields into the snippet. But perhaps that is more of a followup feature request.
Comment #118
jhodgdonThat is a good point. OK, proposed resolution is now updated to just be to rename the Search Results view mode to "Search Snippet" or something similar, so that it is less confusing.
I think we should still also consider exposing the snippet to Views as a field, but that can be a follow-up feature-request issue.
Making a view mode that actually lets you customize search results display... as discussed above, yes it would be great but there are too many technical issues blocking it to get it done soon (like the fact that view modes don't support most of the things we display on search results at this time), so I think that should also be a follow-up feature request issue.
Comment #119
jhodgdonWhoops. Got rid of the H3 for proposed resolution in the summary.
Comment #120
jhodgdonone more try on the summary
Comment #121
jhodgdonAnd, here's a proposed patch, which only renames the Search Result view mode to "Search snippet input". Let the bikeshedding begin!
Comment #122
jhodgdonI just filed follow-up issues:
#2212321: Add highlighted search result snippet as "field" for Views
#2212323: Make a way to better control search result display
Comment #123
pwolanin CreditAttribution: pwolanin commentedI don't understand the idea of making the snippet a real field.
This proposed change seems ok, though I'm not sure its meaning will be understood.
Comment #124
jhodgdonSnippet will never be a "real field". The proposal in #2212321 is to expose it to Views, so that if you are using the Search Keywords filter in a View, you can display a highlighted snippet for each search result.
I'm also not sure that the meaning of "Search snippet input" in the view modes will be understood, but I couldn't come up with a better name. Any ideas? Unfortunately, view modes do not have descriptions, and as far as I know, there isn't a good way to provide any help on what they are for.
Comment #125
jhodgdonHere's a new proposed name "Search result highlighting input", which is a bit long but I think will be clearer.
Wow, 125 comments and a 1-line patch...
Comment #126
pwolanin CreditAttribution: pwolanin commentedok, this is better than what we have now
Comment #128
jhodgdonDoh! The file apparently moved to core/modules/node instead of core/modules/search, for some reason.
Reroll accordingly, still a one-line patch, just a different file now.
Comment #129
jhodgdonAlso unassigning myself because it confuses "jhodgdon worked on this patch" with "jhodgdon will commit this patch".
Comment #131
jhodgdon128: 1166114-rename-view-mode-128.patch queued for re-testing.
Comment #132
jhodgdonUnrelated test failure. Hopefully will go green this time.
Comment #134
Rajendar Reddy CreditAttribution: Rajendar Reddy commentedUpdating patch with reroll. Please review.
Comment #135
Rajendar Reddy CreditAttribution: Rajendar Reddy commentedUpdating with 'Search result highlighting input' label.
Comment #136
jhodgdonThanks for the reroll!
Comment #138
catchThis better documents what the view mode does, so it feels a step better than now, even if it's still confusing overall. Went ahead and committed/pushed to 8.x, thanks!
Comment #139
DamienMcKennaIs there anything in this that could be reclaimed for D7?
Comment #140
rooby CreditAttribution: rooby commentedIMO that text change wouldn't be so bad as a backport, it's not changing any functionality.
Although I agree with #138 that the label is still confusing, although now not misleading.
Comment #141
catchWorth looking at backport at least.
Comment #142
jhodgdonYes, we can backport the patch, which just changes the display mode name.
Comment #143
SidneyGijzen CreditAttribution: SidneyGijzen commentedI'm gonna take a look at this.
Comment #144
SidneyGijzen CreditAttribution: SidneyGijzen commentedI did a
grep -r "Search result"
on the modules folder and found two occurences of the label Search result. One in node.module and one in system.api.php. Both are changed in the attached patch. I checked the change in the UI (see attached screenshot).Comment #145
jhodgdonThanks! That looks like the right patch for D7. (Just as a note, the text in search.api.php is an example function body for implementing hook_entity_info(), which is why it is in there twice in D7.)
Comment #146
SidneyGijzen CreditAttribution: SidneyGijzen commentedYou're welcome.
Aha, good to know! Thanks.
Comment #148
jhodgdonUnrelated test failures in Field API and File API. ??!?
Comment #149
jhodgdon144: rename-search-result-label-1166114-144.patch queued for re-testing.
Comment #151
jhodgdon144: rename-search-result-label-1166114-144.patch queued for re-testing.
Comment #152
jhodgdonSome kind of test glitch (failure in JavaScript test, totally unrelated)
Comment #154
jhodgdon144: rename-search-result-label-1166114-144.patch queued for re-testing.
Comment #155
jhodgdonTest glitch (couldn't install for one of the tests, "variable table already exists").
Back to RTBC.
Comment #158
jhodgdonComment #161
jhodgdonComment #164
jhodgdonWhen you requeue, can you also set it back to RTBC?
Comment #165
SidneyGijzen CreditAttribution: SidneyGijzen commentedAh, yes. Will do next time :)
Comment #168
dcam CreditAttribution: dcam commentedComment #171
dcam CreditAttribution: dcam commentedComment #174
dcam CreditAttribution: dcam commentedComment #177
dcam CreditAttribution: dcam commentedComment #180
SidneyGijzen CreditAttribution: SidneyGijzen commentedComment #181
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! See you in the followups :)
Comment #184
David_Rothstein CreditAttribution: David_Rothstein commentedDrupal 7.32 was a security release, so this is now scheduled to be in 7.33.
Comment #185
Michael_Lessard_micles.biz CreditAttribution: Michael_Lessard_micles.biz commentedGreetings,
I am using Drupal 7.38, 10 months later than the last message in this issue, and my Advanced Search results are a jarbled mess.
The results snippet shows some data in one single paragraph. On some rare ocasions it shows the fields requested by the Content type > Manage Display... most often not for no apparent reason.
Otherwise said, maybe this issue should not be marked Fixed, unless I misunderstood the subject here.
Comment #186
jhodgdonI think you have misunderstood the issue, sorry. Plus, this issue is closed. Please open another issue if you are certain there is a bug in Drupal Core that you are seeing.
But I think it is more likely that you need some support. Note that although you can create issues in Drupal Core and mark the category as "support request", we don't really handle support requests in the Drupal Core issue queue as a regular practice (that option is mostly there for filing support issues for contributed modules and themes).
There are several support options listed if you click on "Support" at the top of Drupal.org, which will take you to:
http://drupal.org/support
There you can find out about the Drupal IRC channels, and the Forums, which are our two main support mechanisms in the Drupal community. You might also try http://drupal.stackexchange.com/
Good luck with your issue!
Comment #187
Michael_Lessard_micles.biz CreditAttribution: Michael_Lessard_micles.biz commentedI disagree. Allow me to quote the original issue problem /motivation :
"Users have an expectation that when they go to Manage Displays for a content type, if they customize the 'Search Results' view mode, it would behave similarly to what would happen on other Manage Displays view modes: i.e., it would customize how search results items are displayed. But it doesn't."
In my Drupal 7.38, this problem/issue is still quite active. As this is not an individual situation, but a Drupal core issue, I do need nor request support.
Comment #188
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commented@Michael_Lessard_micles.biz, renaming the view mode was intended to address that confusion. See also #122 for the followups that were created, in particular #2212323: Make a way to better control search result display.
As for this:
It should be highlighting search words in the snippet and showing their surrounding context. If the search word appears in a particular field (and not too many other places) then text from that field is more likely to appear in the snippet. I forget how many occurrences the snippet shows, but presumably it's only the first few.
Comment #189
jhodgdonAlso, please read the "Proposed resolution" that was actually done in this issue.
Comment #190
jhodgdonAnd if you want more control over how the search results are displayed, you might check out the Display Suite module. Good luck!
Comment #191
Michael_Lessard_micles.biz CreditAttribution: Michael_Lessard_micles.biz commentedOK.
Yes, the searched words are highlighted (bold) in their context.
nb: I will check out the Display Suite module.
Thank you