Problem/Motivation
The search keyword is not always visible in the excerpt provided by Search API. Multiple fields are configured to be included in the highlighted text. Using Solr 6 as search backend, using the provided 6.x solr configuration.
I've debugged the code. In the Solr result the highlighted data is available and contains the search keyword. It looks like the different fields in this data are separated by ellipsis. This data is processed by the Highlight seach API plugin. In Hightlight::createExcerpt() the position of the keyword is determined wrong (mb_stripos() returns the wrong text position).
Perhaps the miscalculation is related to encoding. When the ellipsis separators were removed from the Solr highlight text, the correct position was calculated. A different solution was to replace mb_stripos() by stripos(), which also resulted in a correct position of the keyword.
Proposed resolution
To be determined
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2979316-7--change_to_mb_string_functions.patch | 2.85 KB | drunken monkey |
Comments
Comment #2
sutharsan commentedThis patch removes the ellipsis from the Solr hightlight text.
I do realise this may not be a final solution, but it works in our case where the excerpt text has (almost) no multibyte characters (except for the ellipsis).
Comment #4
drunken monkeyAre you using the latest versions of both Search API (1.8) and Search API Solr (2.0)?
I think, if highlighting data was already set by Solr, the Highlight processor shouldn't try to highlight that again, but just work with the existing data. It definitely works for most other people, in any case.
Comment #5
drunken monkeyComment #6
sutharsan commentedI was using, search_api 8.x-1.7+12-dev and search_api_solr 8.x-2.0-alpha3+39-dev. But also after upgrading to 1.8 and 2.0, the behavior persists.
The string that Solr returns as highlight for a single search result item is 44K large. Not sure if it is relevant, but it seems to contain text from each (drupal) field that is indexed as full text. In this string, the content of the fields are separated by ellipsis. Even when the field does not contain the search word, the string contains the full indexed text of each field. This seems weird to me...
[edited] The string referred above, is the
$textargument passed toHighlight::createExcerpt()Comment #7
drunken monkeyAh, yes, my mistake. It seems Solr only does field value highlighting, not excerpt creation for us. For creating the excerpt, we still manually load the field data from the entities/items and try to highlight that. And since we can't take things like stemming into account there, our highlighting will necessarily be faulty in some cases – and you won't get any excerpt.
Or does
$textin your description above actually contain the highlighted keyword sometimes?Anyways, it seems to me the multi-byte troubles are a much hotter candidate for what's going wrong here. It really doesn't bode well that we're only using a single multi-byte string function in the whole class – that is almost guaranteed to fail.
However, at a quick try, I wasn't able to produce a test that fails on this. (As you mentioned the ellipses being a problem, I just tried with multiple field values, so there would be ellipses in between – but no dice. See the attached "debug" patch.) Can you maybe try to provide one?
Unfortunately, it seems we currently just have a single test for multi-byte handling. (The one that failed for your simpler solution of just switching that one multi-byte function to use the normal kind after all.)
However, the opposite approach, changing all the string functions in the class (or, really, just that method) to their multi-byte counterparts fared much better. I had to get crafty to be able to still work with the one PCRE function (which only ever returns the byte position, as far as I can see), but otherwise this went perfectly fine, and all tests are still passing.
Can you give that a try and see if it works to fix your problem, too?
If that's the case, we'd just need that regression test, then I could commit this.
Comment #10
drunken monkeyComment #11
drunken monkeyI invested quite some time in this, could you please test/review?
Comment #12
sutharsan commentedSorry, I missed this. I will give it a spin now.
Comment #13
sutharsan commentedYour patch works as, and provides almost the same results as mine. I have compared the visible excerpt search result of both pathes and a third case without patch. I also share a dump of the
$textvariable as a file. The search keyword used in this test is 'braken'.Without patch
With patch #2
With patch #7
Comment #14
drunken monkeyAh, excellent! Thanks for reporting back!
So this is RTBC in your opinion?
Comment #15
sutharsan commentedFunctionally this is RTBC. I leave the quality of code to you to decide. I’m currently not in a situation to do a descent review.
Comment #16
drunken monkeyOK, great, thanks for reporting back!
Committed.