Searches for words with diacritics/accents: word not highlighted in results

You are correct. This issue needs to be fixed in Drupal 7, and then backported to Drupal 6. There are multiple problems with the search excerpt function, and this is certainly one of them...

An older issue that reported something similar, which I've just marked as a duplicate of this issue:
#157684: Search module does not highlight non-ASCII search terms

I changed my mind. That other issue said that search didn't find words with diacritics. This issue says (and it's correct, I just tested) that search will find words with diacritics but not highlight them in the (mostly broken) excerpt function.

Anyway, I've confirmed that this is a current issue in Drupal 7.

To confirm:

1) Put the following text in a node (yes, it's probably mixing languages and no, I don't know what it means really):

Det trodde jag aldrig (eller jo, kanske ändå ;-)). Min första seriösa bloggpostning. Det vill säga en skrivelse som praktiskt taget ingen bryr sig ett dyft om. Det är ett obevisat faktum att 90% av allt som skrivs i blogga.

meklēt

2) Search for meklēt, seriösa etc. and everything works fine.

3) Search for meklet, seriosa, and search will find the node but not highlight the words in the excerpt.

May be related to #493270: search_excerpt() doesn't work well with stemming as well, or it may be possible to fix that other issue at the same time.

The basic problem is that the search excerpt function only looks for exact textual matches between the text of the node (or whatever is passed into the function, generally a node) and the keywords. However, search preprocesses keywords before doing searching, so there are many cases where the keywords will not exactly match what's in the node text, and search_excerpt() therefore won't highlight the keyword even though it found the node.

Actually, there is no preprocessing being done to "seriösa" causing "seriosa" to match -- this is happening because of the collation of the search_index table in the database. (To see this, go ahead and inspect the db and you'll see that "seriösa" was indexed as is, yet select * from search_index where word = "seriosa" still returns that row. (See also: http://bugs.mysql.com/bug.php?id=37413)

I think the solution to this problem is to remove diacritics in search_simplify explicitly so that words are indexed without them, which means we'll get diacritic-insensitive matches regardless of the collation of the search_index table and whatever db is being used. Once that's done, the solution to #916086: search_excerpt() doesn't highlight words that are matched via search_simplify() will ensure that words with diacritics are highlighted.

The issue is the search excerpt function doesn't work well. This is a duplicate of #493270: search_excerpt() doesn't work well with stemming

Re-opening. See: http://drupal.org/node/493270#comment-3596818

I personally think this is a feature request, and a rather large change in how the search module works. As such, I think we can't really do it at this stage in D7 development, and it should be pushed off to Drupal 8 for consideration. Feel free to make an argument to move it back to D7 if you think I'm wrong...

I just tested this issue by making a node with body meklēt

This is still a problem in Drupal 8: searching for either meklēt or meklet finds the node, but in the latter case, the word is not highlighted on the page.

I think this is a bug rather than a feature request, and we should fix it.

Updated summary

Hi @pjonckiere, thanks for the patch!

Transliteration is quite a bit more than just removing diacritics. For instance, if you transliterate Chinese text, you end up with something like "ni hao" instead of the actual characters, and there are some language-specific settings as well, such as in some languages u-with-umlaut is transliterated into "ue" while in others it is just "u".

So, transliteration is not quite the right approach here. What I think we would want to do instead is to give the user the option (on the search settings page) to do one of the following:
- Use database collation (i.e. do nothing, as it is now)
- Remove accents/diacritics (which should be the default)
- Complete transliteration
We'll need to explain what the implications of these three choices are, on the settings page and probably in search_help(). [I added a note about this to the issue summary, which actually already said it should maybe be optional]

And then we'd also need to figure out how to remove diacritics reliably.

One way I know of is to use the "intl" package, but this is package is not available on many web servers by default, so when we were putting Transliteration in Core, we decided not to use it. See discussion on #567832: Transliteration in core for more on that.

If you do a Google search, you'll also find suggestions to use iconv() http://se2.php.net/manual/en/function.iconv.php -- but this is not OK because it will either transliterate or silently remove characters that are non-ASCII, neither of which is what we want.

But I found what looks like a good solution on this page:
http://stackoverflow.com/questions/3371697/replacing-accented-characters...

So I found this on php.net page for preg_replace function

// replace accented chars

$string = "Zacarías Ferreíra"; // my definition for string variable
$accents = '/&([A-Za-z]{1,2})(grave|acute|circ|cedil|uml|lig);/';
$string_encoded = htmlentities($string,ENT_NOQUOTES,'UTF-8');
$string = preg_replace($accents,'$1',$string_encoded);

Something like this should work... I think we'd want to do:

if (... the setting is to remove diacritics ...) {
   $text = htmlentities($text, ENT_NOQUOTES,'UTF-8');
   $accents = '/&([A-Za-z]{1,2})(grave|acute|circ|cedil|uml|lig);/';
   $text = preg_replace($accents, '$1', $text);
   $text = entity_decode($text);
}

Want to try it out?

Sounds good!

Take a look at these pages, which lists some additional things:
http://symbolcodes.tlt.psu.edu/web/codehtml.html
http://www.danshort.com/HTMLentities/index.php?w=latin
http://www.starr.net/is/type/htmlcodes.html

Looks like we should do:
grave
acute
circ
cedil
uml
lig
tilde
slash
ring
caron

Sigh. But the case in the issue summary would still not work, because there is not a named HTML entity for it... sigh.

So. Given that... I'm wondering if we should try a different approach. We actually already have tables in the Transliteration component for transliterating various characters to ASCII. The accent removal is specifically for some lower-numbered ASCII characters. Maybe the right approach would be to make a new method on the Transiteration interface and \Drupal\Component\Transliteration class that would just do diacritical mark removal, but skip all the other stuff? It would look similar to the transliterate() method, but be limited to certain character ranges and not do any of the language-specific stuff. Thoughts?

One other note: we don't have to worry about capitalization in search simplify... so if we stick with the HTML entities approach, we do not need to do A-Z, just a-z.

If we need it to fix a bug, we can do API changes. I'm one of the maintainers of the Transliteration system, by the way. ;)

I guess the thing to do, to avoid any developer impact from adding a method to an interface, would be to make a second interface in the same namespace, with a removeAccents() method only. Then we can implement both of the interfaces in the base PhpTransliteration component class -- no API problems, no impact, definitely OK to do at this phase. And the method could be useful for people, outside of the context for Search. Also, it requires knowledge of the UTF-8 character set similar to what is needed by the existing Transliterate class/interface, so I think it belongs there.

I'm not sure what you mean about adding a method to the Transliteration class being confusing to developers? Transliteration is a very broad operation, as discussed above. Removing diacritics is a very narrow operation. We'll need to document the methods, but I think they can be documented in such a way that it's not confusing which one does what.

And we absolutely do not want to call the existing transliterate() method from Search by default for any characters -- it does a lot more than just removing accents, and we really only do want to remove accents, not transliterate.

Wow, fast work! Thanks again for taking this on!

This is really close. A few thoughts:

1. Yes we will need to update the issue summary and make a change notice. Possibly two change notices... We'll need one for the Transliteration interface, saying there is this new method and noting that it's implemented on the base class and should never need to be overridden. I think we would also need to make a change notice for the Search module explaining that there's a new option for removing diacritics, enabled in the default Search module settings.

2. On the new replaceDiacritics method:

a) I think we should call it removeDiacritics, and I think the first-line summary in the docs should be:

Removes diacritics (accents) from certain letters.

(by the way, first lines of function docs need to be **one-line** summaries, not wrapping to the next line and not exceeding 80 characters) (I'm also the API docs maintainer. Lucky you! :) ).

The rest of the docs look good, except the return value. I think it should say something like:

$string, with accented letters replaced by their unaccented equivalents.

And then we should have a paragraph before the @param section that explains in detail which accented characters this works on, something like:

This only applies to certain letters:
- Accented Latin characters like a-with-acute-accent, in the UTF-8 character range of ... to ...
- ...

b) Agreed it doesn't need unknownCharacter. I think it also doesn't need a language input -- this is not a language-dependent method. It should just have one argument, $string.

c) Definitely, we need to figure out which character ranges this applies to. I've done some research on this in the past, so let me know if you want help figuring that out... See #1823454: Verify transliteration data sources and their quality, and potentially eliminate maintenance, which is where I verified and updated the data tables that the Transliteration system in Drupal is actually using. You may also find http://www.unicode.org/charts/ useful -- check the Latin section. For instance, this Latin 1 page: http://www.unicode.org/charts/PDF/U0080.pdf has a chart of the common accented Latin letters. It shows what the letters look like, and below each one is the Hex code. So, we'd need those, plus some others used in Czech and some other languages. These seem to be in the "supplement" sections.

Although... I have to say that looking at the charts, I am not sure it will be easy to decide which characters to use. It might actually be easy to look at the scripts in #1823454: Verify transliteration data sources and their quality, and potentially eliminate maintenance that use "pecl" and make a new set of data that does a "transform" that just removes diacritical marks, save those as a new data set, and use those instead of the transliteration data set that we have. I'm not sure... any thoughts? I can help with that if you think that would be better.

Also, just a thought, it is probably helpful to use 0x notation for the character codes. So the range for those Latin 1 ones goes from 0xC0 to 0xFF, for instance. May make the code more readable.

3. The call to replace diacritics should only be in search_simplify(). search_index() already calls this (indirectly, as part of search_index_split()).

4. We'll need a test for the actual bug in this issue. The issue summary has a "reproduce" that suggests having a node with meklēt in it, searching for "meklet" without the accents, and verifying that it gets highlighted in the results. Or it could get added to existing tests for excerpt highlighting. If you need help figuring out where to add the tests, let me know and I can take a look.

Yes, we would want the new transliteration method to be tested too. But we need to have a test in the Search module for the bug that was reported here also.

I'm not sure what your other question means: "And building on that, can't we just use such a test to give us the ascii codes we need?"?

Hm. A few thoughts:

a) The generic Transliteration class needs to cover both lower-case and upper-case letters. Search doesn't need this, because it lower-cases everything.

b) I kind of think we should do it right/carefully. So maybe we can use ranges with exceptions? So we'd need:
00C0 - 017E, except for:
00D0 [Eth]
0097 [multiply]
00DE [Thorn]
00F0 [eth]
00F7 [divide]
00FE [thorn]
0138 [kra]
014A [Eng]
014b [eng]

Then there is another range at 01CD - 024F except:
01DD [backwards e]
01EE, 01EF [Ezh]
01F6, 01F7
012C, 012D
0220-0223
0241-0242
0245

I also think we should exclude anything that transliterates to more than one character, like the AE ligature. And exclude anything that transliterates to 0 characters or doesn't have a transliteration.

I think that would be good enough?

Looking at the patch, I have another suggestion: Maybe rather than calling replace() and passing in a "not a language" code, it would be better to make a protected method called lookupReplacement(), or something like that, on the PhpTransliteration class that would do the part of replace() starting at:

   // See if there is a generic mapping for this character.
    $bank = $code >> 8;

And then our new replaceDiacritics() method could have logic like:

if ( ... $code is in our ranges and not one of the exceptions) {
  // passing in "xyz" for unknown character so it will fail the test below
 $newcode = $this->lookupReplacement($code, "xyz");
  if (strlen($newcode) == 1) {
    // replace it
  }
}

My idea on the length check was that *just in case* the data tables change, if we say "Do this whole range, but then make sure the result is exactly 1 character" then if something changes in the data tables, we will at least make sure that we are not doing something odd. And also if you do that, you don't have as many exclusions, because for instance the a-e stuck-together letter looks up to "ae", so you say "NO!" in the length check. And also to make sure that we don't get back "unknown character".

So... Anyway, great work and can I just say again THANK YOU for working on this!

This patch is obviously pretty close! Here are my suggestions for documentation, cleanup, and some minor code changes:

a) When calling the new replaceCharacters() method from removeDiacritics, pass in $unknown_character = 'xyz'.

b) Do the length check -- make sure the replacement is exactly one character; if not, don't do a replacement. If we do that, we don't have to have 00C0 (elided AE) and other characters like that in the list of exclusions, so the routine becomes more efficient.

c) Add a comment to the $exclusions lines in removeDiacritics() with a short explanation of why there are exclusions. Something like:

These two Unicode ranges include the accented US-ASCII letters, with a few characters that aren't accented letters mixed in. So define the ranges and the excluded characters.

d) Coding standards, in PhpTransliteration:

+      // unicode range 0x00bf to 0x017f

All in-code comments should be complete sentences, starting with capital letters and ending with . But... really I don't know that we even need this comment. Comments in code should explain *why*, not tell us *what* the code is doing. So if we include the comment I suggested in (c), we can probably get rid of these two "unicode range" comment lines.

e) Spelling, same class:

+  /**
+   * Lookup the replacement for a certain UTF-8 character code.

Lookup is a noun. The verb is "Look up".

f) In this line, I also think we need to say "Look up the generic replacement", just to clarify that it's not the final answer. Take out the word "certain" too.

g) Coding standards, same class:

+   * @param $code
+   *  The UTF-8 character code.
+   * @param string $unknown_character
+   *  (optional) The character to substitute for characters without transliterated
+   *  equivalents.
+   *
+   * @return string
+   *  US-ASCII replacement character. If it has a mapping, it is returned;
+   *  otherwise, $unknown_character is returned.

There should be 2 spaces of indent under @param and @return.

h) Same section... I think the docs for $unknown_character shouldn't mention "transliterated equivalents", maybe say "without entries in the replacement tables"?

And maybe on the @return we should document that it could be more than one character? The wording sounds like it's always one character. We should fix that on the replace() method too, come to think of it.

i) in the TransliterationInterface:

+   * 01CD to 024F. Replacements that would result in zero or two characters are
+   * excluded as well as characters that would need afull transliteration
+   * (e.g. Eth, Multiply, Divide etc.).

Can we change this last sentence to say:

Replacements that would result in the string changing length are excluded, as well as characters that are not accented US-ASCII letters.

j) PhpTransliterationTest:

+   * @param string $expected
+   *  The expected return from PhpTransliteration::removeDiacritics().

See (g).

k) And a few lines down:

+//    throw new \Exception($result);

Take this line out. :) Nice test by the way, on the transliteration! Very thorough!

l) A few lines down in the test:

+      // unicode range 0x00bf to 0x017f

See (d)

m) We still need a test for the Search issue that this is fixing. But you knew that. :)

n) The change in the Search module is incorrect. It needs to be in search_simplify(), and not in search_index(). See comment #24 item 3. [and also the second change notice, which I think explains the process -- see below]

o) And we need change records... I can help with that -- added:
https://www.drupal.org/node/2447327
https://www.drupal.org/node/2447357

Edited the issue summary slightly.

Removing tag as the test is available and working.

Hi pjonckiere -- this is looking great!

I took a look at the test. It is excellent! It verifies both the desired behavior and that certain characters, like þ, are not transliterated. Very nice work!

The other changes look good too. There are only a couple of very small cosmetic (coding standards) things to fix:

a) In PhpTransliteration::removeDiacritics() - comment lines, including in-code // comments and /** */ docs comments, should never exceed 80 characters in the line. The added comment about the Unicode range is loner than 80 characters. [Code lines are allowed to be longer.]

b) In the docs header for the lookupReplacement() method:

+   *   (optional) The character to substitute for characters without entries in
+   *   the replacement tables

Last line needs to end in .

That is it! Let's just fix these two things and it is ready to go!

Excellent!

As a co-maintainer of both Search module and Transliteration component, I heartily endorse this patch. Change records and beta evaluation have been done, and the issue summary is current. Let's fix this bug.

The only possible question is whether it should be an option (on by default) or whether it should just be an always-on feature. See issue summary for discussion. This patch makes it an always-on feature, which I think is the right thing to do -- people expect this from search, and I think it would just lead to confusion to provide the option to turn it off. It's also not an expensive operation. But if the branch maintainers disagree, we can definitely make it into a Search setting.

Tagging retroactively.

As far as I can see we don't have test coverage for the fact that when you search for "meklet" nodes that contain "meklēt" are found and "meklēt" is highlighted. Shouldn't we be adding this?

Yeah, sorry. I didn't notice. These tests were in the patch in #40 and several things got dropped when the patch was remade in #45.

So here is a new patch. This is the patch in #40, with the interdiff 40-45 applied properly. All of this has been previously reviewed, so should be RTBC for real this time.

This issue is a normal bug fix, and doesn't include any disruptive changes beyond the api addition to the transliteration service and I don't think that overriding this is common at this point, so it is allowed per https://www.drupal.org/core/beta-changes. Committed ba500d4 and pushed to 8.0.x. Thanks!

Yay, thanks!

Sorry to comment on an old issue, but regarding #47 and an option to turn this off - I think this at least is definitely needed. I created a new issue, https://www.drupal.org/node/2858337, to explain why, but basically removing diacritical marks completely changes word meanings in several languages. Some examples: Finnish säde 'ray' -> sade 'rain', Swedish vår 'spring', 'our' -> var 'each', Italian purè 'mashed potatoes' -> pure 'also', Estonian olu 'existence' -> ölu 'beer'. Likely to be an issue in other Scandinavian languages as well (e.g. Danish, Norwegian), possibly also in German.

Understood. The other question is whether people would be sloppy: would they always type in the word with the accent, or would they occasionally search from a keyboard without all the accents, and not type the accent when searching? Also, what does Google do when searching in Swedish or Finnish -- they seem to set the standard for what is expected in searches.

In any case, this issue was about highlighting in results, not search matching -- the core Search module has always found matches irrespective of diacriticals. This issue was just about making the highlighted excerpt consistent with the search algorithm.

If you would like to get either the matching or the highlighting turned into optional, what you should do at this point is to file a new issue. Thanks!

Ah, you're correct; search does indeed return results with and without diacritics, diacritics are just removed before hook_search_preprocess sees the search input. I'm working on stemmers, so this is problematic, but it's a separate issue.

Regarding your question, there are a lot of cases in a lot of languages (not just Scandinavian ones, but e.g. German and French too) where you simply can't drop the accent even if you are feeling lazy, because then you'd be searching for something else entirely. For Finnish, changing Ä into A is no more correct than changing it into I (funnily enough: säde 'ray', sade 'rain', side 'bandage'). Getting results for säde and sade - two completely unrelated words - is unexpected. Google doesn't mix the two, either.

Anyway, sorry for the confusion and thanks for the pointers, I'll amend my other issue and probably file another regarding matching in general.