Skip to content

ZCU-PUB/Fix /search console error: drop data-toggle=buttons (Bootstrap null-deref) - #1482

Merged
milanmajchrak merged 1 commit into
customer/zcu-pubfrom
zcu-pub/fe-fix-view-mode-switch-console-error
Aug 25, 2026
Merged

ZCU-PUB/Fix /search console error: drop data-toggle=buttons (Bootstrap null-deref)#1482
milanmajchrak merged 1 commit into
customer/zcu-pubfrom
zcu-pub/fe-fix-view-mode-switch-console-error

Conversation

@Kasinhou

Copy link
Copy Markdown

Problem

The Playwright consoleErrors.spec.ts"page 'search…' should not have console errors" fails on /search with an uncaught pageerror (Firefox consistently; all engines intermittently, as a hydration race):

  • Firefox: can't access property "checked", s is null
  • Chromium: Cannot read properties of null (reading 'checked')
  • WebKit: 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.json scriptsnode_modules/bootstrap/dist/js/bootstrap.bundle.min.js) plus jQuery 2.1.4 (src/index.html). On window.load, Bootstrap's load.bs.button.data-api handler iterates [data-toggle="buttons"] .btn and reads .checked on querySelector('input:not([type="hidden"])').

The search view-mode switch renders <div class="btn-group" data-toggle="buttons"> with <button routerLink> children and no <input>, so querySelector returns null and .checked throws. The inputless buttons only exist after Angular hydration, so whether the crash fires depends on a window.load-vs-hydration race — hence the Firefox-only / flaky appearance.

Change set

Remove the data-toggle="buttons" attribute from src/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 ViewModeSwitchComponent is not themeable (no ThemedViewModeSwitchComponent), 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/.css rules target [data-toggle="buttons"]; no spec asserts it) and is branch-isolated. Not chosen: dropping Bootstrap JS from angular.json (the issue's "Alternative" — broader blast radius).

Test evidence

git grep 'data-toggle="buttons"' -- src/        -> 0 after change (was 1, only this file)
grep '[data-toggle="buttons"]' **/*.{scss,css}  -> 0 (no styling depends on it)
view-mode-switch.component.spec.ts              -> no assertion on the attribute

Local full CI (yarn lint / check-circ-deps / build:prod / test:headless) was not run here: these 7.6.1 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 Playwright consoleErrors.spec.ts run on /search.

Risk & rollback

Very low — removing an inert attribute. The toggle's appearance and behavior are unchanged (active state comes from routerLinkActive/[class.active]). Rollback: revert the single-line commit.

Notes / assumptions

  • The same latent markup exists on other branches that bundle Bootstrap 4 JS (TUL, and dtq-dev/UFAL). Out of scope here — worth a follow-up backport.
  • Not addressed (tracked separately per UFAL/Cherry-pick - update the static licenses #931): the Firefox timeouts in submissionPage.spec.ts:47 / universalPage.spec.ts:157 at homePage.ts:299.

…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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a flaky/engine-specific /search console crash caused by Bootstrap’s data-toggle="buttons" data-API assuming the presence of an <input> inside .btn elements. By removing the incorrect/inert attribute from the view-mode switch markup, it prevents Bootstrap’s JS from dereferencing null during window.load (especially visible in Firefox and in hydration races).

Changes:

  • Removed data-toggle="buttons" from the view-mode switch .btn-group to prevent Bootstrap’s button plugin from running on input-less routerLink buttons.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants