diff --git a/.github/workflows/test-crates.yml b/.github/workflows/test-crates.yml index 98c86c5ff..af7eaf46c 100644 --- a/.github/workflows/test-crates.yml +++ b/.github/workflows/test-crates.yml @@ -16,6 +16,7 @@ # qbz-qt`, the Slint-free dep-graph gate, and an OFFSCREEN BOOT of # debug AND release binaries (including process liveness). # Release also boots on Xvfb with a deliberately silent D-Bus. +# Node executes the actual Local Library filter/tab expressions. # This is the thing `cargo check` cannot give: # a missing QML component or a call to a bridge member that does # not exist compiles clean and throws when the user opens the @@ -152,7 +153,7 @@ jobs: build-essential pkg-config cmake clang libclang-dev lld nasm dbus \ libasound2-dev libjack-jackd2-dev libdbus-1-dev libssl-dev \ libgl1-mesa-dev libegl1-mesa-dev libxkbcommon-dev libwayland-dev \ - xvfb libxcb-cursor0 libxcb-icccm4 libxcb-image0 libxcb-keysyms1 \ + xvfb nodejs libxcb-cursor0 libxcb-icccm4 libxcb-image0 libxcb-keysyms1 \ libxcb-render-util0 libxcb-xinerama0 libxcb-xkb1 libxkbcommon-x11-0 # cxx-qt prefers LLD when present; without it, Ubuntu falls back to # the deprecated gold linker. Do not add workspace-wide RUSTFLAGS. diff --git a/crates/qbz-qt/qml/shell/ContentRouter.qml b/crates/qbz-qt/qml/shell/ContentRouter.qml index 7ec2582c0..915ce4aa9 100644 --- a/crates/qbz-qt/qml/shell/ContentRouter.qml +++ b/crates/qbz-qt/qml/shell/ContentRouter.qml @@ -567,11 +567,14 @@ Item { /// line number.) A handler runs once per item change and registers no /// dependency, so the guard survives and the cycle does not. property bool _itemTabbed: false + property bool _itemTabRequest: false Connections { target: viewLoader function onItemChanged() { root._itemTabbed = viewLoader.item !== null && typeof viewLoader.item.activeTab === "string" + root._itemTabRequest = viewLoader.item !== null + && typeof viewLoader.item.tabNavigationRequest !== "undefined" } } @@ -590,12 +593,14 @@ Item { Binding { target: viewLoader.item - property: "activeTab" + property: root._itemTabRequest ? "tabNavigationRequest" : "activeTab" when: !root._armed && root._itemTabbed && QbzShell.navTab !== "" && QbzShell.navTabView === QbzShell.currentView - value: (QbzShell.navTabSeq, QbzShell.navTab) + value: root._itemTabRequest + ? ({ "tab": QbzShell.navTab, "sequence": QbzShell.navTabSeq }) + : (QbzShell.navTabSeq, QbzShell.navTab) restoreMode: Binding.RestoreNone } } diff --git a/crates/qbz-qt/qml/views/LocalLibraryView.qml b/crates/qbz-qt/qml/views/LocalLibraryView.qml index c71db985c..2d96c01d7 100644 --- a/crates/qbz-qt/qml/views/LocalLibraryView.qml +++ b/crates/qbz-qt/qml/views/LocalLibraryView.qml @@ -87,6 +87,19 @@ Rectangle { return "top" } property string activeTab: localDefaultTab + // Router requests go through the same history path as the in-view bar. + // The sequence makes reselecting a flyout tab after an in-view change work. + property var tabNavigationRequest: ({}) + onTabNavigationRequestChanged: { + if (tabNavigationRequest.tab) activateTab(tabNavigationRequest.tab) + } + function activateTab(tab) { + if (tab === activeTab) return + if (localTabOrder.indexOf(tab) < 0 && !(tab === "ephemeral" && ephemeralActive)) return + if (_navigationReady && !_restoringNavigationState && QbzShell.currentView === "local") + QbzShell.recordLocalTab(tab, navigationStateJson) + activeTab = tab + } // Session-only file reachability. Keys are `source:id`; values are the // user-facing reason. This is deliberately not persisted: a moved file or // unavailable NAS must remain retryable and a rescan remains authoritative. @@ -284,6 +297,28 @@ Rectangle { root._restoringNavigationState = false QbzShell.reportNavState("local", root.navigationStateJson) } + Connections { + target: QbzShell + function onStateRestoreChanged() { + // Back/Forward between tabs keeps this component mounted: there + // is no Component.onCompleted to consume the destination state. + if (root._navigationReady && QbzShell.currentView === "local" + && QbzShell.restoreStateScope === "local" && QbzShell.stateRestore !== "") { + root.restoreNavigationState() + root.loadActiveTab() + } + } + } + Timer { + id: navigationSeed + interval: 0 + onTriggered: { + // Initial persisted state, pending artist and flyout landing tab + // are one arrival, not separate history entries. + root._navigationReady = true + QbzShell.reportNavState("local", root.navigationStateJson) + } + } // One quality/format/source funnel follows the user across Albums, // Artists, Genres and Tracks. Favorites remains an Albums/Genres-only @@ -473,7 +508,7 @@ Rectangle { /// the tab you actually navigated to. Connections { target: QbzLocal - function onLocalEphemeralOpenSeqChanged() { root.activeTab = "ephemeral" } + function onLocalEphemeralOpenSeqChanged() { root.activateTab("ephemeral") } } function loadTabForView(tab) { @@ -621,7 +656,7 @@ Rectangle { function consumePendingArtist() { var pending = QbzLocal.localPendingArtist if (pending === "") return - activeTab = "artists" + activateTab("artists") selectedArtist = pending QbzLocal.clearPendingArtist() } @@ -635,7 +670,7 @@ Rectangle { if (raw === "") return var route try { route = JSON.parse(raw) } catch (e) { QbzLocal.clearPendingRoute(); return } - if (route.tab) activeTab = route.tab + if (route.tab) activateTab(route.tab) // The query pre-filters the TRACKS tab only; the albums and artists // tabs have no search box of their own. if (route.tab === "tracks" && route.query) { @@ -657,19 +692,18 @@ Rectangle { // one query; the Albums/Folders/Artists sets are bounded). Component.onCompleted: { restoreNavigationState() - root._navigationReady = true - // Initial bindings are not a reliable change notification contract: - // explicitly seed history even when this is a fresh entry whose state - // happens to equal every default. - QbzShell.reportNavState("local", root.navigationStateJson) + consumePendingArtist() + consumePendingRoute() + loadActiveTab() + navigationSeed.restart() + } + function loadActiveTab() { if (root.activeTab === "tracks") QbzLocal.tracksSetFilterJson(JSON.stringify(root.tracksFilter)) else root.loadTabForView(root.activeTab) if (root.activeTab === "tracks" && root.tracksSearch !== "") QbzLocal.tracksSearch(root.tracksSearch) - consumePendingArtist() - consumePendingRoute() } Component.onDestruction: { if (QbzLocal.localAlbumsNativeActive) QbzLocal.albumsNativeClearSelection() diff --git a/crates/qbz-qt/qml/views/local/LocalChrome.qml b/crates/qbz-qt/qml/views/local/LocalChrome.qml index 1fcf6849b..a946703aa 100644 --- a/crates/qbz-qt/qml/views/local/LocalChrome.qml +++ b/crates/qbz-qt/qml/views/local/LocalChrome.qml @@ -237,7 +237,7 @@ Item { "count": root.view.ephemeralTrackCount }) return t } - onSelected: function (id) { root.view.activeTab = id } + onSelected: function (id) { root.view.activateTab(id) } } LocalToolbar { diff --git a/crates/qbz-qt/src/nav_qt.rs b/crates/qbz-qt/src/nav_qt.rs index 24186dcf1..4cffd4892 100644 --- a/crates/qbz-qt/src/nav_qt.rs +++ b/crates/qbz-qt/src/nav_qt.rs @@ -62,16 +62,10 @@ struct Entry { /// "album", "library:albums", "local:tracks". Opaque here: this module /// never builds one and never parses one, it only carries it back. /// - /// THIS IS WHY THERE IS NO `scope_for(entry)` TABLE like the Slint build's - /// (`crates/qbz/src/main.rs:3068`). Slint can derive the scope from the - /// entry because ITS history entries carry the tab — `Favorites { tab }` - /// and `LocalLibrary { tab }` are separate entries there. This port's - /// entry is a bare route id, so a derived scope would read `"library"` for - /// all five Library tabs, and coming back would drop the Albums grid's - /// offset into whatever tab the view happened to mount on. Letting the - /// reporter name its own scope buys tab granularity without turning tabs - /// into history entries, and the mismatch case degrades the right way: an - /// unmatched scope restores NOTHING instead of restoring the wrong thing. + /// The route id alone cannot identify a tab's scroll container. Local + /// Library tab changes push entries with view-owned state, while the + /// reporter supplies the exact scope to restore. An unmatched scope + /// restores nothing instead of applying another tab's scroll offset. scope: String, /// Saved `contentY` of that container, in Qt's convention: 0 at the top, /// POSITIVE downward. (Slint stores the same distance as a negative @@ -322,6 +316,32 @@ pub fn shell_entry_view() -> String { } pub fn record(view: &str) { + record_entry(view, false); +} + +/// A user-selected Local Library tab is a distinct history destination even +/// though ContentRouter keeps the same page mounted. Called before QML changes +/// the tab, so the outgoing filters, selection and scroll are still intact. +pub fn record_local_tab(tab: &str, state: &str) { + if current_view() != "local" { + return; + } + let Ok(mut next) = serde_json::from_str::(state) else { + return; + }; + let Some(previous) = next.get("activeTab").and_then(|value| value.as_str()) else { + return; + }; + if previous == tab || tab.is_empty() { + return; + } + set_live_state("local", state); + record_entry("local", true); + next["activeTab"] = serde_json::json!(tab); + set_live_state("local", &next.to_string()); +} + +fn record_entry(view: &str, force: bool) { // Persist BEFORE the history mutation so the write reflects the view the // user actually reached, and only for the safe set — a detail view leaves // the stored value on the last safe root, which is exactly what the @@ -331,7 +351,7 @@ pub fn record(view: &str) { } let (can_back, can_forward, current, pushed) = with_history(|h| { let mut pushed = false; - if h.entries[h.index].view != view { + if force || h.entries[h.index].view != view { // Stamp the page we are leaving with its live scroll position. let (scope, y) = live_scroll(); log::debug!( @@ -444,6 +464,9 @@ fn step(delta: isize) { String::new() }; log::debug!("[qbz-qt] scroll: {current:?} arms scope {armed:?} y={scroll}"); + // A prior flyout request is not the destination of Back/Forward. Leaving + // it armed would override the restored tab when Local Library remounts. + crate::shell_bridge::ui(|mut b| b.as_mut().set_nav_tab(QString::default())); publish( can_back, can_forward, @@ -507,6 +530,61 @@ mod tests { super::reset_live_state(); } + #[test] + fn local_tabs_are_history_entries_with_independent_state_and_scroll() { + let _guard = TEST_LOCK.lock().unwrap(); + reset(); + super::record("home"); + super::record("local"); + let explorer = r#"{"activeTab":"genres","selectedGenres":{"rock":true}}"#; + super::set_live_state("local", explorer); + super::set_live_scroll("local:genres", 412.0); + super::record_local_tab("artists", explorer); + let artists = r#"{"activeTab":"artists","selectedArtist":"Led Zeppelin","artistsFilter":{"local":true}}"#; + super::set_live_state("local", artists); + super::set_live_scroll("local:artists", 620.0); + + super::back(); + assert_eq!(super::current_view(), "local"); + assert_eq!(super::live_state(), ("local".into(), explorer.into())); + assert_eq!(super::live_scroll(), ("local:genres".into(), 412.0)); + super::back(); + assert_eq!(super::current_view(), "home"); + super::forward(); + assert_eq!(super::live_state(), ("local".into(), explorer.into())); + super::forward(); + assert_eq!(super::live_state(), ("local".into(), artists.into())); + assert_eq!(super::live_scroll(), ("local:artists".into(), 620.0)); + + super::back(); + super::record_local_tab("albums", explorer); + assert!(!super::with_history(|history| super::snapshot(history).1)); + assert_eq!( + serde_json::from_str::(&super::live_state().1).unwrap()["activeTab"], + "albums" + ); + super::back(); + assert_eq!(super::live_state(), ("local".into(), explorer.into())); + } + + #[test] + fn local_tab_reselection_does_not_push_or_clear_forward_history() { + let _guard = TEST_LOCK.lock().unwrap(); + reset(); + super::record("local"); + let explorer = r#"{"activeTab":"genres"}"#; + super::record_local_tab("artists", explorer); + super::back(); + let before = super::with_history(|history| (history.entries.len(), history.index)); + super::record_local_tab("genres", explorer); + super::record_local_tab("artists", "bad snapshot"); + assert_eq!( + super::with_history(|history| (history.entries.len(), history.index)), + before + ); + assert!(super::with_history(|history| super::snapshot(history).1)); + } + #[test] fn record_back_forward_cycle() { let _guard = TEST_LOCK.lock().unwrap(); diff --git a/crates/qbz-qt/src/shell_bridge.rs b/crates/qbz-qt/src/shell_bridge.rs index 8091b5405..a37cdd3bd 100644 --- a/crates/qbz-qt/src/shell_bridge.rs +++ b/crates/qbz-qt/src/shell_bridge.rs @@ -519,6 +519,8 @@ pub mod qbz_shell { #[qinvokable] fn report_nav_state(self: Pin<&mut QbzShell>, scope: QString, state: QString); #[qinvokable] + fn record_local_tab(self: Pin<&mut QbzShell>, tab: QString, state: QString); + #[qinvokable] fn local_navigation_state(self: &QbzShell) -> QString; /// Sidebar navigation: record a content view ("home" | "library") @@ -1307,6 +1309,10 @@ impl qbz_shell::QbzShell { QString::from(crate::local_restore_qt::browser_json()) } + pub fn record_local_tab(self: Pin<&mut Self>, tab: QString, state: QString) { + crate::nav_qt::record_local_tab(&tab.to_string(), &state.to_string()); + } + pub fn navigate_to(self: Pin<&mut Self>, view: QString) { crate::navigate_to(&view.to_string()); } diff --git a/scripts/cargo-test.sh b/scripts/cargo-test.sh index 1b170aabf..d839d8425 100755 --- a/scripts/cargo-test.sh +++ b/scripts/cargo-test.sh @@ -34,6 +34,7 @@ # QbzCore initialized, and process still alive at the deadline. # Native Qt SDK content participates in the C++ dependency cache. # 6. release xcb boot against a private silent D-Bus (requires Xvfb). +# The shared runtime gate also executes Local Library QML logic with Node. # # Usage: # ./scripts/cargo-test.sh # job `test` diff --git a/scripts/qt-runtime-gate.sh b/scripts/qt-runtime-gate.sh index 36d086ff5..69cdc5a5f 100644 --- a/scripts/qt-runtime-gate.sh +++ b/scripts/qt-runtime-gate.sh @@ -3,6 +3,7 @@ set -euo pipefail cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." export QBZ_PREBUILT_SHADERS=1 +node scripts/test_qt_local_views.mjs python3 scripts/qt-cargo.py test --manifest-path crates/Cargo.toml -p qbz-qt --no-fail-fast target_dir="${CARGO_TARGET_DIR:-$PWD/crates/target}" logs="$(mktemp -d "${TMPDIR:-/tmp}/qbz-qt-gate-XXXXXX")" diff --git a/scripts/test_qt_local_views.mjs b/scripts/test_qt_local_views.mjs new file mode 100644 index 000000000..14347ad5c --- /dev/null +++ b/scripts/test_qt_local_views.mjs @@ -0,0 +1,78 @@ +#!/usr/bin/env node +// Execute the actual LocalLibraryView expressions against mixed-source rows. +// No Qt window, account, database, or audio input is required. +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import vm from 'node:vm'; +import { fileURLToPath } from 'node:url'; + +const qml = fs.readFileSync(process.argv[2] || fileURLToPath( + new URL('../crates/qbz-qt/qml/views/LocalLibraryView.qml', import.meta.url)), 'utf8'); +function body(marker) { + const offset = qml.indexOf(marker); + assert.ok(offset >= 0, `missing expression: ${marker}`); + const begin = qml.indexOf('{', offset); + for (let depth = 1, pos = begin + 1; pos < qml.length; ++pos) { + if (qml[pos] === '{') ++depth; + if (qml[pos] === '}' && --depth === 0) return qml.slice(begin + 1, pos); + } + throw new Error(`unclosed expression: ${marker}`); +} +const rows = [ + {id: 'local-copy', source: 'user', qualityTier: 'cd', format: 'flac'}, + {id: 'plex-copy', source: 'plex', qualityTier: 'cd', format: 'flac'}, + {id: 'jellyfin-copy', source: 'jellyfin', qualityTier: 'hires', format: 'flac'}, + {id: 'mixed', mediaVariants: [ + {source: 'local', qualityTier: 'cd', format: 'flac'}, + {source: 'qobuz_purchase', qualityTier: 'dsd', format: 'dsd'}, + ]}, +]; +const context = vm.createContext({ + console, albums: rows, selectedArtist: 'Led Zeppelin', artistsFilter: {}, + QbzLocal: {localArtistsNativeActive: false, artistAlbumIds: () => JSON.stringify(rows.map(r => r.id))}, + albumFavorite: () => false, +}); +context.root = context; +vm.runInContext(`function sourceBucket(word) {${body('function sourceBucket(')}} + function applyFilter(rows, selectedFilter) {${body('function applyFilter(')}} + function artistAlbums() {${body('readonly property var artistAlbums:')}}`, context); +function albumIds(filter) { + context.artistsFilter = filter; + return Array.from(context.artistAlbums(), row => row.id); +} +assert.deepEqual(albumIds({local: true}), ['local-copy', 'mixed']); +assert.deepEqual(albumIds({plex: true}), ['plex-copy']); +assert.deepEqual(albumIds({jellyfin: true}), ['jellyfin-copy']); +assert.deepEqual(albumIds({local: true, plex: true}), ['local-copy', 'plex-copy', 'mixed']); +assert.deepEqual(albumIds({local: true, dsd: true}), []); +assert.deepEqual(albumIds({offline: true, dsd: true}), ['mixed']); +assert.deepEqual(albumIds({}), rows.map(r => r.id)); +context.selectedArtist = ''; +assert.deepEqual(albumIds({local: true}), []); + +const transitions = []; +Object.assign(context, { + activeTab: 'genres', localTabOrder: ['genres', 'albums', 'artists', 'folders', 'tracks'], + ephemeralActive: false, _navigationReady: true, _restoringNavigationState: false, + QbzShell: {currentView: 'local', recordLocalTab: (tab, state) => { + assert.equal(JSON.parse(state).activeTab, context.activeTab, 'stamp before switching'); + transitions.push([context.activeTab, tab]); + }}, +}); +Object.defineProperty(context, 'navigationStateJson', { + get: () => JSON.stringify({activeTab: context.activeTab}), +}); +vm.runInContext(`function activateTab(tab) {${body('function activateTab(')}}`, context); +context.activateTab('artists'); +assert.equal(context.activeTab, 'artists'); +context.activateTab('artists'); +context.activateTab('not-a-local-tab'); +assert.deepEqual(transitions, [['genres', 'artists']]); +context._restoringNavigationState = true; +context.activateTab('genres'); +assert.equal(transitions.length, 1, 'history restore must not record itself'); +context._restoringNavigationState = false; +context._navigationReady = false; +context.activateTab('tracks'); +assert.equal(transitions.length, 1, 'initial landing must be one entry'); +console.log('Local view regressions OK: source-filtered artist albums and tab transitions');