Stop one mouse button from driving both navigation directions

Package.swift

@@ -28,9 +28,16 @@ let package = Package(
                 .unsafeFlags(["-warnings-as-errors"])
             ]
         ),
+        .target(
+            name: "SideButtons",
+            path: "Sources/SideButtons",
+            swiftSettings: [
+                .unsafeFlags(["-warnings-as-errors"])
+            ]
+        ),
         .executableTarget(
             name: "WinMice",
-            dependencies: ["SwipeGesturePoster", "ScrollEngine"],
+            dependencies: ["SwipeGesturePoster", "ScrollEngine", "SideButtons"],
             path: "Sources/WinMice",
             swiftSettings: [
                 .unsafeFlags(["-warnings-as-errors"])
@@ -54,6 +61,11 @@ let package = Package(
             name: "ScrollEngineTests",
             dependencies: ["ScrollEngine"],
             path: "Tests/ScrollEngineTests"
+        ),
+        .testTarget(
+            name: "SideButtonsTests",
+            dependencies: ["SideButtons"],
+            path: "Tests/SideButtonsTests"
         )
     ]
 )

Sources/SideButtons/SideButtons.swift

@@ -0,0 +1,86 @@
+/// Which mouse button drives each navigation direction.
+public struct SideButtonMapping: Equatable, Sendable {
+    public let back: Int
+    public let forward: Int
+
+    public init(back: Int, forward: Int) {
+        self.back = back
+        self.forward = forward
+    }
+}
+
+public enum SideButtons {
+    /// Button numbers each direction uses until the user maps another one. CoreGraphics numbers
+    /// from zero, so the left, right, and middle buttons are 0 through 2 and the side buttons
+    /// start at 3.
+    public static let defaultBack = 3
+    public static let defaultForward = 4
+
+    /// Any mouse button the event tap can see and does not already own. Left and right stay
+    /// reserved so Set / Cancel clicks and the menu bar keep working, and the middle button
+    /// belongs to autoscroll — mapping it would leave a scroll latched with no way to end it.
+    public static let assignable = 3...31
+
+    public static func isAssignable(_ button: Int) -> Bool {
+        assignable.contains(button)
+    }
+
+    /// - Parameters:
+    ///   - storedBack: The stored back button, or `nil` when the user has never set one.
+    ///   - storedForward: The stored forward button, or `nil` when the user has never set one.
+    ///   - legacySwap: The superseded `swapSideButtons` flag, still honored so existing installs
+    ///     keep the choice they made under it.
+    /// Both directions resolve together because a fallback has to see what the other direction
+    /// already occupies. Resolving them one at a time is what let a stored value the app no longer
+    /// accepts fall back onto a button the other direction was using, leaving one button driving
+    /// both — and leaving which one fired up to a dictionary lookup, i.e. to the hash seed.
+    public static func resolve(
+        storedBack: Int?,
+        storedForward: Int?,
+        legacySwap: Bool
+    ) -> SideButtonMapping {
+        let mappedBack = mapped(storedBack)
+        var mappedForward = mapped(storedForward)
+        // Two stored values that match cannot both stand. Back keeps the button and forward is
+        // resolved as though it had none, so the result does not depend on which is read first.
+        if mappedBack == mappedForward {
+            mappedForward = nil
+        }
+
+        let back = mappedBack ?? fallback(
+            preferred: preference(ownDefault: defaultBack, wasStored: storedBack != nil, legacySwap: legacySwap),
+            taken: mappedForward
+        )
+        let forward = mappedForward ?? fallback(
+            preferred: preference(ownDefault: defaultForward, wasStored: storedForward != nil, legacySwap: legacySwap),
+            taken: back
+        )
+        return SideButtonMapping(back: back, forward: forward)
+    }
+
+    /// The button the user actually chose, or `nil` when they never chose one or chose one this
+    /// build reserves — a value an older build let through, say.
+    private static func mapped(_ stored: Int?) -> Int? {
+        guard let stored, isAssignable(stored) else { return nil }
+        return stored
+    }
+
+    /// The button a direction would use if nothing else were competing for it.
+    ///
+    /// The superseded flag swapped the pair, but only for a direction the user had never mapped.
+    /// A stored value this build reserves is not the same as no value at all: it falls back to the
+    /// direction's own default with the swap ignored, which is what it has always done.
+    private static func preference(ownDefault: Int, wasStored: Bool, legacySwap: Bool) -> Int {
+        legacySwap && !wasStored ? other(than: ownDefault) : ownDefault
+    }
+
+    /// Steps aside when the preferred button is already taken. Total because the two defaults
+    /// differ, so the pair always holds one button that is not the taken one.
+    private static func fallback(preferred: Int, taken: Int?) -> Int {
+        preferred == taken ? other(than: preferred) : preferred
+    }
+
+    private static func other(than button: Int) -> Int {
+        button == defaultBack ? defaultForward : defaultBack
+    }
+}

Sources/WinMice/AppSettings.swift

@@ -1,6 +1,7 @@
 import Combine
 import Foundation
 import ServiceManagement
+import SideButtons
 
 /// How a middle-click starts and stops autoscrolling.
 enum ScrollMode: Hashable, CaseIterable, Identifiable {
@@ -113,16 +114,31 @@ final class AppSettings: ObservableObject {
         set { write(newValue, to: Key.navigationMethod) }
     }
 
-    var backButton: Int {
-        get { button(Key.backButton, swappedTo: NavigationDirection.forward.defaultButton) }
+    /// Private because the setters take a button without validating it. `assign(_:to:)` is the way
+    /// in: it rejects buttons the tap cannot claim and keeps the two directions on separate ones.
+    private var backButton: Int {
+        get { sideButtons.back }
         set { write(newValue, to: Key.backButton) }
     }
 
-    var forwardButton: Int {
-        get { button(Key.forwardButton, swappedTo: NavigationDirection.back.defaultButton) }
+    private var forwardButton: Int {
+        get { sideButtons.forward }
         set { write(newValue, to: Key.forwardButton) }
     }
 
+    /// Resolved as a pair, so neither direction can fall back onto the button the other uses.
+    private var sideButtons: SideButtonMapping {
+        SideButtons.resolve(
+            storedBack: stored(Key.backButton),
+            storedForward: stored(Key.forwardButton),
+            legacySwap: defaults[Key.swapSideButtons]
+        )
+    }
+
+    private func stored(_ preference: Preference<Int>) -> Int? {
+        defaults.hasValue(for: preference) ? defaults[preference] : nil
+    }
+
     subscript(button direction: NavigationDirection) -> Int {
         get {
             switch direction {
@@ -149,20 +165,8 @@ final class AppSettings: ObservableObject {
         self[button: direction] = button
     }
 
-    /// Any mouse button the event tap can see and does not already own. Left and right (0 and 1)
-    /// stay reserved so Set / Cancel clicks and the menu bar keep working, and the middle button
-    /// (2) belongs to autoscroll — mapping it would leave a scroll latched with no way to end it.
     nonisolated static func isAssignable(_ button: Int) -> Bool {
-        (3...31).contains(button)
-    }
-
-    private func button(_ preference: Preference<Int>, swappedTo swapped: Int) -> Int {
-        let stored = defaults.hasValue(for: preference)
-            ? defaults[preference]
-            : (defaults[Key.swapSideButtons] ? swapped : preference.defaultValue)
-        // A button an older build let through but this one reserves falls back to the default, so
-        // the mapping settings shows is always the one actually in effect.
-        return Self.isAssignable(stored) ? stored : preference.defaultValue
+        SideButtons.isAssignable(button)
     }
 
     var triggerOnMouseDown: Bool {

Sources/WinMice/Navigation.swift

@@ -1,4 +1,5 @@
 import ApplicationServices
+import SideButtons
 import SwipeGesturePoster
 
 enum NavigationMethod: String, Hashable, CaseIterable, Identifiable, PreferenceRepresentable {
@@ -28,12 +29,10 @@ enum NavigationDirection: Hashable, CaseIterable, Identifiable {
         }
     }
 
-    /// Button number this direction uses until the user maps another one. Buttons 0 through 2 are
-    /// the left, right, and middle buttons, so the side buttons start at 3.
     var defaultButton: Int {
         switch self {
-        case .back: 3
-        case .forward: 4
+        case .back: SideButtons.defaultBack
+        case .forward: SideButtons.defaultForward
         }
     }
 
@@ -58,8 +57,14 @@ final class NavigationController {
 
     private let keyboardSource = CGEventSource(stateID: .hidSystemState)
 
+    /// Checked in a fixed order rather than by searching the dictionary, whose iteration order
+    /// varies with the process's hash seed. `SideButtons.resolve` rules out one button driving both
+    /// directions, so the order only decides what happens if that guarantee is ever broken — and
+    /// back comes first to match the tie-break `resolve` uses, so the two agree even then.
     func direction(for buttonNumber: Int64) -> NavigationDirection? {
-        buttons.first { $0.value == buttonNumber }?.key
+        if buttons[.back] == buttonNumber { return .back }
+        if buttons[.forward] == buttonNumber { return .forward }
+        return nil
     }
 
     func perform(_ direction: NavigationDirection) {

Tests/SideButtonsTests/SideButtonsTests.swift

@@ -0,0 +1,107 @@
+import SideButtons
+import XCTest
+
+final class SideButtonsTests: XCTestCase {
+    func testUnsetMappingUsesDefaults() {
+        let mapping = SideButtons.resolve(storedBack: nil, storedForward: nil, legacySwap: false)
+
+        XCTAssertEqual(mapping, SideButtonMapping(back: 3, forward: 4))
+    }
+
+    /// The `swapSideButtons` flag predates per-direction button numbers, so an install that set it
+    /// and never mapped a button by hand has to keep the swapped pair.
+    func testLegacySwapWithNothingStoredSwapsTheDefaults() {
+        let mapping = SideButtons.resolve(storedBack: nil, storedForward: nil, legacySwap: true)
+
+        XCTAssertEqual(mapping, SideButtonMapping(back: 4, forward: 3))
+    }
+
+    /// The flag only ever applied to a direction the user had never mapped. A stored value this
+    /// build reserves is not the same thing as no value at all: it falls back to the direction's
+    /// own default, with the swap ignored. Here back is mapped to 21 by hand and forward holds a
+    /// button an intermediate build allowed, so forward must land on its own default of 4 rather
+    /// than the swapped 3.
+    func testLegacySwapDoesNotApplyToStoredButUnusableValues() {
+        let mapping = SideButtons.resolve(storedBack: 21, storedForward: 2, legacySwap: true)
+
+        XCTAssertEqual(mapping, SideButtonMapping(back: 21, forward: 4))
+    }
+
+    func testStoredButtonsAreKept() {
+        let mapping = SideButtons.resolve(storedBack: 7, storedForward: 9, legacySwap: false)
+
+        XCTAssertEqual(mapping, SideButtonMapping(back: 7, forward: 9))
+    }
+
+    func testReservedStoredButtonFallsBackToADefault() {
+        // The middle button belongs to autoscroll, so a build that once allowed it must not win.
+        let mapping = SideButtons.resolve(storedBack: 2, storedForward: nil, legacySwap: false)
+
+        XCTAssertNotEqual(mapping.back, 2)
+        XCTAssertTrue(SideButtons.isAssignable(mapping.back))
+    }
+
+    /// The regression this type exists for. An older build allowed the middle button for forward,
+    /// and back was mapped to 4 by hand. Resolving each direction on its own sent the unusable
+    /// forward value back to *its* default — which is 4, the button back already occupies. Both
+    /// directions then answered to button 4, and which one fired was decided by the dictionary
+    /// lookup in `NavigationController`, i.e. by the process's hash seed.
+    func testReservedForwardButtonDoesNotCollideWithAMappedBackButton() {
+        let mapping = SideButtons.resolve(storedBack: 4, storedForward: 2, legacySwap: false)
+
+        XCTAssertEqual(mapping.back, 4, "an explicitly mapped button must be honored")
+        XCTAssertNotEqual(mapping.forward, mapping.back)
+        XCTAssertTrue(SideButtons.isAssignable(mapping.forward))
+    }
+
+    func testReservedBackButtonDoesNotCollideWithAMappedForwardButton() {
+        let mapping = SideButtons.resolve(storedBack: 0, storedForward: 3, legacySwap: false)
+
+        XCTAssertEqual(mapping.forward, 3, "an explicitly mapped button must be honored")
+        XCTAssertNotEqual(mapping.back, mapping.forward)
+        XCTAssertTrue(SideButtons.isAssignable(mapping.back))
+    }
+
+    /// Nothing in the app can write this, since `assign` trades places instead of mapping a button
+    /// twice, but a hand-edited defaults domain can.
+    /// Back keeps the button, so the outcome does not depend on which direction is read first.
+    /// `NavigationController.direction(for:)` checks back first for the same reason.
+    func testTwoStoredButtonsThatMatchResolveInFavorOfBack() {
+        let mapping = SideButtons.resolve(storedBack: 6, storedForward: 6, legacySwap: false)
+
+        XCTAssertEqual(mapping.back, 6)
+        XCTAssertNotEqual(mapping.forward, 6)
+        XCTAssertTrue(SideButtons.isAssignable(mapping.forward))
+    }
+
+    /// `fallback` steps aside to the other default when the preferred one is taken, which only
+    /// yields a free button while the two defaults differ.
+    func testTheTwoDefaultsDiffer() {
+        XCTAssertNotEqual(SideButtons.defaultBack, SideButtons.defaultForward)
+        XCTAssertTrue(SideButtons.isAssignable(SideButtons.defaultBack))
+        XCTAssertTrue(SideButtons.isAssignable(SideButtons.defaultForward))
+    }
+
+    /// The invariant the whole type owes its callers: whatever is stored, one button never drives
+    /// both directions, and every button it hands back is one the tap is allowed to claim.
+    func testEveryCombinationResolvesToTwoDistinctAssignableButtons() {
+        let candidates: [Int?] = [nil, -1, 0, 1, 2, 3, 4, 5, 31, 32, 99]
+
+        for back in candidates {
+            for forward in candidates {
+                for legacySwap in [false, true] {
+                    let mapping = SideButtons.resolve(
+                        storedBack: back,
+                        storedForward: forward,
+                        legacySwap: legacySwap
+                    )
+                    let inputs = "back: \(back as Int?), forward: \(forward as Int?), swap: \(legacySwap)"
+
+                    XCTAssertNotEqual(mapping.back, mapping.forward, "collision for \(inputs)")
+                    XCTAssertTrue(SideButtons.isAssignable(mapping.back), "unassignable back for \(inputs)")
+                    XCTAssertTrue(SideButtons.isAssignable(mapping.forward), "unassignable forward for \(inputs)")
+                }
+            }
+        }
+    }
+}