Skip to content

Conversation

@haampie
Copy link
Contributor

@haampie haampie commented Sep 30, 2025

Closes #13916

Alternative to #13918

  • Remove support for ?highlight= query param as a (manual) fallback for search highlighting.

While at it:

  • Simplify document.querySelectorAll("div.body")[0] to document.querySelector("div.body")
  • Simplify document.querySelector("body") with document.body
  • Replace setTimeout with requestAnimationFrame

Purpose

This PR removes the ?highlight= related code, for two reasons:

  1. Sphinx does not use ?highlight= links internally.
  2. Browser native support for Text Fragments #:~:text=... is a decent replacement for ?highlight=... for adding highlighting manually.
  3. The current logic runs window.history.replaceState unconditionally. This has an unpleasant side effect: it removes #:~:text=... fragments from the URL, making it hard to purposefully share URLs that include highlighted text.

References

* Do not use `?highlight=` query param as a fallback for search
  highlighting. All major browsers support `#:~:text=` natively now, and
  Sphinx does not link from search results with `?highlight=` URLs, it
  only uses `localStorage`.
* Simplify `document.querySelectorAll("div.body")[0]` to
  `document.querySelector("div.body")`
* Simplify `document.querySelector("body")` with `document.body`
* Replace `setTimeout` with `requestAnimationFrame`
@haampie haampie changed the title sphinx_highlight.js: remove highlight query param handling sphinx_highlight.js: remove ?highlight= query param handling Sep 30, 2025
haampie added a commit to spack/spack that referenced this pull request Oct 7, 2025
Use Sphinx 8.2.3 with this patch:
sphinx-doc/sphinx#13921, so that it's easier to
link particular paragraphs in the text using Text Fragments.
haampie added a commit to spack/spack that referenced this pull request Oct 7, 2025
Use Sphinx 8.2.3 with this patch:
sphinx-doc/sphinx#13921, so that it's easier to
link particular paragraphs in the text using Text Fragments.

Signed-off-by: Harmen Stoppels <me@harmenstoppels.nl>
@mgeier
Copy link
Contributor

mgeier commented Nov 8, 2025

  • Sphinx does not use ?highlight= links internally.

It depends on what you mean by "use". It supports them in so far as if you append ?highlight=..., the given word will be highlighted.

It doesn't "use" it in so far as it doesn't show URLs with ?highlight=... when you click on a search result.

Until a few years ago, it was added to the URL, and I made a PR for it to be removed when clicking "Hide Search Matches": #9551.

Some people didn't like that ?highlight=... was added at all (#10833) and this behavior was removed in #10854.

I still think that this was a mistake and that removing ?highlight=... when clicking "Hide Search Matches" is the better solution. IMHO the real problem is that many themes show "Hide Search Matches" not prominent enough (or not at all).

  • Browser native support for Text Fragments #:~:text=... is a decent replacement for ?highlight=... for adding highlighting manually.

I don't think it is a decent replacement, since it only highlights the first occurrence, while ?highlight=... highlights all occurrences, which is what I would expect.

@haampie
Copy link
Contributor Author

haampie commented Nov 8, 2025

I don't think it is a decent replacement, since it only highlights the first occurrence, while ?highlight=... highlights all occurrences, which is what I would expect.

So, ?highlight=... is clearly only useful for sharing links, right? When not sharing, you'd do Ctrl+F instead of modifying a URL and reloading the page.

If it's for sharing links, then I would argue #:~:text=... is superior because it scrolls the fragment into view, making it immediately clear what to read from the page. If you open a link with ?highlight= and the keyword is in multiple places, it's unclear which part is relevant, and it requires the user to scroll to the first match (they may use Ctrl+F again?)

That to me suggests there's not a good reason to keep the feature.

@mgeier
Copy link
Contributor

mgeier commented Nov 21, 2025

So, ?highlight=... is clearly only useful for sharing links, right?

Yes, nowadays it is. But previously, it was also there whenever clicking on a search result, which I think was better.

When not sharing, you'd do Ctrl+F instead of modifying a URL and reloading the page.

Maybe, but in many cases I'd already see the highlighted text. If not, I'd scroll a bit to see if I find it, and if the page seems too large, I'd probably use Ctrl+F or equivalent.

If it's for sharing links, then I would argue #:~:text=... is superior because it scrolls the fragment into view, making it immediately clear what to read from the page.

There might be situation where it's superior, but it might also not be.

For example, if the search term is in the middle or end of a paragraph, the beginning of the paragraph might be scrolled out of view.

And what if the relevant hit is not the first appearance of the search term?

I think the best scenario would be for the search result page to show the sub-sections where the search term occurs, and then a user would click on the right sub-section, most likely arriving at the correct place within the page (Similar to what @jayaddison alluded to in #13916 (comment)).

BTW, there are several other objections by @jayaddison in #13916, which we shouldn't forget here.

If you open a link with ?highlight= and the keyword is in multiple places, it's unclear which part is relevant, and it requires the user to scroll to the first match (they may use Ctrl+F again?)

You are assuming that the first match is the intended one?

I think it is preferable to see all possible places (even if it's unclear which is the relevant one) than seeing only one match, which is probably not the relevant one.

@jayaddison
Copy link
Contributor

NB: I think we can close this as it has been superseded by #13918 which has been merged - thanks, @haampie @AA-Turner.

@AA-Turner AA-Turner closed this Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sphinx_highlight.js removes text fragment from URL

4 participants