From 6955f7af5a2976cb77ba12c8f4937a19574e701f Mon Sep 17 00:00:00 2001 From: David Jenkins Date: Thu, 17 Oct 2024 15:37:18 -0500 Subject: [PATCH] Cocoa: Improve Undo/Redo handling for when COCOALOOPHIJACK is defined. If COCOALOOPHIJACK is defined, we override the default NSTextView/NSTextField undoManager and return a custom one that manages 'groupsByEvent' behavior manually. This appears to fix the overly broad grouping for most cases, though I have seen some oddities in testing, so there may be a hole related to either timing or interaction ordering. It does rely on overloading all of the undo manager's register* events, so it's more fragile than I'd like, and if the hijack behavior is ever removed this should be pulled as well. Reported to Lazarus as issue #36073 https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/36073 Our app is operating with COCOALOOPHIJACK --- lcl/interfaces/cocoa/cocoa_extra.pas | 9 ++ lcl/interfaces/cocoa/cocoatextedits.pas | 124 +++++++++++++++++++++++- 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/lcl/interfaces/cocoa/cocoa_extra.pas b/lcl/interfaces/cocoa/cocoa_extra.pas index d640377f08..c51ce9c65d 100644 --- a/lcl/interfaces/cocoa/cocoa_extra.pas +++ b/lcl/interfaces/cocoa/cocoa_extra.pas @@ -15,6 +15,7 @@ unit Cocoa_Extra; {$mode objfpc}{$H+} +{$modeswitch cblocks} {$modeswitch objectivec1} {$include cocoadefines.inc} @@ -627,6 +628,14 @@ type patchVersion: NSInteger; end; + NSUndoManagerUndoWithTargetCBlock = reference to procedure(target: id); cblock; cdecl; + + NSUndoManagerFix = objccategory external (NSUndoManager) + procedure registerUndoWithTarget_handler(target: id; + handler: NSUndoManagerUndoWithTargetCBlock); + message 'registerUndoWithTarget:handler:'; + end; + const // defined in NSApplication.h NSAppKitVersionNumber10_5 = 949; diff --git a/lcl/interfaces/cocoa/cocoatextedits.pas b/lcl/interfaces/cocoa/cocoatextedits.pas index 9609c81314..39e1d322b1 100644 --- a/lcl/interfaces/cocoa/cocoatextedits.pas +++ b/lcl/interfaces/cocoa/cocoatextedits.pas @@ -23,6 +23,9 @@ unit CocoaTextEdits; {.$DEFINE COCOA_DEBUG_SETBOUNDS} {.$DEFINE COCOA_SPIN_DEBUG} {.$DEFINE COCOA_SPINEDIT_INSIDE_CONTAINER} +{$IFDEF COCOALOOPHIJACK} + {$DEFINE COCOA_OVERRIDE_UNDOMANAGER} +{$ENDIF} interface @@ -31,7 +34,7 @@ uses Math, // needed for MinDouble, MaxDouble LCLType, MacOSAll, CocoaAll, CocoaConfig, CocoaUtils, CocoaGDIObjects, - CocoaPrivate, CocoaCallback; + CocoaPrivate, CocoaCallback, Cocoa_Extra; const SPINEDIT_DEFAULT_STEPPER_WIDTH = 15; @@ -62,7 +65,10 @@ type callback: ICommonCallback; maxLength: Integer; fixedInitSetting: Boolean; - + {$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + FUndoManager: NSUndoManager; + procedure dealloc; override; + {$ENDIF} function acceptsFirstResponder: LCLObjCBoolean; override; function lclGetCallback: ICommonCallback; override; procedure lclClearCallback; override; @@ -81,6 +87,10 @@ type procedure scrollWheel(event: NSEvent); override; procedure lclSetMaxLength(amax: integer); + {$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + function undoManagerForTextView(view: NSTextView): NSUndoManager; message 'undoManagerForTextView:'; + {$ENDIF} + end; { TCocoaSecureTextField } @@ -89,6 +99,10 @@ type public maxLength: Integer; callback: ICommonCallback; + {$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + FUndoManager: NSUndoManager; + procedure dealloc; override; + {$ENDIF} function acceptsFirstResponder: LCLObjCBoolean; override; function lclGetCallback: ICommonCallback; override; procedure lclClearCallback; override; @@ -106,6 +120,9 @@ type procedure scrollWheel(event: NSEvent); override; procedure lclSetMaxLength(amax: integer); + {$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + function undoManagerForTextView(view: NSTextView): NSUndoManager; message 'undoManagerForTextView:'; + {$ENDIF} end; { TCocoaTextView } @@ -444,6 +461,20 @@ type end; {$ENDIF} + { TCocoaUndoManager } +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + TCocoaUndoManager = objcclass(NSUndoManager) + lastEvent: NSEvent; // weak reference + function init: id; override; + procedure undo; override; + procedure registerUndoWithTarget_selector_object(target: id; selector: SEL; + anObject: id); override; + procedure registerUndoWithTarget_handler(target: id; + handler: NSUndoManagerUndoWithTargetCBlock); override; + procedure lclCheckGrouping; message 'lclCheckGrouping'; + end; +{$ENDIF} + // these constants are missing from CocoaAll for some reason const NSTextAlignmentLeft = 0; @@ -976,6 +1007,15 @@ end; { TCocoaTextField } +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} +procedure TCocoaTextField.dealloc; +begin + if Assigned(FUndoManager) then + FUndoManager.release; + inherited dealloc; +end; +{$ENDIF} + function TCocoaTextField.acceptsFirstResponder: LCLObjCBoolean; begin Result := NSViewCanFocus(Self); @@ -1083,6 +1123,15 @@ begin maxLength := amax; end; +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} +function TCocoaTextField.undoManagerForTextView(view: NSTextView): NSUndoManager; +begin + if not Assigned(FUndoManager) then + FUndoManager := TCocoaUndoManager.alloc.init; + Result := FUndoManager; +end; +{$ENDIF} + { TCocoaTextView } procedure TCocoaTextView.changeColor(sender: id); @@ -1237,12 +1286,25 @@ end; function TCocoaTextView.undoManagerForTextView(view: NSTextView): NSUndoManager; begin if not Assigned(FUndoManager) then + {$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + FUndoManager := TCocoaUndoManager.alloc.init; + {$ELSE} FUndoManager := NSUndoManager.alloc.init; + {$ENDIF} Result := FUndoManager; end; { TCocoaSecureTextField } +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} +procedure TCocoaSecureTextField.dealloc; +begin + if Assigned(FUndoManager) then + FUndoManager.release; + inherited dealloc; +end; +{$ENDIF} + function TCocoaSecureTextField.acceptsFirstResponder: LCLObjCBoolean; begin Result := NSViewCanFocus(Self); @@ -1331,6 +1393,15 @@ begin MaxLength := amax; end; +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} +function TCocoaSecureTextField.undoManagerForTextView(view: NSTextView): NSUndoManager; +begin + if not Assigned(FUndoManager) then + FUndoManager := TCocoaUndoManager.alloc.init; + Result := FUndoManager; +end; +{$ENDIF} + { TCocoaEditComboBoxList } procedure TCocoaEditComboBoxList.InsertItem(Index: Integer; const S: string; @@ -2335,5 +2406,54 @@ end; {$ENDIF} +{$IFDEF COCOA_OVERRIDE_UNDOMANAGER} + +{ TCocoaUndoManager } + +function TCocoaUndoManager.init: id; +begin + // This manages top-level undo groups automatically to work around an issue + // where, if we hijack the run loop, all undoable actions are combined into a + // single undo group. It isn't necessary for correct behavior in the other + // modes. + Result := inherited init; + Result.setGroupsByEvent(False); +end; + +procedure TCocoaUndoManager.undo; +begin + if not groupsByEvent and (groupingLevel = 1) then + endUndoGrouping; + inherited; +end; + +procedure TCocoaUndoManager.registerUndoWithTarget_selector_object(target: id; + selector: SEL; anObject: id); +begin + lclCheckGrouping; + inherited; +end; + +procedure TCocoaUndoManager.registerUndoWithTarget_handler(target: id; + handler: NSUndoManagerUndoWithTargetCBlock); +begin + lclCheckGrouping; + inherited registerUndoWithTarget_handler(target, handler); +end; + +procedure TCocoaUndoManager.lclCheckGrouping; +begin + if groupsByEvent or isUndoing or isRedoing then + Exit; + if (groupingLevel = 1) and (lastEvent <> NSApp.currentEvent) then + endUndoGrouping; + if groupingLevel = 0 then begin + lastEvent := NSApp.currentEvent; + beginUndoGrouping; + end; +end; + +{$ENDIF} + end.