diff --git a/shell/plugins/panels/bluetooth/Model.js b/shell/plugins/panels/bluetooth/Model.js index 92961700..55b29bf0 100644 --- a/shell/plugins/panels/bluetooth/Model.js +++ b/shell/plugins/panels/bluetooth/Model.js @@ -80,6 +80,25 @@ function sortedByLabel(devices) { return list } +// Primitives-only projection of a BlueZ device for list-model rows. Holding +// the Device QObject in model data puts a live wrapper into every delegate's +// var property, and BlueZ churn (discovery timeouts, unpair) can destroy the +// object while a delegate is still incubating, which segfaults quickshell. +// Actions resolve the backend object via Panel.deviceFor(). +function deviceRow(d) { + if (!d) return null + return { + address: d.address || "", + name: d.name || "", + deviceName: d.deviceName || "", + connected: !!d.connected, + state: d.state !== undefined ? d.state : -1, + batteryAvailable: !!d.batteryAvailable, + battery: d.battery !== undefined ? d.battery : 0, + pairing: !!d.pairing + } +} + function deviceLists(devices) { var values = toArray(devices) var connected = [] @@ -147,6 +166,7 @@ if (typeof module !== "undefined") { nodeText: nodeText, bluetoothSinkMatchesDevice: bluetoothSinkMatchesDevice, sortedByLabel: sortedByLabel, + deviceRow: deviceRow, deviceLists: deviceLists, cloneMap: cloneMap, pendingAction: pendingAction, diff --git a/shell/plugins/panels/bluetooth/Panel.qml b/shell/plugins/panels/bluetooth/Panel.qml index 845b05b3..a27c7cd7 100644 --- a/shell/plugins/panels/bluetooth/Panel.qml +++ b/shell/plugins/panels/bluetooth/Panel.qml @@ -132,13 +132,36 @@ Panel { readonly property var scrollRows: { var rows = [] for (var k = 0; k < knownDevices.length; k++) - rows.push({ dev: knownDevices[k], section: "known", indexInSection: k }) + rows.push({ dev: Model.deviceRow(knownDevices[k]), section: "known", indexInSection: k }) if (sectionVisible("discovered")) for (var d = 0; d < discoveredDevices.length; d++) - rows.push({ dev: discoveredDevices[d], section: "discovered", indexInSection: d }) + rows.push({ dev: Model.deviceRow(discoveredDevices[d]), section: "discovered", indexInSection: d }) return rows } + // Connected devices render above the scroll area; same primitives-only + // projection so those delegates never hold Device QObject wrappers either. + readonly property var connectedRows: { + var rows = [] + for (var i = 0; i < connectedDevices.length; i++) + rows.push(Model.deviceRow(connectedDevices[i])) + return rows + } + + // Live BlueZ device behind a row. Rows carry primitives only, so actions + // resolve the backend object here rather than holding a wrapper that can + // dangle mid-incubation. `devices` is already the raw device array (see the + // property declaration), so it is iterated directly. + function deviceFor(row) { + if (!row || !row.dev) return null + var addr = row.dev.address || "" + var devs = devices || [] + for (var i = 0; i < devs.length; i++) { + if ((devs[i].address || "") === addr) return devs[i] + } + return null + } + // Flat position of the keyboard cursor, or -1 while it sits on the hero or // in the connected list (both of which live outside the scroll area). readonly property int scrollRowIndex: { @@ -660,7 +683,7 @@ Panel { } Repeater { - model: root.connectedDevices + model: root.connectedRows DeviceRow { required property var modelData required property int index @@ -823,14 +846,15 @@ Panel { } onClicked: function(mouse) { - if (!row.dev) return + var dev = root.deviceFor(row) + if (!dev) return if (mouse.button === Qt.RightButton) { - if (row.isConnected) root.disconnectDevice(row.dev) - else if (!row.isDiscovered) root.forgetDevice(row.dev) + if (row.isConnected) root.disconnectDevice(dev) + else if (!row.isDiscovered) root.forgetDevice(dev) return } - if (row.isConnected) root.disconnectDevice(row.dev) - else root.connectDevice(row.dev) + if (row.isConnected) root.disconnectDevice(dev) + else root.connectDevice(dev) } } @@ -909,8 +933,9 @@ Panel { root.actionFocused = true } onClicked: { - if (!row.dev) return - root.forgetDevice(row.dev) + var dev = root.deviceFor(row) + if (!dev) return + root.forgetDevice(dev) } } } diff --git a/shell/plugins/panels/network/Model.js b/shell/plugins/panels/network/Model.js index ff10f489..01d0f34a 100644 --- a/shell/plugins/panels/network/Model.js +++ b/shell/plugins/panels/network/Model.js @@ -263,8 +263,12 @@ function formatPingLatency(ms, hasSamples) { function wifiRow(network) { if (!network) return null + // Primitives only: rows become list-model data, so a WifiNetwork here puts a + // live QObject wrapper in every delegate's var property. NetworkManager churn + // (scans, AP removals) can destroy the object while a delegate is still + // incubating, which segfaults quickshell in wrap_slowPath on the dangling + // wrapper. Callers that need the object resolve it via networkForSsid(). return { - network: network, connected: !!network.connected, known: !!network.known, ssid: network.name || "", diff --git a/shell/plugins/panels/network/Panel.qml b/shell/plugins/panels/network/Panel.qml index bb160f07..2ed28294 100644 --- a/shell/plugins/panels/network/Panel.qml +++ b/shell/plugins/panels/network/Panel.qml @@ -397,7 +397,10 @@ Panel { var net = wifiNetworks[selectedIndex] if (!net) return if (wifiActionFocused && canForgetNetwork(net)) { forget(net); return } - if (net.connected) { disconnect(net.network); return } + // Only act on a row that still resolves. disconnect() falls back to + // connectedWifiNetwork when handed null, so a row left stale by scan churn + // would otherwise tear down whatever is connected now instead. + if (net.connected) { disconnectRow(net.ssid); return } if (isProtected(net.security) && !net.known) { openPasswordPrompt(net.ssid); return } connectKnown(net.ssid) } @@ -756,8 +759,17 @@ Panel { runNetworkAction("disconnect", network || connectedWifiNetwork, function(net) { net.disconnect() }) } + // Disconnect from a row's SSID. Rows are primitive snapshots that can outlive + // their WifiNetwork, and disconnect()'s null fallback targets whatever is + // connected now, so a stale row must do nothing rather than hit an unrelated + // network. Callers that mean "drop the current connection" call disconnect(). + function disconnectRow(ssid) { + var network = networkForSsid(ssid) + if (network) disconnect(network) + } + function forget(net) { - runNetworkAction("forget", net ? net.network : null, function(network) { network.forget() }) + runNetworkAction("forget", net ? networkForSsid(net.ssid) : null, function(network) { network.forget() }) } implicitWidth: button.implicitWidth @@ -918,7 +930,11 @@ Panel { onPressed: function(b) { if (root.opened) root.close() - else { root.open(); root.refresh() } + // open() is enough: onOpenedChanged runs refresh(true), which defers the + // PHY scan past the first frame. The bare refresh() that used to follow + // took the no-scan branch and set scannerEnabled synchronously, undoing + // that deferral and stalling the open on NetworkManager's AP flood. + else root.open() } } @@ -1577,23 +1593,23 @@ Panel { } Connections { - target: row.net ? row.net.network : null + target: row.net ? root.networkForSsid(row.net.ssid) : null function onConnectionFailed(reason) { // Background auto-connect retries fire this too; only reprompt for // the connect started from this panel. Checked before // failNetworkAction, which clears the action state. var ours = root.actionKind === "connect" && root.actionSsid === (row.net.ssid || "") - root.failNetworkAction(row.net.network, reason) + root.failNetworkAction(root.networkForSsid(row.net.ssid), reason) if (ours && root.shouldRepromptPassphrase(reason, row.isProtected)) root.openPasswordPrompt(row.net.ssid) } function onConnectedChanged() { - if (row.net) root.checkActionCompletion(row.net.network) + if (row.net) root.checkActionCompletion(root.networkForSsid(row.net.ssid)) } function onKnownChanged() { - if (row.net) root.checkActionCompletion(row.net.network) + if (row.net) root.checkActionCompletion(root.networkForSsid(row.net.ssid)) } function onStateChangingChanged() { - if (row.net) root.checkActionCompletion(row.net.network) + if (row.net) root.checkActionCompletion(root.networkForSsid(row.net.ssid)) } } @@ -1642,7 +1658,7 @@ Panel { root.selectedIndex = row.index root.wifiActionFocused = false if (row.isConnected) { - root.disconnect(row.net.network) + root.disconnectRow(row.net.ssid) return } if (row.isProtected && !row.isKnown) { diff --git a/test/shell.d/bluetooth-test.sh b/test/shell.d/bluetooth-test.sh index 9f46f628..9a3b8fe8 100644 --- a/test/shell.d/bluetooth-test.sh +++ b/test/shell.d/bluetooth-test.sh @@ -58,6 +58,17 @@ assertDeepEqual(arrayLikeLists.connected.map(bluetooth.deviceLabel), ['Earbuds'] assertDeepEqual(arrayLikeLists.known.map(bluetooth.deviceLabel), ['Trackpad'], 'bluetooth groups known devices from array-like values') assertDeepEqual(arrayLikeLists.discovered.map(bluetooth.deviceLabel), ['Gamepad'], 'bluetooth groups discovered devices from array-like values') +assertDeepEqual( + bluetooth.deviceRow({ name: 'Deadbeef', address: '1', connected: false }), + { address: '1', name: 'Deadbeef', deviceName: '', connected: false, state: -1, batteryAvailable: false, battery: 0, pairing: false }, + 'bluetooth projects device rows with primitives only' +) +assertEqual( + bluetooth.deviceLabel(bluetooth.deviceRow({ name: 'Generic', deviceName: 'MX Master 3S', address: '2', connected: true })), + 'MX Master 3S', + 'bluetooth keeps deviceName in row projections so labels survive QObject-free rows' +) + assertDeepEqual( bluetooth.withPendingAction({ a: 'connecting' }, 'b', 'forgetting'), { a: 'connecting', b: 'forgetting' }, diff --git a/test/shell.d/network-test.sh b/test/shell.d/network-test.sh index 18007aea..c6b4a947 100644 --- a/test/shell.d/network-test.sh +++ b/test/shell.d/network-test.sh @@ -12,6 +12,24 @@ const panelSource = fs.readFileSync(root + '/shell/plugins/panels/network/Panel. assert(/IpcHandler[\s\S]*?function toggleNetwork\(\) \{ root\.toggleNetwork\(\) \}/.test(panelSource), 'network exposes the Wi-Fi radio toggle over IPC') assert(/manageIpc: false/.test(panelSource), 'network owns its IPC handler so it can extend the target methods') +// Opening from the bar must call open() and nothing else. open() runs +// refresh(true), which defers the PHY scan; a second bare refresh() defaults +// scanWifi to false, sets scannerEnabled synchronously, and stalls the open on +// NetworkManager's access-point flood. +const barPress = panelSource.match(/onPressed: function\(b\) \{[\s\S]*?\n {4}\}/) +assert(barPress, 'network bar button has an onPressed handler') +const barPressCode = barPress[0].replace(/\/\/.*$/gm, '') +assert(!/refresh\(/.test(barPressCode), 'network bar click opens the panel without a second refresh that would undo the deferred scan') + +// A row is a primitive snapshot that can outlive its WifiNetwork, and +// disconnect() falls back to the live connection when handed null, so row +// activation must go through the guarded disconnectRow(). +assert( + /function disconnectRow\(ssid\) \{\s*var network = networkForSsid\(ssid\)\s*if \(network\) disconnect\(network\)/.test(panelSource), + 'network guards row disconnects so a stale row cannot drop an unrelated connection' +) +assert(!/disconnect\(\s*(root\.)?networkForSsid\(/.test(panelSource), 'network never passes an unguarded networkForSsid() lookup to disconnect()') + assertDeepEqual( network.parseNetworkStatus('wifi\tCafe WiFi\t78\t5200\n'), { kind: 'wifi', label: 'Cafe WiFi', signalStrength: 78, frequency: '5200' }, @@ -105,6 +123,18 @@ assertDeepEqual(rows.map(row => row.ssid), ['Connected', 'Known', 'Open'], 'netw assertEqual(network.wifiSectionTitle(rows, 0), 'KNOWN NETWORKS', 'network labels known wifi section') assertEqual(network.wifiSectionTitle(rows, 2), 'OTHER NETWORKS', 'network labels other wifi section') +const wifiRow = network.wifiRow({ connected: true, known: true, name: 'Home', signalStrength: 0.8, security: 1 }) +assertDeepEqual( + wifiRow, + { connected: true, known: true, ssid: 'Home', signal: 80, security: 1 }, + 'network projects wifi rows with primitives so delegates never hold the live WifiNetwork object' +) +assertDeepEqual( + Object.keys(wifiRow).sort(), + ['connected', 'known', 'security', 'signal', 'ssid'], + 'network wifi rows project exactly the primitive fields, so each delegate stores no live QObject' +) + const reasons = { NoSecrets: 1, WifiAuthTimeout: 2, WifiNetworkLost: 3, WifiClientDisconnected: 4, WifiClientFailed: 5 } assertEqual(network.networkFailureReason(1, reasons), 'Passphrase required', 'network maps missing passphrase failures') assertEqual(network.networkFailureReason(2, reasons), 'Wrong password', 'network maps auth timeout failures')