Skip to content

Lindat/Search console error on view-mode switch: drop data-toggle=buttons (Bootstrap null-deref) - #1486

Merged
milanmajchrak merged 1 commit into
dtq-devfrom
ufal/fe-fix-view-mode-switch-console-error
Aug 26, 2026
Merged

Lindat/Search console error on view-mode switch: drop data-toggle=buttons (Bootstrap null-deref)#1486
milanmajchrak merged 1 commit into
dtq-devfrom
ufal/fe-fix-view-mode-switch-console-error

Conversation

@Kasinhou

@Kasinhou Kasinhou commented Aug 26, 2026

Copy link
Copy Markdown

One of the fan-out PRs for the /search console-error fix (see the blast-radius table below). The ZCU PRs (#1482, #1483) merged and the Playwright consoleErrors test on /search went green, confirming the fix.

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 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). Not chosen: dropping Bootstrap JS from angular.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.

Customer Version Bootstrap BS-JS + jQuery Status
ZCU-PUB / ZCU-DATA 7.6.1 4.6.1 yes ✅ fixed & merged (#1482 / #1483)
TUL 7.5 4.6.1 yes this fan-out
VSB-TUO 7.6.5 4.6.1 yes this fan-out
dtq-dev / UFAL 7.6.5 4.6.1 yes this fan-out
SAV 7.6.1 4.6.1 yes intentionally deferred (owner's call)
MENDELU / JCU 9.1 / 9.3 5.3 no not affected (BS5 dropped the plugin)

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: 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 Playwright consoleErrors test on /search — already confirmed green on the equivalent ZCU fix.

…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 removes the data-toggle="buttons" attribute from the /search view-mode switch markup to prevent Bootstrap 4’s button data-api from null-dereferencing a missing <input> during window.load, eliminating the flaky/Firefox console error caused by the hydration timing race.

Changes:

  • Drop data-toggle="buttons" from the view-mode switch .btn-group container.
  • Preserve existing active-state behavior via routerLinkActive="active" and [class.active] (no functional change intended).

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

@Kasinhou
Kasinhou requested a review from milanmajchrak August 26, 2026 07:34
@Kasinhou Kasinhou changed the title Fix /search console error on view-mode switch: drop data-toggle=buttons (Bootstrap null-deref) Lindat/Search console error on view-mode switch: drop data-toggle=buttons (Bootstrap null-deref) Aug 26, 2026
@milanmajchrak
milanmajchrak merged commit 914a688 into dtq-dev Aug 26, 2026
6 checks passed
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