VSB-TUO/Fix /search console error: drop data-toggle=buttons (Bootstrap null-deref) - #1485
Merged
milanmajchrak merged 1 commit intoAug 26, 2026
Conversation
…ull-deref on /search
The search view-mode switch rendered <div class="btn-group" data-toggle="buttons">
with <button routerLink> children and no <input>. On window.load, the bundled
Bootstrap 4 JS runs its load.bs.button.data-api handler, which iterates
[data-toggle="buttons"] .btn and reads .checked on
querySelector('input:not([type="hidden"])'); with no input it dereferences null
and throws an uncaught pageerror ("can't access property 'checked', s is null"),
failing the Playwright consoleErrors test on /search.
The attribute is semantically wrong here (there are no toggle inputs) and inert in
stock ng-bootstrap DSpace; routerLinkActive/[class.active] already drive the active
state, so the list/grid/detail toggle is unchanged visually and functionally.
Fixes dataquest-dev/dspace-customers#931
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Removes an invalid Bootstrap 4 data-toggle="buttons" attribute from the search view-mode switch markup to prevent Bootstrap’s data-api handler from null-dereferencing a missing <input> during the /search load/hydration race, eliminating flaky Playwright console errors.
Changes:
- Drop
data-toggle="buttons"from the view-mode switch.btn-groupcontainer. - Leave view-mode active-state behavior driven by
routerLinkActive/[class.active](no functional toggle behavior change).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
One of the fan-out PRs for the
/searchconsole-error fix (see the blast-radius table below). The ZCU PRs (#1482, #1483) merged and the PlaywrightconsoleErrorstest on/searchwent green, confirming the fix.Problem
The Playwright
consoleErrors.spec.ts→ "page 'search…' should not have console errors" fails on/searchwith an uncaughtpageerror(Firefox consistently; all engines intermittently, as a hydration race):can't access property "checked", s is nullCannot read properties of null (reading 'checked')null is not an object (evaluating 's.checked')Fixes dataquest-dev/dspace-customers#931
Root cause
The theme bundles Bootstrap 4.6.1 JS (
angular.jsonscripts→node_modules/bootstrap/dist/js/bootstrap.bundle.min.js) plus jQuery 2.1.4 (src/index.html). Onwindow.load, Bootstrap'sload.bs.button.data-apihandler iterates[data-toggle="buttons"] .btnand reads.checkedonquerySelector('input:not([type="hidden"])').The search view-mode switch renders
<div class="btn-group" data-toggle="buttons">withrouterLinkchildren and no<input>, soquerySelectorreturnsnulland.checkedthrows. The inputless buttons only exist after Angular hydration, so whether the crash fires depends on awindow.load-vs-hydration race — hence the Firefox-only / flaky appearance.Change set
Remove the
data-toggle="buttons"attribute fromsrc/app/shared/view-mode-switch/view-mode-switch.component.html— the minimal change that removes the element the handler chokes on. The attribute is semantically wrong here (there are no toggle<input>s) and inert in stock ng-bootstrap DSpace;routerLinkActive="active"+[class.active]already drive the active state, so the list/grid/detail toggle is unchanged visually and functionally.Scope note (core vs theme): the markup is in DSpace core, but
ViewModeSwitchComponentis not themeable (noThemedViewModeSwitchComponent), so a ThemedComponent override would require adding a wrapper to core — a larger core change. Removing the inert attribute is provably safe for every customer (0.scss/.cssrules target[data-toggle="buttons"]; no spec asserts it). Not chosen: dropping Bootstrap JS fromangular.json(the issue's "Alternative" — broader blast radius).Blast radius (why this branch is included)
The crash needs all three at once: the attribute present + Bootstrap 4 JS bundled + jQuery loaded.
Test evidence
Local full CI (
yarn lint/check-circ-deps/build:prod/test:headless) was not run here: the 7.x branches use the Yarn 1.x + Node 16/18 toolchain and this box is on Node 22 (documented incompatibility). Authoritative gates: this branch's CI + the PlaywrightconsoleErrorstest on/search— already confirmed green on the equivalent ZCU fix.