diff --git a/.github/workflows/test-crates.yml b/.github/workflows/test-crates.yml index 2d42b924d..ba0f420df 100644 --- a/.github/workflows/test-crates.yml +++ b/.github/workflows/test-crates.yml @@ -86,6 +86,7 @@ on: - "scripts/test-text-edit-menu-qml.sh" - "scripts/test-compact-seek-qml.sh" - "scripts/test-library-folders-qml.sh" + - "scripts/test-array-model-qml.sh" - "scripts/test-orbit-daemon.py" - ".github/workflows/test-crates.yml" push: @@ -114,6 +115,7 @@ on: - "scripts/test-text-edit-menu-qml.sh" - "scripts/test-compact-seek-qml.sh" - "scripts/test-library-folders-qml.sh" + - "scripts/test-array-model-qml.sh" - "scripts/test-orbit-daemon.py" - ".github/workflows/test-crates.yml" workflow_dispatch: diff --git a/crates/qbz-qt/build.rs b/crates/qbz-qt/build.rs index ecb4ae03a..53dd03f98 100644 --- a/crates/qbz-qt/build.rs +++ b/crates/qbz-qt/build.rs @@ -574,6 +574,7 @@ fn main() { "qml/controls/PlaylistImportModal.qml", "qml/controls/PlaylistPickerModal.qml", "qml/controls/PmFolderIcon.qml", + "qml/controls/QbzArrayModel.qml", "qml/controls/QbzCircleAction.qml", "qml/controls/QbzClipboard.qml", "qml/controls/QbzColorPicker.qml", diff --git a/crates/qbz-qt/qml/controls/QbzArrayModel.qml b/crates/qbz-qt/qml/controls/QbzArrayModel.qml new file mode 100644 index 000000000..b5fe9677a --- /dev/null +++ b/crates/qbz-qt/qml/controls/QbzArrayModel.qml @@ -0,0 +1,87 @@ +// QbzArrayModel — a DelegateModel over a JS array that a view can SWAP +// (2026-09-13). Assigning a fresh array straight to `ListView.model` after +// the view has completed has two side effects in Qt 6.11 +// (qquickitemview.cpp, QQuickItemViewPrivate::connectModel): +// +// 1. `currentIndex` is forced to 0 and updateCurrent() calls +// setFocus(true) on delegate 0. The view is a focus scope, so a search +// box living in its header loses the keyboard on every rebuild — the +// album track filter lost focus on each keystroke (qt.quick.focus +// trace: "focus trackCell in scope pageFlick", activeFocusItem +// TextInput -> trackCell). A `currentIndex: -1` on the view does not +// survive the assignment. +// 2. setModel() clears the view and resets contentY to the content start. +// +// Swapping the array HERE instead reaches the view as remove+insert changes: +// `currentIndex: -1` stays honoured (nothing is ever focused) and the view +// keeps its rows. ListView re-seats the inserted rows around the viewport, +// which drifts the content origin, so the viewport is re-expressed relative +// to that origin (0 = the header's top edge) before the change and restored +// once it is laid out. A swap that replaces the page calls `scrollToTop()`. +// +// Probe that measured both behaviours against Qt 6.11.2: +// scripts/qml-tests/tst_array_model.qml. + +import QtQuick +import QtQml.Models + +DelegateModel { + id: swap + + /// The rows. Replace the array freely; the view keeps its place. + property var rows: [] + /// The ListView (or GridView) this model feeds — needed for the restore. + property Item view: null + + model: rows + + property bool _live: false + property int _rowsBefore: 0 + property real _offset: 0 + property int _epoch: 0 + property bool _toTop: false + + Component.onCompleted: { + swap._live = true + swap._rowsBefore = swap.count + } + + onModelChanged: { + if (!swap._live || !swap.view) + return + var had = swap._rowsBefore + swap._rowsBefore = swap.count + // Nothing to keep: the page had no rows (first document, or the + // empty document a detail publishes while the next one loads), or a + // page replacement already asked for the top. + if (had === 0 || swap._toTop) + return + // Before the pending remove+insert is laid out: how far the viewport + // sits below the content origin. + swap._offset = swap.view.contentY - swap.view.originY + var epoch = ++swap._epoch + Qt.callLater(function () { + if (epoch !== swap._epoch || !swap.view) + return + swap.view.forceLayout() + var minY = swap.view.originY + var maxY = minY + Math.max(0, swap.view.contentHeight - swap.view.height) + swap.view.contentY = Math.max(minY, Math.min(minY + swap._offset, maxY)) + }) + } + + /// A swap that replaces the page (a different document): the viewport + /// goes to the top once the new rows are laid out. Works whether it is + /// called before or after the `rows` assignment in the same turn. + function scrollToTop() { + swap._toTop = true + var epoch = ++swap._epoch + Qt.callLater(function () { + swap._toTop = false + if (epoch !== swap._epoch || !swap.view) + return + swap.view.forceLayout() + swap.view.positionViewAtBeginning() + }) + } +} diff --git a/crates/qbz-qt/qml/controls/ScrollMemory.qml b/crates/qbz-qt/qml/controls/ScrollMemory.qml index cb6925528..037218467 100644 --- a/crates/qbz-qt/qml/controls/ScrollMemory.qml +++ b/crates/qbz-qt/qml/controls/ScrollMemory.qml @@ -44,10 +44,12 @@ // every one of those snaps the list back to the top — over a restore that had // already run correctly. // -// (The same reset also throws a plain forward visit to the top if a rail lands -// while you are reading. The re-assert below only stops it from eating the -// restore; fixing that properly means giving those rails a model of their own, -// which is an AlbumView change and not this file's business.) +// (The album page no longer resets: since 2026-09-13 its tape is swapped on a +// QbzArrayModel, which reaches the ListView as remove+insert changes and keeps +// the viewport — see that control for the Qt 6.11 focus theft that forced the +// move. Those swaps drift `originY` instead, which is what `relativeToOrigin` +// is for. The re-assert below stays for every page that still assigns +// `model` directly.) // // So the offset is held in `_wanted` and re-applied on every contentHeight // change, every count change and every contentY move we did not make, until diff --git a/crates/qbz-qt/qml/kiosk/KioskLocalAlbum.qml b/crates/qbz-qt/qml/kiosk/KioskLocalAlbum.qml index 863163a22..45bc0a017 100644 --- a/crates/qbz-qt/qml/kiosk/KioskLocalAlbum.qml +++ b/crates/qbz-qt/qml/kiosk/KioskLocalAlbum.qml @@ -53,14 +53,28 @@ Rectangle { function toggleSelected(id, mods) { selected = selection.next(selected, id, visibleTracks, mods === undefined ? Qt.NoModifier : mods) } readonly property string navigationStateJson: JSON.stringify({trackQuery: trackQuery}) function restoreNavigationState(state) { trackQuery = state.trackQuery || "" } - onDocChanged: exitMultiSelectMode() + // An in-place album switch starts at the top; the first document of a + // mount does not touch the viewport (ScrollMemory may be restoring it). + property string loadedAlbumId: "" + onDocChanged: { + exitMultiSelectMode() + var id = root.album ? String(root.album.id || "") : "" + if (id === root.loadedAlbumId) + return + if (root.loadedAlbumId !== "") + rowsModel.scrollToTop() + root.loadedAlbumId = id + } ListView { id: list anchors.fill: parent anchors.margins: 12 clip: true - model: root.visibleTracks + // Swapped on a QbzArrayModel, never on `model:` — a fresh array there + // makes Qt 6.11 focus delegate 0 and the header's search box loses the + // keyboard on every keystroke (see the control's header). + model: QbzArrayModel { id: rowsModel; view: list; rows: root.visibleTracks } cacheBuffer: height reuseItems: true boundsBehavior: Flickable.StopAtBounds @@ -168,6 +182,6 @@ Rectangle { ] onPicked: function(a) { if (root.selectedCount > 0) root.bulkAction(a) } } - ScrollMemory { target: list; scope: "localalbum" } + ScrollMemory { target: list; scope: "localalbum"; relativeToOrigin: true } QbzScrollBar { target: list; anchors.right: parent.right; anchors.top: parent.top; anchors.bottom: parent.bottom } } diff --git a/crates/qbz-qt/qml/kiosk/KioskPlaylist.qml b/crates/qbz-qt/qml/kiosk/KioskPlaylist.qml index 19db08f59..37eb41fe8 100644 --- a/crates/qbz-qt/qml/kiosk/KioskPlaylist.qml +++ b/crates/qbz-qt/qml/kiosk/KioskPlaylist.qml @@ -84,6 +84,10 @@ Rectangle { var id = doc.id || "" if (id === loadedPlaylistId) return + // An in-place playlist switch starts at the top; the first document + // of a mount leaves the viewport to ScrollMemory. + if (loadedPlaylistId !== "") + rowsModel.scrollToTop() loadedPlaylistId = id setMultiSelect(false) } @@ -396,7 +400,10 @@ Rectangle { cacheBuffer: height reuseItems: true boundsBehavior: Flickable.StopAtBounds - model: root.tracks + // Swapped on a QbzArrayModel, never on `model:` — a fresh array there + // makes Qt 6.11 focus delegate 0, and the header's search box lost the + // keyboard whenever a filtered document landed (see the control). + model: QbzArrayModel { id: rowsModel; view: trackList; rows: root.tracks } header: Column { width: trackList.width spacing: 12 @@ -586,6 +593,6 @@ Rectangle { ] onPicked:function(a){if(root.selectedCount>0)root.bulkAction(a)} } - ScrollMemory { target:trackList; scope:"playlist" } + ScrollMemory { target:trackList; scope:"playlist"; relativeToOrigin:true } QbzScrollBar { target:trackList; anchors.right:parent.right; anchors.top:parent.top; anchors.bottom:parent.bottom } } diff --git a/crates/qbz-qt/qml/views/AlbumView.qml b/crates/qbz-qt/qml/views/AlbumView.qml index 95205bb32..5b50fcc2f 100644 --- a/crates/qbz-qt/qml/views/AlbumView.qml +++ b/crates/qbz-qt/qml/views/AlbumView.qml @@ -59,14 +59,15 @@ Rectangle { // The view's album + url-keyed cover map (artwork pipeline). // - // Applied imperatively so a SAME-ALBUM deferred rail publish can preserve - // the ListView offset. Binding `model` to a freshly parsed JS array makes - // QQuickItemView reset contentY whenever Similar albums / Suggestions / More - // from this artist lands — the forced jump the owner caught while reading. + // Applied imperatively so the page can tell a SAME-ALBUM deferred rail + // publish (Similar albums / Suggestions / More from this artist landing + // while the user reads: the viewport stays put) from a NEW album (the + // track filter resets and the page starts at the top). The tape itself is + // swapped on `cellsModel`, a QbzArrayModel: reassigning the ListView's + // `model` would reset contentY on every publish and, on Qt 6.11, focus a + // delegate — see that control's header for both. property var album: ({}) property string documentAlbumId: "" - property int albumDocumentEpoch: 0 - property real albumRestoreY: 0 function parseAlbumDocument() { try { @@ -82,27 +83,18 @@ Rectangle { var nextId = nextHeader.id !== undefined && nextHeader.id !== null ? String(nextHeader.id) : "" var sameAlbum = nextId !== "" && nextId === root.documentAlbumId - var savedY = sameAlbum ? pageFlick.contentY : 0 - root.albumDocumentEpoch += 1 - var epoch = root.albumDocumentEpoch + if (!sameAlbum && root.documentAlbumId !== "") { + // A different release replacing the one on screen (the empty + // document a load publishes first counts): its track list starts + // unfiltered — the header's search box mirrors `trackQuery` — and + // at the top. The first document of a mount leaves the viewport + // alone: ScrollMemory may be restoring a Back navigation. + root.trackQuery = "" + cellsModel.scrollToTop() + } root.album = next if (nextId !== "") root.documentAlbumId = nextId - if (!sameAlbum || savedY <= pageFlick.originY) - return - root.albumRestoreY = savedY - // The model reset and its content-height polish happen after the - // document assignment. One deferred restore coalesces a publish burst - // and clamps if the replacement document is genuinely shorter. - Qt.callLater(function () { - if (epoch !== root.albumDocumentEpoch - || nextId !== root.documentAlbumId) - return - pageFlick.forceLayout() - var minY = pageFlick.originY - var maxY = minY + Math.max(0, pageFlick.contentHeight - pageFlick.height) - pageFlick.contentY = Math.max(minY, Math.min(root.albumRestoreY, maxY)) - }) } readonly property var albumHeader: album.header || ({}) @@ -850,7 +842,17 @@ Rectangle { anchors.fill: parent clip: true boundsBehavior: Flickable.StopAtBounds - model: root.listCells + // The tape is swapped on this model, never on `model:` itself: a + // fresh array assigned to the ListView after completion makes Qt 6.11 + // force currentIndex 0 and focus delegate 0, which robbed the header's + // track filter of the keyboard on every keystroke (2026-09-13). The + // wrapper also keeps the viewport across a same-album rail publish; + // applyAlbumDocument() asks for the top when the album changes. + model: QbzArrayModel { + id: cellsModel + view: pageFlick + rows: root.listCells + } reuseItems: true currentIndex: -1 // The track tape is cheap enough that a fast wheel can consume one @@ -1561,13 +1563,25 @@ Rectangle { height: 1 } // Track filter (QbzSearchField: Escape clears + blurs, - // clear cross, focus kept across the list rebuild). + // clear cross). The keyboard survives the tape rebuild + // because the tape lives on a QbzArrayModel. QbzSearchField { + id: trackSearch width: 168 height: 34 anchors.verticalCenter: parent.verticalCenter placeholder: QbzSession.tr("Search tracks...", QbzSession.trRev) onEdited: function (text) { root.trackQuery = text } + // A new release resets `trackQuery` + // (applyAlbumDocument); the box follows, so no + // stale query outlives the album it was typed on. + Connections { + target: root + function onTrackQueryChanged() { + if (trackSearch.text !== root.trackQuery) + trackSearch.text = root.trackQuery + } + } } // Multi-select toggle (AlbumPageView.slint:752-787): // accent border + tint while active; leaving the mode @@ -2029,7 +2043,9 @@ Rectangle { // Back/forward scroll memory (controls/ScrollMemory.qml): reports // this container's offset while it is the live page, and restores it // when a back/forward step arms this route. - ScrollMemory { target: pageFlick; scope: "album" } + // Origin-relative: the tape swaps on cellsModel drift ListView.originY, + // so an absolute contentY would restore a different row after Back. + ScrollMemory { target: pageFlick; scope: "album"; relativeToOrigin: true } QbzScrollBar { anchors.right: parent.right anchors.rightMargin: 4 diff --git a/scripts/qml-tests/tst_array_model.qml b/scripts/qml-tests/tst_array_model.qml new file mode 100644 index 000000000..50dde6d25 --- /dev/null +++ b/scripts/qml-tests/tst_array_model.qml @@ -0,0 +1,103 @@ +// QbzArrayModel: swapping the rows array must not rob the view's header of +// the keyboard (Qt 6.11 forces currentIndex 0 on a ListView.model assignment +// and focuses delegate 0), must keep the viewport where it was relative to +// the content origin, and scrollToTop() must land on the origin. +import QtQuick +import QtQml.Models +import Qt.labs.qmlmodels +import QtTest +import "../../crates/qbz-qt/qml/controls" + +Item { + id: root + width: 600; height: 400 + property var cells: [] + function makeCells(n) { + var out = [] + for (var i = 0; i < n; i++) + out.push({ "kind": (i % 5 === 0) ? "track" : "gap", "i": i }) + return out + } + + ListView { + id: view + width: 600; height: 400 + clip: true + reuseItems: true + currentIndex: -1 + cacheBuffer: 800 + model: QbzArrayModel { id: rowsModel; view: view; rows: root.cells } + header: Item { + width: 600; height: 300 + TextInput { objectName: "field"; width: 200; height: 30; y: 250; text: "x" } + } + delegate: DelegateChooser { + role: "kind" + DelegateChoice { roleValue: "gap"; delegate: Item { width: 600; height: 10 } } + DelegateChoice { + roleValue: "track" + delegate: Item { + required property var modelData + width: 600; height: 10 + Rectangle { width: 600; height: 50; color: "#333" } + } + } + } + } + + TestCase { + name: "ArrayModel" + when: windowShown + function field() { return view.headerItem.children[0] } + function settle() { wait(0); waitForRendering(view); wait(0) } + + function test_a_swap_keeps_header_focus_and_current_index() { + root.cells = root.makeCells(200) + settle() + compare(view.count, 200) + var input = field() + input.forceActiveFocus() + verify(input.activeFocus) + root.cells = root.makeCells(150) + settle() + compare(view.count, 150) + compare(view.currentIndex, -1) + verify(input.activeFocus, "the header's field keeps the keyboard across a swap") + } + + function test_b_swap_keeps_viewport_relative_to_origin() { + root.cells = root.makeCells(200) + settle() + view.contentY = view.originY + 1000 + settle() + var before = view.contentY - view.originY + root.cells = root.makeCells(300) + settle() + fuzzyCompare(view.contentY - view.originY, before, 0.5) + compare(view.count, 300) + } + + function test_c_scroll_to_top_lands_on_the_origin() { + root.cells = root.makeCells(200) + settle() + view.contentY = view.originY + 900 + settle() + rowsModel.scrollToTop() + root.cells = root.makeCells(120) + settle() + fuzzyCompare(view.contentY, view.originY, 0.5) + compare(view.count, 120) + } + + function test_d_first_rows_do_not_move_the_viewport() { + root.cells = [] + settle() + compare(view.count, 0) + var y = view.contentY + root.cells = root.makeCells(80) + settle() + compare(view.count, 80) + fuzzyCompare(view.contentY, y, 0.5) + } + } +} diff --git a/scripts/qt-runtime-gate.sh b/scripts/qt-runtime-gate.sh index c6d17b6dc..5af3d6841 100644 --- a/scripts/qt-runtime-gate.sh +++ b/scripts/qt-runtime-gate.sh @@ -9,6 +9,7 @@ bash scripts/test-playback-cache-qml.sh bash scripts/test-text-edit-menu-qml.sh bash scripts/test-compact-seek-qml.sh bash scripts/test-library-folders-qml.sh +bash scripts/test-array-model-qml.sh bash scripts/test-orbit-qml.sh node scripts/test_qt_local_views.mjs node scripts/test_qt_library.mjs diff --git a/scripts/test-array-model-qml.sh b/scripts/test-array-model-qml.sh new file mode 100755 index 000000000..cb8a1c85d --- /dev/null +++ b/scripts/test-array-model-qml.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# QbzArrayModel regression: a rows swap keeps the header's keyboard focus and +# the viewport (Qt 6.11 ListView.model assignment side effects). Same run in +# CI and locally. +set -euo pipefail +cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." +if [[ -n "${QT_ROOT_DIR:-}" ]]; then + qt_array_model_bins="$QT_ROOT_DIR/bin" +else + qt_array_model_qmake="${QMAKE:-}" + if [[ -z "$qt_array_model_qmake" ]]; then + qt_array_model_qmake="$(command -v qmake6 || command -v qmake)" + fi + qt_array_model_bins="$("$qt_array_model_qmake" -query QT_INSTALL_BINS)" +fi +QT_QPA_PLATFORM=offscreen QT_QUICK_BACKEND=software \ + "$qt_array_model_bins/qmltestrunner" \ + -input scripts/qml-tests/tst_array_model.qml \ + -import scripts/qml-tests/imports