Skip to content
This repository has been archived by the owner on Sep 7, 2023. It is now read-only.

Ignore double-quotes when highlighting query parts #2553

Merged

Conversation

danielhones
Copy link
Contributor

What does this PR do?

This is to fix a minor UI bug where using a search with an exact phrase in quotes wouldn't highlight the matching phrase in the search results. This replaces #901 which I'll close.

For example:

search: master of none
result: master of none

search: "master of none"
result: master of none

The cause of this bug is that a query like "master of none" gets split into ['"master', 'of', 'none"'], and the code tries to highlight strings like none" which aren't typically in the text. This change works by ignoring double quotes in the query. I added test coverage for other paths through the webutils.highlight_content function. Currently there are no tests that cover the scenario where there needs to be span elements inserted in the result.

Why is this change important?

Without this change, only the interior parts of a phrase in quotes will be highlighted. In many cases that might be the least important words of the phrase and it's surprising behavior to a user.

How to test this PR locally?

The unit tests for this function pass and to test manually, you can search for several queries, first without quotation marks, and then with them, and the highlighted words in the results should be the same.

Author's checklist

I considered pulling the existing test cases in to the new data tuple in the unit test, but I decided it was useful to keep them separate because they're all testing edge cases or cases where no highlights would be inserted.

Copy link
Contributor

@return42 return42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect / thanks!

@dalf dalf self-requested a review February 9, 2021 07:36
Copy link
Contributor

@dalf dalf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you !

@dalf dalf merged commit 6c51309 into searx:master Feb 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants