From 8e8b58626aff5fe051be67ea4c83b1bf53cd83f6 Mon Sep 17 00:00:00 2001 From: "Frenky Harry S. Sinaga" Date: Fri, 24 Jul 2026 19:27:25 +0700 Subject: [PATCH] test(mac): lock status-item context menu policy (#802) Extract StatusItemContextMenuPolicy (event mask, debounce, presentation mode) and unit-test it so rightMouseUp + statusItemMenu cannot regress to the flash/scroll-jump paths without a failing test. --- mac/Sources/CodeBurnMenubar/CodeBurnApp.swift | 10 ++- .../StatusItemContextMenuPolicy.swift | 47 ++++++++++ .../StatusItemContextMenuPolicyTests.swift | 87 +++++++++++++++++++ 3 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 mac/Sources/CodeBurnMenubar/StatusItemContextMenuPolicy.swift create mode 100644 mac/Tests/CodeBurnMenubarTests/StatusItemContextMenuPolicyTests.swift diff --git a/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift b/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift index 1fc54e2..e60cf3c 100644 --- a/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift +++ b/mac/Sources/CodeBurnMenubar/CodeBurnApp.swift @@ -922,7 +922,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate, NSM // Presenting on up (after the click completes) keeps it open. Harmless // on 15/26 too (the debounce in showContextMenu prevents a double-present // if the legacy path also fires). - rightClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: [.rightMouseUp]) { [weak self] _ in + rightClickMonitor = NSEvent.addGlobalMonitorForEvents(matching: StatusItemContextMenuPolicy.presentEventMask) { [weak self] _ in guard let self, let button = self.statusItem.button, let window = button.window, @@ -1139,9 +1139,11 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSPopoverDelegate, NSM private func showContextMenu(from button: NSStatusBarButton) { // Debounce: on macOS <= 26 both the legacy action path and the global // monitor can fire for a single right-click. Present at most once per click. - let now = Date() - guard now.timeIntervalSince(lastContextMenuPresentedAt) > 0.3 else { return } - lastContextMenuPresentedAt = now + // Policy lives in StatusItemContextMenuPolicy so the gate is unit-tested (#802). + guard StatusItemContextMenuPolicy.acceptPresent( + now: Date(), + lastPresentedAt: &lastContextMenuPresentedAt + ) else { return } // Don't let an open popover steal the click / sit under the menu. if popover?.isShown == true { diff --git a/mac/Sources/CodeBurnMenubar/StatusItemContextMenuPolicy.swift b/mac/Sources/CodeBurnMenubar/StatusItemContextMenuPolicy.swift new file mode 100644 index 0000000..845a9f3 --- /dev/null +++ b/mac/Sources/CodeBurnMenubar/StatusItemContextMenuPolicy.swift @@ -0,0 +1,47 @@ +import AppKit +import Foundation + +/// Pure policy for the status-item right-click menu (#802). +/// +/// Keeps the event-mask, debounce, and presentation choices out of AppDelegate +/// so they can be unit-tested without spinning up an NSStatusItem. The bugs this +/// encodes: +/// +/// 1. **Flash** — presenting on `rightMouseDown` lets the matching `rightMouseUp` +/// dismiss the menu immediately. Monitor (and legacy action) use mouse-*up*. +/// 2. **Jump/scroll** — manual `NSMenu.popUp(at:in:)` tracks poorly while the +/// cursor still sits on the status item above the menu. Attach via +/// `statusItem.menu` + `performClick` instead. +/// 3. **Double-present** — on macOS ≤26 both the button action and the global +/// monitor can fire for one click; debounce collapses them. +enum StatusItemContextMenuPolicy { + /// Global-monitor / action event. Must be mouse-up (see flash note above). + static let presentEventMask: NSEvent.EventTypeMask = .rightMouseUp + + /// Minimum gap between presents. Covers the dual-path race on macOS ≤26. + static let presentDebounceSeconds: TimeInterval = 0.3 + + /// How the menu is shown once a present is accepted. + enum Presentation: Equatable { + /// Assign `statusItem.menu` then `button.performClick`. AppKit positions + /// and tracks the menu under the status item. Clear menu in menuDidClose. + case statusItemMenu + /// Manual `NSMenu.popUp(at:in:)`. Causes scroll-chevron jump on mouse move + /// when the cursor starts above the menu. Kept only as a named anti-pattern + /// so tests can lock that we do *not* use it. + case manualPopUp + } + + static let presentation: Presentation = .statusItemMenu + + /// Returns true and advances `lastPresentedAt` when a new present is allowed. + static func acceptPresent( + now: Date, + lastPresentedAt: inout Date, + debounce: TimeInterval = presentDebounceSeconds + ) -> Bool { + guard now.timeIntervalSince(lastPresentedAt) > debounce else { return false } + lastPresentedAt = now + return true + } +} diff --git a/mac/Tests/CodeBurnMenubarTests/StatusItemContextMenuPolicyTests.swift b/mac/Tests/CodeBurnMenubarTests/StatusItemContextMenuPolicyTests.swift new file mode 100644 index 0000000..e601fa8 --- /dev/null +++ b/mac/Tests/CodeBurnMenubarTests/StatusItemContextMenuPolicyTests.swift @@ -0,0 +1,87 @@ +import AppKit +import XCTest +@testable import CodeBurnMenubar + +/// Locks the right-click menu policy that fixed flash + scroll-jump (#802). +/// Does not drive a real NSStatusItem — that needs manual/AppKit integration. +final class StatusItemContextMenuPolicyTests: XCTestCase { + func testPresentEventMaskIsRightMouseUpNotDown() { + // Presenting on mouse-down lets the matching up dismiss the menu (flash). + XCTAssertEqual( + StatusItemContextMenuPolicy.presentEventMask, + NSEvent.EventTypeMask.rightMouseUp + ) + XCTAssertNotEqual( + StatusItemContextMenuPolicy.presentEventMask, + NSEvent.EventTypeMask.rightMouseDown + ) + // Mask must include up and must not include down (single-bit masks here). + XCTAssertTrue(StatusItemContextMenuPolicy.presentEventMask.contains(.rightMouseUp)) + XCTAssertFalse(StatusItemContextMenuPolicy.presentEventMask.contains(.rightMouseDown)) + } + + func testPresentationUsesStatusItemMenuNotManualPopUp() { + // Manual popUp tracks against a point while the cursor sits on the status + // item above the menu → scroll chevron / Today-row jump on mouse move. + XCTAssertEqual( + StatusItemContextMenuPolicy.presentation, + .statusItemMenu + ) + XCTAssertNotEqual( + StatusItemContextMenuPolicy.presentation, + .manualPopUp + ) + } + + func testDebounceAcceptsFirstPresent() { + var last = Date.distantPast + let now = Date(timeIntervalSince1970: 1_000) + XCTAssertTrue( + StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last) + ) + XCTAssertEqual(last, now) + } + + func testDebounceRejectsWithinWindow() { + let t0 = Date(timeIntervalSince1970: 1_000) + var last = t0 + // Just inside the 0.3s window + let t1 = t0.addingTimeInterval(0.299) + XCTAssertFalse( + StatusItemContextMenuPolicy.acceptPresent(now: t1, lastPresentedAt: &last) + ) + XCTAssertEqual(last, t0, "reject must not advance lastPresentedAt") + } + + func testDebounceAcceptsAfterWindow() { + let t0 = Date(timeIntervalSince1970: 1_000) + var last = t0 + let t1 = t0.addingTimeInterval(StatusItemContextMenuPolicy.presentDebounceSeconds + 0.001) + XCTAssertTrue( + StatusItemContextMenuPolicy.acceptPresent(now: t1, lastPresentedAt: &last) + ) + XCTAssertEqual(last, t1) + } + + func testDebounceBoundaryIsStrictlyGreaterThan() { + // Gate uses `>` not `>=`: exactly debounce seconds later is still rejected. + let t0 = Date(timeIntervalSince1970: 1_000) + var last = t0 + let exact = t0.addingTimeInterval(StatusItemContextMenuPolicy.presentDebounceSeconds) + XCTAssertFalse( + StatusItemContextMenuPolicy.acceptPresent(now: exact, lastPresentedAt: &last) + ) + XCTAssertEqual(last, t0) + } + + func testCustomDebounceOverride() { + var last = Date(timeIntervalSince1970: 0) + let now = Date(timeIntervalSince1970: 0.5) + XCTAssertFalse( + StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last, debounce: 1.0) + ) + XCTAssertTrue( + StatusItemContextMenuPolicy.acceptPresent(now: now, lastPresentedAt: &last, debounce: 0.4) + ) + } +}