From f007af506fa820df0589c234ee094fb4709e7c13 Mon Sep 17 00:00:00 2001 From: Milan Hoppe Date: Sun, 5 Jul 2026 02:29:51 +0200 Subject: [PATCH] Fix menu click sometimes pasting the newest clipping instead of the clicked 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 --- AppController.m | 60 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/AppController.m b/AppController.m index 3898ab0..cd98ccd 100755 --- a/AppController.m +++ b/AppController.m @@ -906,21 +906,25 @@ - (void)moveItemAtStackPositionToTopOfStack } } -- (void)pasteIndexAndUpdate:(int) position { +// Returns true if a clipping was found and placed on the pasteboard. +- (bool)pasteIndexAndUpdate:(int) position { // If there is an active search, we need to map the menu index to the stack position. NSString* search = [searchBox stringValue]; if ( nil != search && 0 != search.length ) { NSArray *mapping = [flycutOperator previousIndexes:[[NSUserDefaults standardUserDefaults] integerForKey:@"displayNum"] containing:search]; + if ( position < 0 || position >= (int)[mapping count] ) + return false; // The list changed since the menu was built. position = [mapping[position] intValue]; } NSString *content = [flycutOperator getPasteFromIndex: position]; - if ( nil != content ) - { - [self addClipToPasteboard:content]; - [self updateMenu]; - } + if ( nil == content ) + return false; + + [self addClipToPasteboard:content]; + [self updateMenu]; + return true; } - (void)metaKeysReleased @@ -1536,8 +1540,22 @@ - (void)updateMenuContaining:(NSString*)search { -(IBAction)processMenuClippingSelection:(id)sender { - int index=[[sender menu] indexOfItem:sender]; - [self pasteIndexAndUpdate:index]; + // pollPB: runs in NSRunLoopCommonModes, so a clipboard change noticed while + // the menu is open triggers updateMenu, which replaces every clipping item. + // If that rebuild lands between the click and this action, the clicked item + // is no longer in the menu and [sender menu] is nil, so the previous + // [[sender menu] indexOfItem:sender] messaged nil and yielded index 0 -- + // pasting the most recent clipping instead of the one the user clicked. + // Bail out instead of pasting the wrong clipping. + NSMenu *senderMenu = [sender menu]; + if ( nil == senderMenu ) + return; + NSInteger index = [senderMenu indexOfItem:sender]; + if ( index < 0 ) + return; + + if ( ! [self pasteIndexAndUpdate:(int)index] ) + return; // Nothing was placed on the pasteboard, so don't fake a Cmd-V. if ( [[NSUserDefaults standardUserDefaults] boolForKey:@"menuSelectionPastes"] ) { [self performSelector:@selector(hideApp) withObject:nil]; @@ -1894,19 +1912,35 @@ - (void)updateSearchResults - (IBAction)searchWindowItemSelected:(id)sender { - NSInteger selectedRow = [searchWindowTableView selectedRow]; - if (selectedRow < 0) { - selectedRow = 0; // Default to first item if none selected + NSInteger selectedRow; + if (sender == searchWindowTableView) { + // Invoked by double-click: use the row that was actually clicked. The + // selection can change between the click and this action (for example + // updateSearchResults re-selects row 0 when the search field action + // fires), which pasted the newest entry instead of the clicked one. + selectedRow = [searchWindowTableView clickedRow]; + if (selectedRow < 0) { + return; // Double-click below the last row. + } + } else { + // Invoked by Enter in the search field. + selectedRow = [searchWindowTableView selectedRow]; + if (selectedRow < 0) { + selectedRow = 0; // Default to first item if none selected + } } - + if (selectedRow < [searchResults count]) { // Get the content and paste it like bezel does NSString* searchText = [searchWindowSearchField stringValue]; NSArray *mapping = nil; int position = (int)selectedRow; - + if (searchText && [searchText length] > 0) { mapping = [flycutOperator previousIndexes:[[NSUserDefaults standardUserDefaults] integerForKey:@"displayNum"] containing:searchText]; + if (selectedRow >= (NSInteger)[mapping count]) { + return; // The store changed since the results were built. + } position = [mapping[selectedRow] intValue]; }