Skip to content

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

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

ZCU-DATA/Fix /search console error: drop data-toggle=buttons (Bootstrap null-deref)#1483
milanmajchrak merged 1 commit into
customer/zcu-datafrom
zcu-data/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

Removes the invalid Bootstrap button-toggle attribute causing /search console errors while preserving view-mode behavior.

Changes:

  • Removed data-toggle="buttons".
  • Preserved navigation and active-state handling.
Suppressed comments (1)

src/app/shared/view-mode-switch/view-mode-switch.component.html:1

  • This fix is only guarded by the static search described in the PR; the existing component suite renders this template but never asserts that the wrapper lacks data-toggle="buttons". A future template change could reintroduce the Bootstrap null dereference without a test failure. Add a regression assertion in view-mode-switch.component.spec.ts that the .btn-group has no data-toggle attribute.
<div class="btn-group">

💡 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