fix: search page removeChild crash — stop mutating React-owned no-results message - #795
Merged
Merged
Conversation
The search page's no-results message is React-owned, and jellyfin-web's SearchResults renders both the message and the results container as plain keyless divs — so React reuses the same DOM node across those states, and with react-query's cache a previously-typed query transitions directly between them with no <Loading/> commit in between. Overwriting the message's textContent replaced React's own text node, so the next reconciliation of that div threw: NotFoundError: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node. Reproduced deterministically (React 18 + jsdom harness mimicking the jellyfin-web search page) by typing, deleting, and retyping queries with a no-results query in between — exactly the reported repro. The crash disappears with this change and remains absent across the same input sequence. Instead of rewriting the native message's text, hide it with a CSS class (attribute-only changes are safe on React-owned nodes) and insert a plugin-owned replacement message beside it. All cleanup paths now go through clearInjectedSearchResults(), which also removes the replacement message and unhides the native one, so no stale takeover survives query changes, navigation, or Seerr deactivation. Fixes the crash behind PR #792 with a minimal, targeted change.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
NotFoundError: Failed to execute 'removeChild' on 'Node'crash on the search page (the bug behind #792) with a minimal, root-cause-targeted change — no features removed.Root cause
jellyfin-web's
SearchResultsrenders both the no-results message and the results container as plain keyless divs, so React reuses the same DOM node when switching between those states. With react-query's cache, retyping a previously-searched query transitions directly between them (no<Loading/>commit in between). The plugin'spositionSection()did:on the React-owned message div. That replaces React's own text node — so on the next reconciliation of that (reused) div, React calls
removeChildon a node that no longer exists and throws. This matches the reported repro exactly: search → delete part → type another string → delete → correct it, a few times, with a no-results state along the way.Notably, merely inserting the Seerr section among React's children is tolerated by the reconciler — the text mutation was the sole trigger.
Fix
clearInjectedSearchResults()helper removes everything the plugin injected and restores the native message; all four cleanup paths (new query, query cleared, navigation away, Seerr inactive) go through it so no stale takeover survives.3 files, +50/−14. Inline placement, infinite scroll, the Seerr-only filter, the search-field icon, and unbounded results all keep working.
Verification
JellyfinTarget=jf12, .NET 10) and ran it in freshjellyfin/jellyfin:12.0-rc6andjellyfin/jellyfin:unstablecontainers with a scanned movie library and a stubbed Seerr backend, driving the real web UI with headless Chromium through the reporter's repro sequence (type/delete/retype across result and no-result queries):node_modules.react-dom.bundle.js, search page torn down by React Router)node --checkpasses on all touched files;git diff --checkclean.Why not #792's approach
Tested that branch in the same environment: the crash disappears, but only because the integration stops running entirely — jellyfin-web rewrites
?query=on every keystroke, and that branch'sonNavigateteardown cancels its own pending search each time, so no Seerr request is ever made and nothing renders (details in the comment on #792).