From c5924978440bd7f815208f2711281ddadaafdf73 Mon Sep 17 00:00:00 2001 From: Anibal Ribeiro Date: Fri, 4 Sep 2026 00:31:28 +0200 Subject: [PATCH 1/3] Resolve both side buttons together Each direction resolved its stored button on its own, so neither could see what the other already occupied. A stored value this build reserves fell back to that direction's own default, which is the button the other direction uses by default, leaving one button mapped to both. Resolving the pair in one pass guarantees two distinct assignable buttons for every combination of stored values. Co-authored-by: Cursor --- Package.swift | 14 ++- Sources/SideButtons/SideButtons.swift | 88 +++++++++++++++++++ Tests/SideButtonsTests/SideButtonsTests.swift | 86 ++++++++++++++++++ 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 Sources/SideButtons/SideButtons.swift create mode 100644 Tests/SideButtonsTests/SideButtonsTests.swift diff --git a/Package.swift b/Package.swift index b3329d6..244d8f1 100644 --- a/Package.swift +++ b/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" ) ] ) diff --git a/Sources/SideButtons/SideButtons.swift b/Sources/SideButtons/SideButtons.swift new file mode 100644 index 0000000..c847283 --- /dev/null +++ b/Sources/SideButtons/SideButtons.swift @@ -0,0 +1,88 @@ +/// 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( + ownDefault: defaultBack, + otherDefault: defaultForward, + legacySwap: legacySwap, + taken: mappedForward + ) + let forward = mappedForward ?? fallback( + ownDefault: defaultForward, + otherDefault: defaultBack, + 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 to use for a direction the user has not mapped, avoiding the one already taken. + private static func fallback( + ownDefault: Int, + otherDefault: Int, + legacySwap: Bool, + taken: Int? + ) -> Int { + // The superseded flag swapped the pair, so an install still relying on it prefers the + // opposite default. + let preferred = legacySwap ? otherDefault : ownDefault + let second = legacySwap ? ownDefault : otherDefault + // The full range is a backstop: with 29 assignable buttons and at most one taken, some + // button is always free, so this can never fail to find one. + let candidates = [preferred, second] + assignable + return candidates.first { $0 != taken } ?? preferred + } +} diff --git a/Tests/SideButtonsTests/SideButtonsTests.swift b/Tests/SideButtonsTests/SideButtonsTests.swift new file mode 100644 index 0000000..4ed765f --- /dev/null +++ b/Tests/SideButtonsTests/SideButtonsTests.swift @@ -0,0 +1,86 @@ +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)) + } + + 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) + + XCTAssertFalse(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. + func testTwoStoredButtonsThatMatchAreSeparated() { + let mapping = SideButtons.resolve(storedBack: 6, storedForward: 6, legacySwap: false) + + XCTAssertNotEqual(mapping.back, mapping.forward) + XCTAssertTrue(SideButtons.isAssignable(mapping.back)) + XCTAssertTrue(SideButtons.isAssignable(mapping.forward)) + } + + /// 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)") + } + } + } + } +} From de65fdcb45269a124ca0ae5b3a9811b725b636fc Mon Sep 17 00:00:00 2001 From: Anibal Ribeiro Date: Fri, 4 Sep 2026 00:31:28 +0200 Subject: [PATCH 2/3] Read side buttons through SideButtons Also look the direction up in a fixed order. Searching the dictionary returned whichever match came first, and its iteration order varies with the process's hash seed, so a doubly-mapped button navigated back in one launch and forward in the next. Co-authored-by: Cursor --- Sources/WinMice/AppSettings.swift | 32 ++++++++++++++++--------------- Sources/WinMice/Navigation.swift | 14 +++++++++----- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/Sources/WinMice/AppSettings.swift b/Sources/WinMice/AppSettings.swift index 47bc57a..e6daa92 100644 --- a/Sources/WinMice/AppSettings.swift +++ b/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 { @@ -114,15 +115,28 @@ final class AppSettings: ObservableObject { } var backButton: Int { - get { button(Key.backButton, swappedTo: NavigationDirection.forward.defaultButton) } + get { sideButtons.back } set { write(newValue, to: Key.backButton) } } var forwardButton: Int { - get { button(Key.forwardButton, swappedTo: NavigationDirection.back.defaultButton) } + 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? { + defaults.hasValue(for: preference) ? defaults[preference] : nil + } + subscript(button direction: NavigationDirection) -> Int { get { switch direction { @@ -149,20 +163,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, 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 { diff --git a/Sources/WinMice/Navigation.swift b/Sources/WinMice/Navigation.swift index 5468d8f..e3b0024 100644 --- a/Sources/WinMice/Navigation.swift +++ b/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,13 @@ 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. 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) { From fe4021d163eafc1d66dd67c07d8d619b0827a5e0 Mon Sep 17 00:00:00 2001 From: Anibal Ribeiro Date: Fri, 4 Sep 2026 00:48:04 +0200 Subject: [PATCH 3/3] Apply review feedback on the side-button fix Keep the superseded swap flag applying only to a direction that was never mapped: a stored value this build reserves falls back to the direction's own default, as it did before, rather than the swapped one. Verified against a reimplementation of the old resolution across 2888 stored-value combinations - the only outcomes that change now are the 110 the old code resolved to one button driving both directions. Also pin the back-wins tie-break, which a mutation showed no test caught, and make the unvalidated button setters private so assign is the only way to write a mapping. Co-authored-by: Cursor --- Sources/SideButtons/SideButtons.swift | 40 +++++++++---------- Sources/WinMice/AppSettings.swift | 6 ++- Sources/WinMice/Navigation.swift | 3 +- Tests/SideButtonsTests/SideButtonsTests.swift | 29 ++++++++++++-- 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/Sources/SideButtons/SideButtons.swift b/Sources/SideButtons/SideButtons.swift index c847283..b3f8849 100644 --- a/Sources/SideButtons/SideButtons.swift +++ b/Sources/SideButtons/SideButtons.swift @@ -48,15 +48,11 @@ public enum SideButtons { } let back = mappedBack ?? fallback( - ownDefault: defaultBack, - otherDefault: defaultForward, - legacySwap: legacySwap, + preferred: preference(ownDefault: defaultBack, wasStored: storedBack != nil, legacySwap: legacySwap), taken: mappedForward ) let forward = mappedForward ?? fallback( - ownDefault: defaultForward, - otherDefault: defaultBack, - legacySwap: legacySwap, + preferred: preference(ownDefault: defaultForward, wasStored: storedForward != nil, legacySwap: legacySwap), taken: back ) return SideButtonMapping(back: back, forward: forward) @@ -69,20 +65,22 @@ public enum SideButtons { return stored } - /// The button to use for a direction the user has not mapped, avoiding the one already taken. - private static func fallback( - ownDefault: Int, - otherDefault: Int, - legacySwap: Bool, - taken: Int? - ) -> Int { - // The superseded flag swapped the pair, so an install still relying on it prefers the - // opposite default. - let preferred = legacySwap ? otherDefault : ownDefault - let second = legacySwap ? ownDefault : otherDefault - // The full range is a backstop: with 29 assignable buttons and at most one taken, some - // button is always free, so this can never fail to find one. - let candidates = [preferred, second] + assignable - return candidates.first { $0 != taken } ?? preferred + /// 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 } } diff --git a/Sources/WinMice/AppSettings.swift b/Sources/WinMice/AppSettings.swift index e6daa92..c0aca3b 100644 --- a/Sources/WinMice/AppSettings.swift +++ b/Sources/WinMice/AppSettings.swift @@ -114,12 +114,14 @@ final class AppSettings: ObservableObject { set { write(newValue, to: Key.navigationMethod) } } - var backButton: Int { + /// 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 { + private var forwardButton: Int { get { sideButtons.forward } set { write(newValue, to: Key.forwardButton) } } diff --git a/Sources/WinMice/Navigation.swift b/Sources/WinMice/Navigation.swift index e3b0024..0263628 100644 --- a/Sources/WinMice/Navigation.swift +++ b/Sources/WinMice/Navigation.swift @@ -59,7 +59,8 @@ final class NavigationController { /// 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. + /// 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? { if buttons[.back] == buttonNumber { return .back } if buttons[.forward] == buttonNumber { return .forward } diff --git a/Tests/SideButtonsTests/SideButtonsTests.swift b/Tests/SideButtonsTests/SideButtonsTests.swift index 4ed765f..31820fc 100644 --- a/Tests/SideButtonsTests/SideButtonsTests.swift +++ b/Tests/SideButtonsTests/SideButtonsTests.swift @@ -16,6 +16,17 @@ final class SideButtonsTests: XCTestCase { 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) @@ -26,7 +37,7 @@ final class SideButtonsTests: XCTestCase { // 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) - XCTAssertFalse(mapping.back == 2) + XCTAssertNotEqual(mapping.back, 2) XCTAssertTrue(SideButtons.isAssignable(mapping.back)) } @@ -53,14 +64,24 @@ final class SideButtonsTests: XCTestCase { /// Nothing in the app can write this, since `assign` trades places instead of mapping a button /// twice, but a hand-edited defaults domain can. - func testTwoStoredButtonsThatMatchAreSeparated() { + /// 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) - XCTAssertNotEqual(mapping.back, mapping.forward) - XCTAssertTrue(SideButtons.isAssignable(mapping.back)) + 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() {