diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b1ef16..ccab899 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **Tuner: opening the player screen no longer throws `NotFoundError` and aborts the player render (feedBack#800).** `injectPlayerButton()` anchored the injected Tuner button with `controls.querySelector('button:last-child')`, which — unlike a `:scope`-scoped query — can match a **nested** button that is not a direct child of `#player-controls`. `controls.insertBefore(btn, nestedButton)` then throws `NotFoundError` (the reference node must be a direct child), and because the injection runs from the tuner's `screen:changed` → player handler, the throw propagated out of the player-screen transition and stalled its render (surfaced by a headless render of a notation arrangement; the v3 path was already safe via the plugin-control slot, only the classic path had the bad anchor). The anchor is now `:scope > button:last-of-type` (a direct child only) with a `parentNode === controls` guard before `insertBefore`, falling back to `appendChild`. `plugins/tuner` → 1.3.4. Tests: `tests/plugins/tuner/js/inject_player_button.test.js` (nested-last-button repro, direct-child insert, no-button append, idempotency, v3 slot path). - **Auto-sync: DTW step constraint — riff-based songs no longer produce garbage sync points.** `librosa.sequence.dtw`'s default step pattern allows unbounded horizontal/vertical path runs, and on music with long self-similar chroma stretches (riff-driven stoner/doom, drone sections) the flat cost surface let the warping path collapse — minutes of score mapped onto a single audio frame, so the per-bar warp imported charts wildly out of sync while reporting success (observed on a real 138 BPM tab: effective displayed tempo 159 BPM, three sync points sharing one audio timestamp). `_dtw_align` now uses the standard music-sync slope-constrained step pattern (`[[1,1],[1,2],[2,1]]`, local tempo ratio bounded to 0.5x–2x), which makes the degenerate path impossible, with a fallback to unconstrained steps when the global length ratio makes the constrained pattern infeasible (e.g. a tab aligned against a full-concert video). Validated on the failing song: coarse points track the recording 1:1, refined downbeats land on onset peaks at 3.3x background energy. ### Added diff --git a/plugins/tuner/plugin.json b/plugins/tuner/plugin.json index 50105f6..937c43b 100644 --- a/plugins/tuner/plugin.json +++ b/plugins/tuner/plugin.json @@ -1,7 +1,7 @@ { "id": "tuner", "name": "Guitar/Bass Tuner", - "version": "1.3.3", + "version": "1.3.4", "bundled": true, "private": false, "script": "screen.js", diff --git a/plugins/tuner/utils/ui.js b/plugins/tuner/utils/ui.js index fa96deb..91b098d 100644 --- a/plugins/tuner/utils/ui.js +++ b/plugins/tuner/utils/ui.js @@ -869,8 +869,15 @@ window._tunerUI = function(state, actions) { btn.textContent = 'Tuner'; btn.title = 'Open Tuner'; btn.onclick = window.tuner.toggle; - const closeBtn = isV3 ? null : controls.querySelector('button:last-child'); - if (closeBtn) controls.insertBefore(btn, closeBtn); + // Anchor to the last DIRECT-child button of `controls` (the classic + // transport's close/exit button). A bare `button:last-child` can match + // a NESTED button that is not a direct child of `controls`, and + // `insertBefore()` then throws NotFoundError — which propagated out of + // the player-screen transition and aborted its render (feedBack#800). + // `:scope > button:last-of-type` restricts the anchor to a direct child; + // the parentNode check is a belt-and-suspenders guard before insertBefore. + const closeBtn = isV3 ? null : controls.querySelector(':scope > button:last-of-type'); + if (closeBtn && closeBtn.parentNode === controls) controls.insertBefore(btn, closeBtn); else controls.appendChild(btn); updatePlayerButton(); } diff --git a/tests/plugins/tuner/js/inject_player_button.test.js b/tests/plugins/tuner/js/inject_player_button.test.js new file mode 100644 index 0000000..2331fc3 --- /dev/null +++ b/tests/plugins/tuner/js/inject_player_button.test.js @@ -0,0 +1,191 @@ +// Regression test for feedBack#800: tuner injectPlayerButton() must anchor the +// injected button to a DIRECT-child button of #player-controls. The old +// `controls.querySelector('button:last-child')` could resolve to a NESTED +// button, and `controls.insertBefore(btn, nestedButton)` then throws +// NotFoundError — which propagated out of the player-screen transition and +// aborted its render. +// +// Same isolation strategy as the core tests/js suite: extract the real function +// from source with extractFunction() and run it in a vm sandbox over a small +// but faithful DOM model. The model's insertBefore() enforces the real DOM +// invariant (reference node must be a direct child, else NotFoundError), and +// querySelector() implements the exact semantics of both the old +// (`button:last-child`) and new (`:scope > button:last-of-type`) selectors — so +// reverting the fix makes this test throw. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +const { extractFunction } = require('../../../js/test_utils'); + +const UI_JS = path.join(__dirname, '..', '..', '..', '..', 'plugins', 'tuner', 'utils', 'ui.js'); +const SRC = fs.readFileSync(UI_JS, 'utf8'); +const FN_SRC = extractFunction(SRC, 'function injectPlayerButton('); + +// ── Minimal, faithful DOM model ────────────────────────────────────────────── + +class El { + constructor(tag, id = '') { + this.tagName = tag.toUpperCase(); + this.id = id; + this.children = []; + this.parentNode = null; + this.textContent = ''; + this.title = ''; + this.onclick = null; + } + appendChild(node) { + node.parentNode = this; + this.children.push(node); + return node; + } + insertBefore(node, ref) { + const idx = this.children.indexOf(ref); + if (ref == null || idx === -1) { + // Faithful to the browser: ref must be a direct child. + const e = new Error( + "Failed to execute 'insertBefore' on 'Node': The node before which the " + + 'new node is to be inserted is not a child of this node.' + ); + e.name = 'NotFoundError'; + throw e; + } + node.parentNode = this; + this.children.splice(idx, 0, node); + return node; + } + querySelector(sel) { + if (sel === ':scope > button:last-of-type') { + // Last direct-child