Skip to content

Comments

Add test coverage for search highlight persistence (#14047, #9802)#14049

Merged
cderv merged 6 commits intomainfrom
fix/issue-14047
Feb 20, 2026
Merged

Add test coverage for search highlight persistence (#14047, #9802)#14049
cderv merged 6 commits intomainfrom
fix/issue-14047

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Feb 17, 2026 •
edited
Loading

PR #13442 by @jtbayly removed scroll-based search highlight clearing, fixing
both #9802 (highlights cleared on scroll) and #14047 (highlights cleared by
layout events before user sees them).

This PR adds Playwright test coverage and a changelog entry for the expected behavior:

  • Search highlights persist through scroll and layout events at any time
  • Search highlights clear when the user changes the search query
  • No highlights appear without ?q= parameter

Closes #14047

@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented Feb 17, 2026 •
edited
Loading

✅ Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cderv
Copy link
Collaborator Author

cderv commented Feb 17, 2026 •
edited
Loading

FWIW I have find this problem and this fix by looking at some more ideas to improve search user experience as we discussed. I assumed something was not correct, and found that. This is the required change to follow up on my other ideas. I'll share at next meeting what this enable.

@cscheid cscheid added this to the v1.9 milestone Feb 17, 2026
@cderv cderv mentioned this pull request Feb 18, 2026
Closed
@cderv
Copy link
Collaborator Author

cderv commented Feb 18, 2026

Tested the interaction with text fragments from #14003 (context: #13123).

This PR does not worsen the text fragment situation. On fuse.js sites, the highlight() DOM mutation already destroys text fragment highlights before our setTimeout delay has any effect — the two mechanisms are in conflict regardless of this change.

This fix correctly preserves the JS <mark> highlights, which remain the primary highlight mechanism for fuse.js search and the only one that highlights all matches on a page (vs text fragments which highlight a single match). Text fragments continue to work on Algolia-powered sites where highlight() doesn't run.

@cderv cderv mentioned this pull request Feb 18, 2026
Merged
3 tasks
@cderv
Copy link
Collaborator Author

cderv commented Feb 19, 2026

This needs to be looked at in addition to other one

@cderv cderv mentioned this pull request Feb 19, 2026
Merged
@vezwork
Copy link
Contributor

vezwork commented Feb 19, 2026

Other PR for this: #13442

@cderv cderv mentioned this pull request Feb 20, 2026
Open
cderv and others added 4 commits February 20, 2026 17:21
@cderv
Add .gitignore for search-highlight test fixture
16361d0
@cderv @claude
Add test verifying search highlight listeners register after delay
b2b2c34 
Complements the persistence test by confirming that quarto-hrChanged
does clear marks after the 1000ms registration delay elapses.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cderv
Add changelog entry for search highlight fix
2c83a84
@cderv @claude
Add test coverage for search highlight persistence (#14047, #9802)
a5434e6 
PR #13442 removed scroll-based highlight clearing. This commit adds
Playwright tests verifying: scroll events never clear marks, query-change
clearing still works, and no marks appear without ?q= parameter.

Restores test fixture files dropped during rebase (source files were in
the skipped JS fix commit). Updates changelog to credit @jtbayly and
reference both #9802 and #14047.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cderv cderv changed the title Fix search highlights cleared by layout settling events Add test coverage for search highlight persistence (#14047, #9802) Feb 20, 2026
cderv and others added 2 commits February 20, 2026 18:10
@cderv @claude
Simplify scroll persistence test to use actual scrolling
417faf0 
The previous test dispatched quarto-hrChanged/quarto-sectionChanged
custom events, which were leftovers from when search code listened
to those events. Since #13442 removed those listeners entirely,
dispatching those events tested nothing. Replace with actual scroll
behavior which is the real user scenario from #14047.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cderv @claude
Use role-based locators instead of Algolia CSS classes
440b654 
Replace .aa-DetachedSearchButton and .aa-Input with ARIA role
locators (getByRole('button'), getByRole('searchbox')). These are
resilient to Algolia autocomplete class name changes and follow
Playwright best practices used elsewhere in the test suite.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cderv cderv merged commit a3189c4 into main Feb 20, 2026
51 checks passed
@cderv cderv deleted the fix/issue-14047 branch February 20, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.9

Development

Successfully merging this pull request may close these issues.

Search highlights (<mark>) cleared within milliseconds, invisible to users

4 participants

@cderv @posit-snyk-bot @vezwork @cscheid