Search results 'fix' explodes markup generation

[rootwork]

This is my own bug that I introduced in #329. I am really sorry. Here's what markup looks like without any search being performed:

Yeesh. So I'm not sure how best to fix it but at the very least I need to exclude the JS function from running until/unless a search has actually been done.

Creating this issue to establish that it's a known bug. As far as I can tell it doesn't impede the functionality of the site, but a) it has to be having some performance implications, especially on pages with a lot of content, and b) should you be so unfortunate as to have styled your <mark> elements, you'll see them appear between every character in your text. Which is how I discovered this catastrophe. Sigh...sorry.

[rootwork]

I believe this is now fixed with #348

[jbowdre]

I think I'm still seeing this issue on the staging branch of my site after incorporating the latest theme updates (6d90564fef2c4d6783b1e7bfbf2105f81301760e), and it's actually completely crashing the Chrome browser on my Android phone.(I don't see any performance issues on a desktop browser.)

An example of an impacted page: https://staging.virtuallypotato.com/gitea-self-hosted-git-server/

And my staging repo: https://github.com/jbowdre/virtuallypotato/tree/staging

My main branch uses the theme at 69751d47e52293c22100bdd297679dacb9f4e9e3, and it doesn't exhibit the crash: https://www.virtuallypotato.com/gitea-self-hosted-git-server/

[rootwork]

I can definitely see it on your staging site, you're right.

On my local site search doesn't seem to be working at all, but I'll take a look when I get to work tomorrow -- it might be unrelated. But given what you're seeing I'll leave this open until I can test more.

[chipzoller]

348 did fix search for my site (neonmirrors.net)

[onweru]

Hi @rootwork, #348 did not fix this issue, it merely

1. Fixed the index.json URL,
2. prevented looping over falsy nodes using the custom forEach function introduced in #329.

I did identify the cause of this issue though. This function call should not be at the top level of initializeSearch function, which is called automatically on load. It should be inside the liveSearch and passiveSearch. Or maybe in the search function; just once. This way, text will only be highlighted when the user actually does a search.

If you're not already working on this, I could help. What do you say?

I like your new highlight function a lot as it highlights only within text nodes 🤝. My previous function was crude 😆

[rootwork]

@onweru if you have the bandwidth to work on this in the next few days, that would be great. Otherwise I can pitch in partway through the week, but won't have time before then.

Thanks for investigating!

[rootwork]

@onweru Does the variables.js file get pulled in to search.js? I just noticed that on my local install baseURL is erroring in the console as undefined (and thus breaking search).

I had thought the variables were pulled into each JS file using the getJavascriptBundle function but maybe I'm wrong? If there's not an easy way to do that then maybe we should re-declare it in search.js, or at least set a default value for it of /.

[onweru]

@rootwork, yes the contents of variables.js should be accessible to the other scripts. Unless these lines are swapped

[rootwork]

I was wrong about what was happening on my local; it wasn't baseURL, it was because I had an overridden and not-recently-updated baseof.html and my language code was missing.

Anyway, I have a PR for this bug now: #359

[rootwork]

359 has been merged, and after thinking about it I believe we can call this bug fixed. As I noted in that PR, the search-term highlighting could be further improved, but as this bug was about how it negatively affected the rest of the site (outside of searches) and that has now been fixed, I think this can be closed.

[jbowdre]

Thanks for the quick fix! All better on my end after pulling the latest. You folks are great!