Select Text Format in Views Field for textual content with values from search index

I think you forgot to actually upload the patch.

Thanks for the suggestion!
I don't think two handlers are necessary here, though. The distinction between string and (full)text doesn't apply here, since we're talking about properties, not Search API fields. Properties have a different set of types, where there isn't such a distinction.

Also, I don't think a select box with such long option names is a good UI for this. I changed that to radios, looks more sensible to me. (I'm open for other suggestions, too, though. In general, though, I think select boxes should be avoided, especially if there are only a few options.)

Finally, I don't think overwriting the definition and calling the parent in render_item() makes sense – we can just copy the one line of code we need instead.

Apart from these minor problems, the patch looks pretty good though, thanks again! However, I wonder why it was even necessary for you? It seems we already take care to set the filter type to "xss" for all fields that might contain HTML. (Although I guess "xss_admin" would be a better match in many cases …) Why isn't that the case for your property/field, how do you add that?

OK, that makes sense. And yes, I agree, this option could still make sense. However, I think I'm gonna wait for a bit more feedback before committing.

The attached should fix the tests. Of course: if we add a new plugin, we also need to define its schema. Should finally remember that, after running into that this often …

Should also use this for the "Search: Excerpt" field, of course.

It doesn't seem to work for me but I think I do something wrong.
I 've applied the patch and I see this function in Drupal\search_api\Plugin\views\field\SearchApiText.php

public function buildOptionsForm(&$form, FormStateInterface $form_state) {
    parent::buildOptionsForm($form, $form_state);

    $args = [
      '@strip' => $this->t('Strip HTML tags'),
      '@rewrite' => $this->t('Rewrite results'),
    ];
    $form['filter_type'] = [
      '#title' => $this->t('Enable HTML in this field'),
      '#type' => 'radios',
      '#options' => [
        'xss' => $this->t('Field value can contain HTML'),
        'xss_admin' => $this->t('Field value can contain HTML (even potentially unsafe tags)'),
        'plain' => $this->t('Do not allow HTML in this field'),
      ],
      '#default_value' => $this->options['filter_type'],
      'xss' => [
        '#description' => $this->t('This will allow the most common HTML tags, except unsafe ones.'),
      ],
      'xss_admin' => [
        '#description' => $this->t('This will allow almost all tags (except scripts and styles). Use with caution.'),
      ],
      'plain' => [
        '#description' => $this->t('This will display any HTML tags in the field value as plain text. For instead removing those tags, use the "@strip" option under "@rewrite".', $args),
      ],
    ];
  }

But I can't see a filter Type in the view.
3 files have been patched by "2874641-12--views_field_handler_escape_method_option.patch"
- config/schema/search_api.views.schema.yml
- search_api.views.inc
- src/Plugin/views/field/SearchApiText.php

What do I do and where do I expect that?
Starting point is the "Database Search Defaults" View.
There I change the Format to show Fields.
Then I add the fields
- Content datasource: Title (indexed field)
- Search: Excerpt
I don't have an option to alter the filter type whether in title or in excerpt.

Where do I find the option to change the filter type?

cheers,
Christian

When you click on "Search: Excerpt" in the admin UI, the option should be hard to miss (see screenshot). If it doesn't appear, clear the cache.

This needs tests, because it adds new functionality.

Hm, I guess you're right. Stupid test coverage …
Anyways, since there doesn't seem to be much interest anyways, and no-one commenting who tested this successfully, for now the question is moot anyways. Might as well hold off on the test until I'd actually commit this.
Still, good point. (Setting back to "Needs review" so people will know to try it out. The "Needs tests" tag should be enough for me to remember to still not commit it after it's RTBC'd.)

@drunkenmonkey I've tested the patch in #11 with success with search_api 1.x-dev#abb17dd06024288c87c4fefc611da818cccdb5df
Now, I can't get it working again with either 1.x or 1.x-dev. Go figure.

How exactly is it not working? (I assume you've tried clearing the caches?)

@drunken It's related to something regarding that sandbox module we're using at Digitalist - i.e. search_api_solr_datasource.
The way it presents itself is that when you add fields in Views some of the fields have the category "Index whatever" and some have the category "Solr Document datasource". The fields that are in the category "Index whatever" I can use the output formats. The ones in "Solr Document datasource" I can't use the output formats for. They just don't show up.

In that case, maybe the datasource reports the wrong property types for that to work correctly. But you can check in the Views data for the search_api_* tables. The field handlers for the columns in question should be search_api_text. It should also work to just override them otherwise.

I have one issue with the patch: I would like to exclude certain tags completely form the excerpt and the index. I have <script type="application/ld+json"> tags which are currently being displayed. In the "HTML Filter" processor I have this: script: 0 So they should be escluded from the index. But they seem to get rendered into the excerpt.

I tried "Strip HTML tags" option under "Rewrite results" on and off in combination with all text format settings of the view field.

Am I doing something wrong or is this a bug?

Sorry for that.

I guess my problem is rather a bug with the "HTML Filter" processor. This data should never end up in the index at all.

Debuging I saw that ends up in Highlight::createExcerpt contains the <script type="application/ld+json"> tags.

Is the thing I am missing a "HTML Filter" postprocessor?

I guess my problem is rather a bug with the "HTML Filter" processor. This data should never end up in the index at all.

Is it definitely also included in the index?
The Highlight processor doesn't use the indexed data for creating the excerpt, unless it's contained in the results data (i.e., you're using Solr and have it configured that way – but in that case, you shouldn't use the Highlight processor anyways, I'd say). Normally, it would load the item, extract the necessary properties from it and base the excerpt on that. No other processors run on that data, so the HTML Filter has nothing to do with this.
However, \Drupal\search_api\Plugin\search_api\processor\Highlight::createExcerpt() actually tries to base the excerpt on the text value stripped from any HTML, so that seems to be failing in your case for some reason.

Are you using Solr?

Oh, also, apart from all that, the "Field value can contain HTML (even potentially unsafe tags)" option will actually still run Xss::filterAdmin() on the value, so <script> and <style> will be replaced in any case. Does that resolve your problem?

No, I am using the database search.

I tried all combinations of those settings. I think the issue is that the <script> tags are escaped before that and therefore just end up being displayed as regular text in the excerpt.

Any ideas how to solve this? By now the site is live. You can see the issue in the search results for pages: https://ensoie.com/search?s=anna

No, sorry, since I still don't know what the actual problem is in your case. Most of your theories don't make any sense. You'll need to find out (or do a better job of telling me) what the actual problem is before I can help you.
I also can't see any <script> (or other) tags on the page you linked to. Just a bunch of JSON fragments – which are, of course, also undesirable, but you didn't mention.

Or did you mean that the contents of those tags get displayed on the page?
That's a completely different issue, but at least entirely plausible. Once the Highlight processor strips the <script> tags, its contents of course remain as part of the base text.
While I don't think having <script> tags as part of a rendered entity's HTML is a good idea in general, I guess we could easily just strip out the entire contents of those elements before highlighting. After all, there's hardly ever a scenario in which you'd want those elements' contents as part of the entity text.
However, just selecting a view mode for the "Rendered item" field in which the <script> tags aren't included would be a more sensible course of action, I think. (If you're not using "Rendered item" but have the <script> tags directly in the node body, then that sounds like an even worse idea.)

The script tags contain the JSON-LD needed for the microdata we want to provide. Rendering this within the entity totally makes sense since it represents the entity as microdata. An no, of course I didn't input this into a field. This happens in our Twig templates.

There are definitely cases where it is difficult to get rid of this. For example you have something like related content listing teasers of other entities that by default output something like JSON-LD. The you have to create different view modes for all kind of content. It would be very nice if we were able to exclude certain parts of the rendered HTML.

Ah, OK, I automatically assumed <script> means JavaScript, but of course that's not always the case. This seems like a more reasonable use case, yes.
However, I still don't think this should be discussed in this issue. This might be a separate feature request later, or even right now (if it makes sense), but it's leading this issue too far off-topic. Also, I'm not sure whether this wouldn't be something better added to Views itself. It's not really specific to our field handlers, or to this option, as far as I can see.

If you do create a new issue for this, please try to find out what exactly is going on, and try to explain it as well as possible.

Looks like this got set back to needs review by accident since there's no new patch since the last time it was set to Needs work and we're still missing the test coverage.

I have an use case for this in a project I'm working on. #12 works as expected for field values with HTML markup. I did some changes at the UI level: option label summarized and reordered from less risky to less restrictive. I also added the list of allowed tags in xss/xss_admin to the options description. Patch also now applies cleanly in the latest -dev

I'd tried to add test coverage, but to be honest, I felt a bit lost examining the current tests. Not sure about the right place to add test coverage for this. Could somebody who knows the Search API test architecture provide some initial guide? Thanks,

Looks good, thanks for working on this!
For the tests, you probably want to add a new method to \Drupal\Tests\search_api\Functional\ViewsTest. There are some existing views defined in tests/search_api_test_views/config/install/ for those tests – you could either try expanding/editing one to provide the necessary fields, or create a new one. Then, create test content with appropriate structure and contents, switch the field to different settings for the new option and see if the output is correct.
(Another option could be to add it to the existing testViewsAdmin() method. That way, we’d also test the admin UI for the new option, to make sure that’s defined correctly. We did have options in the past where the selection was just not saved, without us noticing for quite some time.)

I realize that writing tests for Views functionality is quite a hassle, but I do think it’s important to keep good test coverage. And Kernel tests often don’t cut it in the case of Views, as there are so many interlinked parts that could fail. So, if we just tested the three new methods in SearchApiText, there is still a lot that could go wrong, I think.

Rerolled

Thanks for sharing.

The patch in #35 works as expected, but I'm a bit reluctant to render HTML content with the xss_admin filter (should be safe, but could still allow huge images, etc, that break layouts...)

I didn't dig too deeply, but is it possible to instead simply use the format that is stored with the text field value? This presumably gets indexed, but perhaps not?

Somewhat related here, what's particularly odd about this behavior is that correct formatting is used if a highlighted search term is in play (the associated filter format), but when no highlighting occurs, the default xss filter is used, stripping tags such as <p>...

Possibly related, the issue I found yesterday was that our results would show the HTML formatting, and would show highlighted search terms, but it would lose the spaces on both sides. So "this is bold text in the middle of a sentence" would become "this isbold textin the middle of a sentence."

What I discovered is that it did that if the setting on the field in the view for "use entity field rendering" was unchecked, I got this behaviour with losing spaces. If I checked "use entity field rendering", the spaces would be maintained, but I lost the search term highlighting. For now I've opted to maintain spaces at the cost of highlighting, but it would be ideal if we could have both.

I figured out why when text fields are highlighted that this isn't an issue. It turns out that xss_admin is being used anyway for highlighted fields. In SearchApiFieldTrait::combineHighlightedValues:

    // Pre-sanitize the highlighted values with a very permissive setting to
    // make sure the highlighting HTML won't be escaped later.
    foreach ($highlighted_values as $i => $value) {
      if (!($value instanceof MarkupInterface)) {
        $highlighted_values[$i] = $this->sanitizeValue($value, 'xss_admin');
      }
    }

This would be a great enhancement of Search API and Views integration, thanks to everyone here for working on it. It works well, and here's a re-roll. I guess we're still waiting for tests?

@ressa, thanks for the re-roll. The patch works for my needs and I agree this is a great feature to have for a fast and interactive search experience.

Let's finally add this cool feature to the module. It is really helpfull. Used patches from this issue in a lot of projects :)

Also, I like descriptions to the options this is very informative.

#41 works well for me - lets add this! +1!!! :)

This patch really is essential for nearly all my systems and I am really tired to apply it every time. Can we please add it? it works great!

I guess we already have a test, so we're no longer waiting for that, we should make this into a merge request to see if the test actually passes.

This patch really is essential for nearly all my systems and I am really tired to apply it every time. Can we please add it? it works great!

@knurg, this issue is in the "needs work" status, for it to be committed, it should at least get to the "Reviewed and Tested by the Community" status.

A comment like the ones in #42, #43 and #44 could also put this into rtbc, if you feel safe enough that this will not break any other installations of the module.

So in this case the next steps are:

• Make the patch into a merge Request
• Get a code review on the code and tests
• Get this into RTBC state

I've done my best :)

Thanks @knurg!

Looks like the latest merge request broke all tests, because of a missing ".ignored-deprecations.txt", I don't think this is because of this MR though. I wonder if this is something more general that's going on. I asked in #gitlab on Slack.

Pipelines now pass again, thanks!
I just added return type hints to the new methods.
If a few people could give this a try and tell us whether it still works for them I can merge it.

*bump*

The "Allow almost any HTML" is showing html on a body field, but it isn't displaying the same on the Search: Excerpt field. Is there specific configuration I need for the Excerpt field?

@capysara: Excerpts are already created without any HTML (except for the highlighting tags, of course) by the Highlight processor, so there is nothing more to display for the view. By switching to “Do not allow HTML” you could just make the <strong> tags (or whatever you have configured) around matches visible.

If you want HTML preserved inside the excerpt you’ll need to implement your own code for creating the it and use that instead of the Highlight processor.

This works for me.

Great to hear, thanks a lot for testing!
Merged.

Thanks everyone for getting this MR over the finishing line, it feels great to have one less patch in composer.json :)