Cocoa: Fix TEdit/TMemo Undo/Redo
Original Reporter info from Mantis: zpeterson @boramis
-
Reporter name: Zoë Peterson
Original Reporter info from Mantis: zpeterson @boramis
- Reporter name: Zoë Peterson
Description:
This fixes various issues with Undo/Redo in memos and edits and replaces some of the work from the patch in issue 35421. It's split into two because the second patch is riskier.
Patch 1:
* Changes the way TCocoaTextView (TMemo) returns its undoManager from overriding the message to using the delegate method. This doesn't affect stock LCL much, since NSResponderHotKeys works either way, but the old code doesn't work with NSMenu's validation protocol, so a auto-enabled menu item with an undo:/redo: action (instead of actionButtonClick:) won't enable properly or update it's caption (visible in the sample project).
- Undo stack is now cleared whenever we set the textview contents programmatically. This fixes two issues:
1) Setting the initial value for a memo was treated as an undoable edit.
2) If you make an edit, modify the text programmatically, then try to undo, the app crashes with a Cocoa assertion failure. removeAllActionsWithTarget(text) doesn't clear it properly, which is why removeAllActions is used.
Patch 2:
With only patch 1 applied, TEdit/TMemo undo/redo is still essentially unusable with the stock LCL. NSUndoManager has a property, groupsByEvent, that makes it so multiple edits within a single NSEvent are combined into a single undoable action. Making the same type of edit multiple times (e.g., typing multiple characters) is also combined into a single edit. Making different types of edits (type, cut, paste) should remain distinct undoable actions. With the current LCL, that doesn't work; every edit is combined into a single undo group, so trying to undo one thing clears the edit back to its initial state.
The problem is that the way the runloop is handled when COCOALOOPHIJACK is defined breaks the groupsByEvent behavior. Changing from COCOALOOPHIJACK to COCOALOOPNATIVE solves the problem, but at least in our app introduces various other issues.
This patch fixes the issue by returning a custom undo manager that manages the groupsByEvent behavior manually. Whenever a new undoable item is registered we start a new undo group, and close it out whenever undo is triggered.
Since we have to capture all undo entries, this also needs to override registerUndoWithTarget:handler which was introduced in macOS 10.11. It is susceptible to breaking if other register* functions are added in the future, so making COCOALOOPNATIVE work properly is probably the best long-term fix.
Patch 2 also has the side effect that all NSTextFields (TEdit) have a private undoManager now too, so they no longer clear the undo stack when gaining/losing focus.
If COCOALOOPNATIVE isn't defined, all of the new behavior is $IFDEFed out.
Steps to reproduce:
The attached sample project has 2 edits, 2 memos, and a raw NSTextView in the lower left corner for comparison purposes.
The "Edit" menu includes all of the standard actions (Undo/Redo/Cut/Copy/Paste/Select All) and uses NSMenu's auto-enable support to update itself rather than relying on the LCL layer. As you make edits and change focus you can see the menu items enabling automatically. When the raw NSTextView has focus, Undo/Redo are enabled appropriately and their captions reflect the state of the edit ("Undo Typing", "Redo Paste", etc). When one of the TMemos has focus, Undo/Redo stay permanently disabled.
The "Clear" button sets all 5 edits to empty. If you make an edit in one, click that button, then go back in and press Cmd+Z, the app will crash.
Mantis conversion info:
- Mantis ID: 36073
- Version: 2.0.4