Bind paste transactions to the selected item, target and clipboard ownership - #7
Draft
MiMoHo wants to merge 8 commits into
Draft
Bind paste transactions to the selected item, target and clipboard ownership#7MiMoHo wants to merge 8 commits into
MiMoHo wants to merge 8 commits into
Conversation
…licked one The pollPB: timer runs in NSRunLoopCommonModes, so a clipboard change can be noticed while the status menu is open (deliberate, for Universal Clipboard). That triggers updateMenu, which removes every clipping item and inserts new NSMenuItem objects. If the rebuild lands between the user's click and the action dispatch, the clicked item is no longer in the menu, [sender menu] is nil, and [[sender menu] indexOfItem:sender] messages nil and yields 0 -- pasting stack position 0, the most recently copied clipping, instead of the entry the user clicked. With the 1-second poll interval this commonly happens when the user copies something and opens the menu right away. - processMenuClippingSelection: bail out when the sender is orphaned or its index can't be resolved, instead of pasting the wrong clipping. - pasteIndexAndUpdate: now returns whether a clipping was placed on the pasteboard, so no Cmd-V is faked when nothing was pasted (previously that re-pasted whatever was already on the pasteboard). Also bounds-check the search mapping, which could throw NSRangeException. - searchWindowItemSelected: use clickedRow for double-clicks so a selection change between click and action can't redirect the paste to row 0, and bounds-check the search mapping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pollPB: runs in NSRunLoopCommonModes so it keeps firing during menu tracking. When it notices a clipboard change mid-selection it inserts a new clipping at store index 0, shifting every clipping's index by one. The pending selection is then resolved against the shifted store, so the user pastes the wrong entry - typically the freshly-arrived newest one - instead of the one they clicked. The same shift breaks the search window, whose result list is a snapshot while getPasteFromIndex: uses live indices. Guard pollPB: to skip capture while isMenuOpen or isSearchWindowDisplayed, without touching pbCount so the change is still detected and captured the moment the surface closes (menuDidClose: fires a catch-up poll). Track menu open state via menuWillOpen:/menuDidClose:. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the first commit, which stopped the wrong paste but could only bail out: when the rebuild had already detached the clicked item there was nothing left to resolve, so the click did nothing. Each clipping row now carries what it stands for instead of relying on where it sits: -updateMenuContaining: attaches [store position, display string] as the item's representedObject, and -storePositionForMenuItem: resolves the click from that, using the position only as a hint that is re-validated against the current store. -[FlycutClipping displayString] returns the same string object on every call and -previousDisplayStrings: passes those objects through unchanged, so pointer equality identifies exactly one clipping; a content comparison is only used as a fallback for a store reloaded from disk. A menu rebuild between the click and the delivery of the action is therefore harmless rather than merely detected. While the store position is known at build time, the search mapping is resolved there as well, so -pasteStorePositionAndUpdate: (renamed from -pasteIndexAndUpdate:) no longer reads the menu's search box at paste time. That box is cleared asynchronously by -updateMenu, so a click arriving after the clearing used to resolve a filtered row number against the unfiltered store - another way to paste the newest clipping. The bezel path benefits too: -moveItemAtStackPositionToTopOfStack passes a store position, which the old method would have run through the menu's search mapping. Menu items removed by a rebuild are held for one generation. jcMenu is their only owner, so a sender AppKit delivers after the rebuild would otherwise be a freed object - which the first commit's nil check would already have had to touch. When a click still cannot be resolved, nothing is pasted, the failure is logged without any clipping contents and NSBeep gives feedback, so "nothing happened" can be told apart from a wrong paste in a bug report. Co-Authored-By: Claude <noreply@anthropic.com>
…tring The previous commit claimed that -[FlycutClipping displayString] hands out a pointer-unique string per clipping. That is wrong for short strings: a display string is the first line truncated to displayLen, and short strings are tagged pointers, so two different clippings whose first lines happen to match compare pointer-equal. A test program built against FlycutClipping shows "test\nZeile A" and "test\nZeile B" both yielding 0x802e93b4acce3b26. The consequence was the very bug this PR is about, only rarer: with the hint position invalidated by a store change, resolution could land on the wrong twin, and the hint check itself could accept a position that a same-looking clipping had moved into. So carry the FlycutClipping object instead. Object pointers are exact, the comparison is cheaper than a string compare, and a contents comparison remains as a fallback for a store reloaded from disk, where the objects are new but two clippings with equal contents are interchangeable for pasting anyway. This needs one read-only accessor on FlycutOperator, -clippingAtPosition:, forwarding to the store; the clipping is fetched before the NSMenuItem is allocated so a store change mid-build cannot leak an item. Verified with a test program covering: unchanged store, shifted store, a same-looking twin sitting at the hint position, a store reloaded from disk, and a deleted clipping (which must resolve to -1 and paste nothing). All six resolve to the clipping the user clicked. Co-Authored-By: Claude <noreply@anthropic.com>
The freeze added in the second commit stops -pollPB: while isMenuOpen or isSearchWindowDisplayed is set. Both flags can be left standing, and because capture then never resumes, Flycut silently stops recording clippings with nothing in the UI or the log to say why. Option-click on the status icon is the documented way to pause capture while copying a password (help.md, readme.md). It cancels the menu from inside -menuWillOpen:, and AppKit does not send -menuDidClose: for a menu cancelled there - verified with a standalone AppKit program: menuWillOpen fires, menuDidClose never does. So isMenuOpen stayed set forever. The first option-click was harmless because -pollPB: also requires the store to be enabled, but the second one re-enabled the store while capture stayed frozen. Cleared in that branch, where the menu is not opening anyway. -windowDidResignKey: routed every window to -hideApp, which does not close the search window properly, so isSearchWindowDisplayed survived clicking away from it. It now closes the search window through -hideSearchWindow. Before the freeze this only leaked a bit of state; with it, capture died. Both flags are additionally cleared in -hideApp as a backstop: hiding the app tears down the menu and the search window anyway, and neither flag is meant to outlive its surface. Co-Authored-By: Claude <noreply@anthropic.com>
While the bezel is open, -pollPB: keeps capturing, and -addClipping: reset the stack position to 0 on every insert. A clipping arriving mid-selection therefore moved the selection onto it, and the user pasted the newest entry instead of the one they had picked - the same defect this PR fixes for the status menu, on the third selection surface. The position now follows the clipping it points at: -addClipping: anchors on that clipping before changing the store and looks up where it ended up afterwards. -indexOfClipping: matches on contents, which is what is wanted here - two clippings with equal contents are interchangeable for pasting, and it also covers removeDuplicates, where the existing copy is moved to the top rather than a new one being inserted. The anchor is retained because the store can drop clippings while inserting. Position 0 is deliberately not anchored. It is where every selection starts, so it expresses no choice, and the bezel always shows whatever sits at the top; following the old top clipping from there would move the selection away from what the user is looking at. Staying put keeps display and paste in agreement. Because the bezel opening at the newest clipping used to be a side effect of that reset, -hitMainHotKey: now says so explicitly. Not in -showBezel:, which the favourites toggle also calls after -toggleToFromFavoritesStore has swapped in that store's own remembered position. Finally, the store redraws the bezel from inside its own insert, by way of -endUpdates, before the position has settled - so the bezel would show the neighbour of the clipping a paste would deliver. The clipping-added callback now redraws once the position is settled. No new state is introduced, so there is no new way for a flag to be left standing and stop capture, which is what the previous commit had to repair twice. Verified with a test program against FlycutStore/FlycutClipping: a clipping arriving mid-selection, two in a row, an untouched bezel at position 0 (also with a single stored clipping, and after navigating back up to 0), the anchor pushed out by the rememberNum limit, and removeDuplicates both with a duplicate above the selection and of the selection itself. Co-Authored-By: Claude <noreply@anthropic.com>
With pasteMovesToTop enabled, -getPasteFromIndex: moved the clipping and only then set the stack position to 0. The store redraws the bezel from inside that move (-delegateEndUpdates -> -[AppController endUpdates], which always has needBezelUpdate set via -noteChangeAtIndex:), so the redraw ran with the pre-move position - which after the move points at the neighbour of the clipping being pasted. Reported from a visual test: pressing Return on the third entry flashed the fourth one for an instant. The paste itself was correct. Assigning the position first fixes it, because once the clipping has been moved to the top, position 0 is that clipping. Reproduced and verified with a test program that stands in for the store delegate and records what the bezel would draw at -endUpdates: before, the redraw shows the neighbour; after, it shows the clipping that is pasted. Co-Authored-By: Claude <noreply@anthropic.com>
MiMoHo
force-pushed
the
menu-click-paste-race
branch
from
September 9, 2026 15:22
a4aafc6 to
edbbeac
Compare
This was referenced Sep 9, 2026
Updates haad#7. AI assistance: Hermes Agent, gpt-6-astra. Reasoning effort: not exposed by this session.
MiMoHo
changed the title
MiMoHo
marked this pull request as draft
September 9, 2026 21:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bind delayed paste to one owned, cancellable invocation rather than a mutable row index, a loose callback or an elapsed-time debounce.
FlycutPasteKey.has Resolve Paste using command-modified keyboard layouts #19. An unresolved key or event allocation failure posts nothing. Denied access in this targeted path requests native macOS consent and cancels without an app-owned prerequisite dialog.Related PRs and composition
Related: #9 and #16 cover older duplicate/escape strategies; this revision uses the owned transaction instead. #19 owns the standalone keyboard-layout hook and #15 owns the complete startup/manual permission flow. Those baseline helpers are not silently rewritten here. Do not treat independently green PRs as a verified combined tree. The frozen predecessor was measured to conflict with published #15 in
AppController.h/.mand #19 inAppController.m; this lifecycle repair does not resolve those overlaps or claim a combined-tree check. Reconcile both sides explicitly before integrating these PRs.Verification
Limits
Draft: this is source/native-fixture verification, not a newly installed editor-paste or multi-account/TCC result. No app bundle/resource build, installation, signing, real clipboard use or global input was performed for this revision.
NSPasteboardhas no atomic cross-process compare-and-swap, andCGEventPostToPiddoes not acknowledge clipboard consumption. The guards reject observed interference; they do not guarantee atomic delivery against every concurrent external copy. Promised clipboard representations can block and snapshots consume memory proportional to their contents. Failed rollback is logged, not reported as a successful restoration.AI assistance
Implemented and independently reviewed with Hermes Agent using gpt-6-astra. The session does not expose its reasoning-effort setting; no value is inferred.