Don't include HTML content in title search index

[wlach]

Purpose

Fixes issue where the search index would contain HTML content (e.g. tags, &amp escaped content) which inhibited searching and also bloated the size of the index.

References

Closes #13355.

[AA-Turner]

Could you add a CHANGES entry?

A

[wlach]

Best I can tell the title is only used for indexing purposes here (corroborated by tests not failing, I hope).

[AA-Turner]

Will titles with literals still render properly in search results? E.g. https://docs.python.org/dev/search.html?q=migrating+optparse, the second result currently renders as "Migrating optparse code to argparse"

A

[wlach]

For whatever it's worth, here are the changes in the cpython documentation index with this applied:

https://gist.github.com/wlach/d6fd822b8f894fccb7d71f33e1b76198

[AA-Turner]

https://docs.python.org/dev/:

This PR:

[wlach]

Will titles with literals still render properly in search results? E.g. https://docs.python.org/dev/search.html?q=migrating+optparse, the second result currently renders as "Migrating optparse code to argparse"

A

Ah good spot, no, this behaviour is lost. Looking again, one of the (primary?) purposes of this metadata is to render the titles so maybe it is worth the cost/weirdness. Though there is some existing behaviour that assumed that the content was plain-text and won't work properly without it, note how the relevant document is now higher up in the results in the pictured screenshots.

https://github.com/wlach/sphinx/blob/bdf776a3568a3bc211495acacd56d95d946c8d05/sphinx/themes/basic/static/searchtools.js#L340

I'm not really sure what to suggest now. 😅

[jayaddison]

Despite the arguable title-display degradation, I'd tend to agree that HTML doesn't belong in the index representation, and that that's potentially the more important factor.

I'm curious why the total count of results differed (9 vs 10) - is it due to de-duplication of this Migrating optparse code to argparse > Migrating optparse code to argparse result, by any chance?

[wlach]

Despite the arguable title-display degradation, I'd tend to agree that HTML doesn't belong in the index representation, and that that's potentially the more important factor.

I think I'm leaning towards this too, although maybe there's something I'm missing.

I'm curious why the total count of results differed (9 vs 10) - is it due to de-duplication of this Migrating optparse code to argparse > Migrating optparse code to argparse result, by any chance?

Yup this line now operates as expected:

https://github.com/wlach/sphinx/blob/bdf776a3568a3bc211495acacd56d95d946c8d05/sphinx/themes/basic/static/searchtools.js#L343

[jayaddison]

There is one quirk I'd like to be careful about: titles containing greater-than / less-than can currently be interpreted as HTML when found in search results.

An obvious example (although others could perhaps be more subtle):

<code>testing</code>
====================

(note the title-displayed-as-a-code-block in the above; the HTML was interpreted by the browser directly)

[jayaddison]

Perhaps we could locate the code that retrieves/formats the subtitle (where the literals aren't included in the output), and use the same approach as that?

[AA-Turner]

Can we split the title-to-be-searched from the title-to-be-displayed? That would seem to solve the issue?

A

[wlach]

Can we split the title-to-be-searched from the title-to-be-displayed? That would seem to solve the issue?

We could, it would be a pretty straightforward extension of the PR. Looking at it more though, I'm not really sure if seeing the code-type formatting for the titles really makes things any easier to read/skim though.

[wlach]

There is one quirk I'd like to be careful about: titles containing greater-than / less-than can currently be interpreted as HTML when found in search results.

Ah that's a good point, if we kept this behaviour (which I'm leaning towards thinking is the right solution) we would definitely want to escape it before display.

[AA-Turner]

Looking at it more though, I'm not really sure if seeing the code-type formatting for the titles really makes things any easier to read/skim though.

Personally I think the value is more that the search-result heading matches the actual heading on the page. I agree it is a fine balance though---we could proceed as currently proposed and see if users complain that the code formatting is gone.

A

[wlach]

@jayaddison This update to an existing test shows quoting HTML content

[wlach]

Looking at it more though, I'm not really sure if seeing the code-type formatting for the titles really makes things any easier to read/skim though.

Personally I think the value is more that the search-result heading matches the actual heading on the page. I agree it is a fine balance though---we could proceed as currently proposed and see if users complain that the code formatting is gone.

Yeah I think this is what I'm going to recommend for now. I honestly doubt anyone is going to notice the difference and preserving the old behaviour would be a bunch of extra code and bandwidth (to download the extra headers).

[jayaddison]

👍 Looks good to me. I removed myself from the sphinx-doc org, so I can't add the green approved review status.

As I understand it, briefly: we don't escape the user query text, and I think that's as-intended; we'll then match that unescaped query text against the unescaped and text-only title terms (also good; that should improve the match accuracy/fidelity), and other existing index contents, before displaying escaped HTML titles and descriptions in the results.

[wlach]

@AA-Turner I think this is ready to merge if we're agreed this is the right approach.