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

  1. User wants to customize the search results page for their node "Test #3":
  2. 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:
  3. 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?

Files: 
CommentFileSizeAuthor
#144 1166114-screen-UI-change.png77.24 KBMF82
#144 rename-search-result-label-1166114-144.patch902 bytesMF82
PASSED: [[SimpleTest]]: [MySQL] 40,911 pass(es).
[ View ]
#135 drupal8-rename_view_mode-1166114-135.patch508 bytesRajendar Reddy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,359 pass(es).
[ View ]
#134 1166114-rename-view-mode-134.patch496 bytesRajendar Reddy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,978 pass(es).
[ View ]
#128 1166114-rename-view-mode-128.patch476 bytesjhodgdon
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1166114-rename-view-mode-128.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

pwolanin’s picture

Looking at:
http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...

we have :

$build = node_view($node, 'search_result');

So it would seem like node module at least is doing the right thing, so perhaps this is a bug in Field API?

pwolanin’s picture

making the screenshots into attachments...

Agogo’s picture

This bug is in Drupal 7 as well.

Subscribing

Agogo’s picture

This 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.

jhodgdon’s picture

Status:Active» Postponed (maintainer needs more info)

Has 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?

tagawa’s picture

I'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.

jhodgdon’s picture

Status:Postponed (maintainer needs more info)» Active

OK, looks like this needs more investigation, and it's probably a real bug.

jhodgdon’s picture

Priority:Normal» Major
jhodgdon’s picture

Issue tags:+needs backport to D7
tomogden’s picture

After 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 Only local images are allowed.) 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.

tomogden’s picture

StatusFileSize
new36.23 KB

See attached screenshot of my test. The three fields needing to display are:

  • Image:
  • Location (city, country/state):
  • Date:

But as you see they are all run together into a single string.

tomogden’s picture

Status:Active» Needs review
StatusFileSize
new2.24 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _1166114_search_74.patch.
[ View ]

See 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.

Status:Needs review» Needs work

The last submitted patch, _1166114_search_74.patch, failed testing.

tomogden’s picture

StatusFileSize
new2.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _1166114_search_74_0.patch.
[ View ]

File paths set to website root.

tomogden’s picture

Status:Needs work» Needs review
StatusFileSize
new2.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _1166114_search_74_1.patch.
[ View ]

Set 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?

The last submitted patch, _1166114_search_74.patch, failed testing.

tomogden’s picture

StatusFileSize
new2.43 KB
FAILED: [[SimpleTest]]: [MySQL] 33,543 pass(es), 0 fail(s), and 875 exception(es).
[ View ]

Paths reset once more.

Status:Needs review» Needs work

The last submitted patch, _1166114_search_v4.patch, failed testing.

tomogden’s picture

Status:Needs work» Needs review
StatusFileSize
new2.48 KB
FAILED: [[SimpleTest]]: [MySQL] 33,588 pass(es), 0 fail(s), and 44 exception(es).
[ View ]

Corrected missing index.

Status:Needs review» Needs work

The last submitted patch, _1166114_search_5.patch, failed testing.

tomogden’s picture

Status:Needs work» Needs review
StatusFileSize
new2.58 KB
PASSED: [[SimpleTest]]: [MySQL] 33,591 pass(es).
[ View ]

Corrected potential empty reference.

xjm’s picture

Tagging issues not yet using summary template.

jhodgdon’s picture

Not every issue necessarily needs a summary, actually...

xjm’s picture

@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. :)

xjm’s picture

Er, and in this case most contributors can't actually write a summary because of the input format, so untagging.

catch’s picture

Status:Needs review» Needs work

I 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.

douggreen’s picture

@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?

swentel’s picture

I 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 :)]

yoroy’s picture

Issue tags:+Usability

#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?

swentel’s picture

StatusFileSize
new28.65 KB

@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)

yoroy’s picture

Thanks 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 :)

swentel’s picture

Well, 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).

yoroy’s picture

jkaine’s picture

subscribe

BarisW’s picture

subscribe. #1286100: Display: Search Result not working is a duplicate of this as well.

rogical’s picture

+1 this feature is very important

kingswoodute’s picture

Subscribing - if anyone comes up with a fix for D7 that would be great.

jkaine’s picture

Kingswoodute,

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:

<?php
     
print ('<li class="');
    print
$classes;
    print (
'"');
    print
$attributes;
    print (
'>');
    print
render($title_prefix);
    print (
'<h3 class="title"');
    print
$title_attributes;
    print (
'><a href="');
    print
$url;
    print (
'">');
    print
$title;
    print (
'</a></h3>');
    print
render($title_suffix);
    print (
$variables['result']['node']->rendered);
    print (
'</li>');
 
?>

Your file should end at line 83 with a closing li tag. Here's the entire file, if it helps:

<?php

/**
 * @file
 * Default theme implementation for displaying a single search result.
 *
 * This template renders a single search result and is collected into
 * search-results.tpl.php. This and the parent template are
 * dependent to one another sharing the markup for definition lists.
 *
 * Available variables:
 * - $url: URL of the result.
 * - $title: Title of the result.
 * - $snippet: A small preview of the result. Does not apply to user searches.
 * - $info: String of all the meta information ready for print. Does not apply
 *   to user searches.
 * - $info_split: Contains same data as $info, split into a keyed array.
 * - $module: The machine-readable name of the module (tab) being searched, such
 *   as "node" or "user".
 * - $title_prefix (array): An array containing additional output populated by
 *   modules, intended to be displayed in front of the main title tag that
 *   appears in the template.
 * - $title_suffix (array): An array containing additional output populated by
 *   modules, intended to be displayed after the main title tag that appears in
 *   the template.
 *
 * Default keys within $info_split:
 * - $info_split['type']: Node type (or item type string supplied by module).
 * - $info_split['user']: Author of the node linked to users profile. Depends
 *   on permission.
 * - $info_split['date']: Last update of the node. Short formatted.
 * - $info_split['comment']: Number of comments output as "% comments", %
 *   being the count. Depends on comment.module.
 *
 * Other variables:
 * - $classes_array: Array of HTML class attribute values. It is flattened
 *   into a string within the variable $classes.
 * - $title_attributes_array: Array of HTML attributes for the title. It is
 *   flattened into a string within the variable $title_attributes.
 * - $content_attributes_array: Array of HTML attributes for the content. It is
 *   flattened into a string within the variable $content_attributes.
 *
 * Since $info_split is keyed, a direct print of the item is possible.
 * This array does not apply to user searches so it is recommended to check
 * for its existence before printing. The default keys of 'type', 'user' and
 * 'date' always exist for node searches. Modules may provide other data.
 * @code
 *   <?php if (isset($info_split['comment'])) : ?>

*     <span class="info-comment">
*       <?php print $info_split['comment']; ?>
*     </span>
*   <?php endif; ?>
* @endcode
*
* To check for all available data within $info_split, use the code below.
* @code
*   <?php print '<pre>'. check_plain(print_r($info_split, 1)) .'</pre>'; ?>
* @endcode
*
* @see template_preprocess()
* @see template_preprocess_search_result()
* @see template_process()
*/
?>
  <?php
     
print ('<li class="');
    print
$classes;
    print (
'"');
    print
$attributes;
    print (
'>');
    print
render($title_prefix);
    print (
'<h3 class="title"');
    print
$title_attributes;
    print (
'><a href="');
    print
$url;
    print (
'">');
    print
$title;
    print (
'</a></h3>');
    print
render($title_suffix);
    print (
$variables['result']['node']->rendered);
    print (
'</li>');
 
?>

</li>

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.

erics14’s picture

I 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?

jkaine’s picture

Eric,

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

erics14’s picture

StatusFileSize
new3.17 KB

Hi jkaine,

Thank you so much for responding. I'm running Bartik. I've attached my search-result.tpl file.

Regards,

Eric

jkaine’s picture

Eric,

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.

erics14’s picture

Jkaine,

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.

splatio’s picture

StatusFileSize
new3.39 KB

I'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 using render(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?

dhayles’s picture

I 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.

efc84’s picture

Are we any closer to a release fix?

remedact’s picture

Hello,

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

remedact’s picture

I'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 !

shenzhuxi’s picture

#21 patch works in Drupal 7.12,

DamienMcKenna’s picture

Question 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.

remedact’s picture

just tried the patch #21 in drupal 7.14: still working ! ;)

catch’s picture

Assigned:Unassigned» David_Rothstein

I'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.

drupalninja99’s picture

I might not have done it right but the patch in #21 worked for everything but the search result tpl for 7.14

drupalninja99’s picture

I am getting notice errors when I tried the patch from #21

Notice: Undefined property: stdClass::$rendered in template_preprocess_search_result() (line 135
Notice: Undefined property: stdClass::$type in template_preprocess_search_result() (line 136

That property doesn't seem to exist

David_Rothstein’s picture

Assigned:David_Rothstein» Unassigned

If 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:

  1. Node module adds a new checkbox in the UI (where exactly, I'm not sure) which is the equivalent of "Do you want to run the search_excerpt() function on nodes before displaying them on the search results page?" (Or maybe another way to say it, "Do you want your search results to look like Google's, as opposed to looking like nodes?" :)
  2. If yes (the default), the code behaves as it does now.
  3. If not, then node_search_execute() sets 'snippet' to the plain old rendered node, so that the theme layer automatically uses that instead.

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.

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.

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.

DamienMcKenna’s picture

@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.

David_Rothstein’s picture

the Search Results display settings are not used to control the results display, i.e. field changes make no difference,

Sure 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.

yannickoo’s picture

After applying patch from #21 I don't see the entity node rendered with search result view mode. Did I forget something?

remedact’s picture

Version:8.x-dev» 7.15

Patch #21 still works for me in D7.15.

Be sure to change the search-result.tpl.php, and not the search-resultS.tpl.php

swentel’s picture

Version:7.15» 8.x-dev

Dont't change the version.

yannickoo’s picture

Version:8.x-dev» 7.15
StatusFileSize
new30.06 KB
new62.48 KB

I 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.

Display Suite – Search result view mode

Drupal uses the manage display settings from the search result view mode without mind the Display Suite settings.

Search result with Display suite

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?

yannickoo’s picture

Version:7.15» 8.x-dev

Dunno why the version switched to 7.15 in the previous comment.

yannickoo’s picture

Oh 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?

jenlampton’s picture

@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.

swentel’s picture

StatusFileSize
new1.4 KB
PASSED: [[SimpleTest]]: [MySQL] 42,296 pass(es).
[ View ]

This is a really easy approach: as soon as there are custom settings, do not run search_excerpt(). Thoughts ?

swentel’s picture

Status:Needs work» Needs review

Let's see what the bot says

swentel’s picture

Note - 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.

icanko’s picture

After 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).

David_Rothstein’s picture

Priority:Major» Normal

This 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 :)

David_Rothstein’s picture

Sorry for the noise; I'm told that slashes in tags can break the autocomplete on drupal.org.

Kazanir’s picture

The 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.

David_Rothstein’s picture

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.

Isn'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).

Kazanir’s picture

Right 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.

David_Rothstein’s picture

Do 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.

remedact’s picture

Patch #21 still works for me in D7.21.

Be sure to change the search-result.tpl.php, and not the search-resultS.tpl.php

clemens.tolboom’s picture

The 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.

jhodgdon’s picture

Status:Needs review» Needs work

Ummm...

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.

jhodgdon’s picture

OK, 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?

tim.plunkett’s picture

Why 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.

jhodgdon’s picture

Well... 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?

tim.plunkett’s picture

You 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....

jhodgdon’s picture

The 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.

tim.plunkett’s picture

Well 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.

jhodgdon’s picture

Title:Manage Displays Search Results doesn't manage the display of the search results» The "Search Results" display mode has a confusing name

The 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.

Valeratal’s picture

I used # 38

But I get full node without changes in display/search_index

icanko’s picture

Title:The "Search Results" display mode has a confusing name» Manage Displays Search Results doesn't manage the display of the search results

I think the #85 suggested title sets emphasis on renaming the display mode instead of making it work.

dp85’s picture

Is 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.

jhodgdon’s picture

I 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.

jhodgdon’s picture

Issue summary:View changes

I'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.

jhodgdon’s picture

Thought 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...

jhodgdon’s picture

Issue summary:View changes

I 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).

DamienMcKenna’s picture

Why 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.

jhodgdon’s picture

RE #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.

DamienMcKenna’s picture

Off-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:

  • Search index
  • Search result
  • Search extract

Excluding performance or development effort concerns, is it at least a logical step?

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new3.88 KB
FAILED: [[SimpleTest]]: [MySQL] 59,458 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here'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.

Status:Needs review» Needs work

The last submitted patch, 96: search_display_modes_93.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new14.83 KB
FAILED: [[SimpleTest]]: [MySQL] 59,607 pass(es), 14 fail(s), and 0 exception(s).
[ View ]

Sigh. 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.

Status:Needs review» Needs work

The last submitted patch, 98: search_display_modes_98.patch, failed testing.

larowlan’s picture

StatusFileSize
new16.16 KB
new5.46 KB

The 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)

larowlan’s picture

StatusFileSize
new350.77 KB
new25.07 KB

Screenshots demonstrating the issue:

During the test

shot of manage fields mid-test

After loading a node of that type

Loading back a node and running $node->getPropertyDefinitions().
field properties

larowlan’s picture

jhodgdon’s picture

OK, I'm looking into this further. I don't think the properties stuff is relevant, because these are fields not properties...

Berdir’s picture

It 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 :)

jhodgdon’s picture

Yeah, 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?

jhodgdon’s picture

And 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.

clemens.tolboom’s picture

+++ b/core/modules/comment/comment.module
@@ -543,7 +543,7 @@ function comment_entity_view(EntityInterface $entity, EntityDisplay $display, $v
       }
-      elseif ($view_mode != 'search_index' && $view_mode != 'search_result') {
+      elseif ($view_mode != 'search_index' && $view_mode != 'search_result' && $view_mode != 'search_result_extra') {
         // Entity in other view modes: add a "post comment" link if the user is
         // allowed to post comments and if this entity is allowing new comments.

This 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?

clemens.tolboom’s picture

I 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

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new12.83 KB
PASSED: [[SimpleTest]]: [MySQL] 59,551 pass(es).
[ View ]
new5.11 KB

I'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).

jhodgdon’s picture

clemens.tolboom - maybe I was editing it at the same time. Are you still unable to edit?

jhodgdon’s picture

Fixed 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.

jhodgdon’s picture

Assigned:Unassigned» jhodgdon
Issue summary:View changes
Status:Needs review» Needs work

I 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.

rooby’s picture

That sounds pretty good to me.
Like you say, contrib modules can provide fancier solutions, we just need to make the default UI less confusing.

clemens.tolboom’s picture

+1 for #112

b)

So we should eliminate the view mode that is currently called "Search Results"

I prefer the term 'Search Result' over 'Search Indexing'. The latter suggests how it is indexed not presented (as a result).

jhodgdon’s picture

RE #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).

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new2.23 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,054 pass(es).
[ View ]

Here'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.

David_Rothstein’s picture

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.

I'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:

... 5642432533 Chickens are wonderful animals. etc blah blah blah ...

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.

jhodgdon’s picture

Issue summary:View changes
Status:Needs review» Needs work

That 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.

jhodgdon’s picture

Issue summary:View changes

Whoops. Got rid of the H3 for proposed resolution in the summary.

jhodgdon’s picture

Issue summary:View changes

one more try on the summary

jhodgdon’s picture

Status:Needs work» Needs review
StatusFileSize
new472 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 65,105 pass(es).
[ View ]

And, here's a proposed patch, which only renames the Search Result view mode to "Search snippet input". Let the bikeshedding begin!

pwolanin’s picture

I 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.

jhodgdon’s picture

Snippet 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.

jhodgdon’s picture

StatusFileSize
new484 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1166114-rename-view-mode-125.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here'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...

pwolanin’s picture

Status:Needs review» Reviewed & tested by the community

ok, this is better than what we have now

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 125: 1166114-rename-view-mode-125.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new476 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 1166114-rename-view-mode-128.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Doh! 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.

jhodgdon’s picture

Assigned:jhodgdon» Unassigned

Also unassigning myself because it confuses "jhodgdon worked on this patch" with "jhodgdon will commit this patch".

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 128: 1166114-rename-view-mode-128.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Needs review
jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Unrelated test failure. Hopefully will go green this time.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 128: 1166114-rename-view-mode-128.patch, failed testing.

Rajendar Reddy’s picture

Status:Needs work» Needs review
StatusFileSize
new496 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,978 pass(es).
[ View ]

Updating patch with reroll. Please review.

Rajendar Reddy’s picture

StatusFileSize
new508 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 68,359 pass(es).
[ View ]

Updating with 'Search result highlighting input' label.

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Thanks for the reroll!

  • Commit 7625cea on 8.x by catch:
    Issue #1166114 by jhodgdon, tomogden, Rajendar Reddy, larowlan, swentel...
catch’s picture

Status:Reviewed & tested by the community» Fixed

This 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!

DamienMcKenna’s picture

Is there anything in this that could be reclaimed for D7?

rooby’s picture

IMO 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.

catch’s picture

Version:8.x-dev» 7.x-dev
Status:Fixed» Patch (to be ported)

Worth looking at backport at least.

jhodgdon’s picture

Yes, we can backport the patch, which just changes the display mode name.

MF82’s picture

Assigned:Unassigned» MF82

I'm gonna take a look at this.

MF82’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new902 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,911 pass(es).
[ View ]
new77.24 KB

I 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).

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Thanks! 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.)

MF82’s picture

You're welcome.

Aha, good to know! Thanks.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Unrelated test failures in Field API and File API. ??!?

jhodgdon’s picture

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Some kind of test glitch (failure in JavaScript test, totally unrelated)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Test glitch (couldn't install for one of the tests, "variable table already exists").

Back to RTBC.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

When you requeue, can you also set it back to RTBC?

MF82’s picture

Ah, yes. Will do next time :)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

dcam’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

dcam’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

dcam’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

dcam’s picture

Status:Needs work» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 144: rename-search-result-label-1166114-144.patch, failed testing.

MF82’s picture

Status:Needs work» Reviewed & tested by the community
David_Rothstein’s picture

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.32 release notes

Committed to 7.x - thanks! See you in the followups :)

  • David_Rothstein committed 8a6142a on 7.x
    Issue #1166114 by jhodgdon, tomogden, Rajendar Reddy, larowlan, swentel...

Status:Fixed» Closed (fixed)

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

David_Rothstein’s picture

Drupal 7.32 was a security release, so this is now scheduled to be in 7.33.