Add debounce to HUD.search() method

[darpham]

Description

This PR adds a debounce mechanism to the HUD.search() method in the content_scripts/hud.js file. The purpose of this change is to improve performance and reduce unnecessary searches when users are typing quickly in the HUD search field.

The implementation uses a setTimeout to delay the search execution by 300ms, clearing any previous timeout if a new search is initiated before the delay expires. This approach helps to minimize the number of searches performed while the user is still typing, potentially reducing strain on system resources and improving the overall user experience.

Related to issue: #4546

Changes made:

1. Added a searchTimeout property to the HUD object to store the timeout ID.
2. Implemented a debounce mechanism in the search method using Utils.setTimeout.
3. Moved the existing search logic inside the debounce callback.

Rationale:

This change aligns with Vimium's design principles, particularly:

1. Reliable: By reducing the frequency of searches, we ensure smoother performance on various websites.
2. Feels native: The debounce mechanism is a common pattern in web applications, making Vimium feel more like a native part of the browser.
3. Simple: The implementation is straightforward and doesn't add significant complexity to the codebase.

Additional context:

The PR introduces a small change (less than 50 LOC) that should be beneficial for many Vimium users, especially those who frequently use the HUD search feature on content-heavy pages.

Testing:

I've run the test suite, and all tests have passed:

❯ ./make.js test
run: started
test-unit: started
Pass (192/192)
test-unit: finished (149ms)
test-dom: started
Listening on http://localhost:35500/
Running dom_tests.html
Pass (109/109)
Running vomnibar_test.html
Pass (3/3)
Unstable API 'Deno.Server.shutdown'. The --unstable flag must be provided.

Additionally, I've manually tested the changes by installing from source on Firefox Browser. The debounce functionality works as expected, reducing the number of searches performed during rapid typing.
Here's a screenshot of the Developer Tools, showing the Debugger Tab and Console (I didn't keep the console.debug()):

Follow-up / Open questions

• Upgrading Deno
• Add debounce test(s) using FakeTime
  • I took a stab at it, but failed to integrate with shoulda, some documentation on how to add tests would be helpful.

[darpham]

@philc @UncleSnail can I have a review when you have time please 🙏

@leoheitmannruiz as an active contributer, I'd appreciate any review or feedback as well.

[leoheitmannruiz]

All I've done is delete a few unused PNGs and add a few SVGs ^^

I'm not capable of reviewing your patch, sorry.

All the best!

[UncleSnail]

I love the idea of debouncing this as there is a real issue when on huge pages. There are two things I would like to see changed.
Firstly, I do not believe this implementation is correctly debouncing. The previously queued callback functions will still run after 300ms, even if another letter was typed. In a debounce, if another letter is typed within 300ms, the original callback should not run. I would suggest looking at reference JS debounce implementations and putting one in the utils next to the setTimeout.
Secondly, in 99.9% of cases, this is not an issue. I don't really want to make every user's experience "more laggy" all the time for a 0.1% use case. Even on a huge example page, the only potential for freezing is when there are LOTS of matches, which typically only occurs when the search input is small (about 3 or fewer letters). Once we have more letters, we don't really need the debouncing, since the number of results has drastically lowered. I would just only debounce when the search term is 3 or fewer letters, and not debounce otherwise. This is an easy way to make the experience snappy for most cases, but also ensure it will not freeze on large pages.
Another (arguably better) option would be to figure out how much of the slow down is from finding and counting the matches, and how much is from rendering the yellow boxes. I bet most is from the yellow boxes. As a way to ensure we always debounce only when we need, we could count the matches immediately all the time, then debounce the box drawing if there is a large number. That is assuming we can verify that will fix the freezing.

Either of those options will introduce some complexity and should come with a comment explaining why it is done that way. @philc will need to approve any PRs and will have helpful thoughts on the complexity of implementations and their benefit. I can, however, help you get the code working correctly so that his job is easier when/if he does decide to review your PR.
Thank you again for finding this issue and making a fix. I hope we can get something merged in to avoid freezes and crashes in the future.

[darpham]

Firstly, I do not believe this implementation is correctly debouncing. The previously queued callback functions will still run after 300ms, even if another letter was typed. In a debounce, if another letter is typed within 300ms, the original callback should not run. I would suggest looking at reference JS debounce implementations and putting one in the utils next to the setTimeout.

Admittedly, I'm not a software developer (by trade) :-) and I appreciate the feedback and guidance. I will review and reference the JS debounce implementation(s) and docs, furthermore I'll create a Utils.debounce() method per your reccommendation.

Secondly, in 99.9% of cases, this is not an issue. I don't really want to make every user's experience "more laggy" all the time for a 0.1% use case. Even on a huge example page, the only potential for freezing is when there are LOTS of matches, which typically only occurs when the search input is small (about 3 or fewer letters).

I agree and adding an if statement to bypass debounce where input data.query has length > 3 is feasible. I'd be curious on your thoughts on how (and if) this would interact when the input query is a regular expression or when the option backwards is enabled, and whether this might necessitate special handling.

Another (arguably better) option would be to figure out how much of the slow down is from finding and counting the matches, and how much is from rendering the yellow boxes.

I didn't mention in this PR or the linked GH Issue, however my findings with Firefox Profiler indicated the bottleneck to be within the mode_find.js as part of finding the matches, specifically the function getAllTextNodes.
This PR serves to provide a robust "band-aid solution" wherein future follow-ups to better identify and address the root cause could be had.

I can, however, help you get the code working correctly so that his job is easier when/if he does decide to review your PR.
Thank you again for finding this issue and making a fix. I hope we can get something merged in to avoid freezes and crashes in the future.

Thank you @UncleSnail, I hope to also have something merged in the near future. I'll fixup the PR to properly debounce while bypassing if search input length is <= 3 letters. If I'm unable to properly implement, I would appreciate some of your time to resolve. I hope contribute more in the future as this is a great FOSS extension.