Make NSOpenPanel Open/Cancel always end the modal session

Open with no row selected passed nil to -_setFilename:, and NSOpenPanel boxed it in an
array literal, so the action threw before the modal session was stopped and the panel
stayed up. Both buttons now go through one helper that stops the session only when
this panel is the modal window and always orders the panel out; Open accepts the
selection (filtered by canChooseFiles/canChooseDirectories and multiple-selection),
or the shown directory for directory choosers. Escape cancels.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

patches/cocotron.patch

@@ -11316,13 +11316,80 @@ index 00000000..19cd9ea9
 +
 +@end
 diff --git a/AppKit/NSSavePanel.subproj/NSSavePanel.m b/AppKit/NSSavePanel.subproj/NSSavePanel.m
-index 50c3d59e..5872e594 100644
+index 50c3d59e..be5a8463 100644
 --- a/AppKit/NSSavePanel.subproj/NSSavePanel.m
 +++ b/AppKit/NSSavePanel.subproj/NSSavePanel.m
-@@ -124,8 +124,48 @@ static NSSavePanel *_newPanel = nil;
-     [NSApp stopModalWithCode: NSCancelButton];
+@@ -113,19 +113,108 @@
+     }
  }
  
++/* Paths of the rows selected in the file list. Safe with no selection. */
++- (NSArray *) _selectedPaths {
++    NSMutableArray *paths = [NSMutableArray array];
++    NSIndexSet *rows = [_outlineView selectedRowIndexes];
++    for (NSUInteger row = [rows firstIndex]; row != NSNotFound;
++         row = [rows indexGreaterThanIndex: row]) {
++        id item = [_outlineView itemAtRow: row];
++        if ([item isKindOfClass: [NSURL class]] && [[item path] length] > 0)
++            [paths addObject: [item path]];
++    }
++    return paths;
++}
++
++/* What the Open/Save button accepts: the selected rows, or with nothing
++ * selected the directory being shown. An empty result keeps the panel up. */
++- (NSArray *) _acceptablePaths {
++    NSArray *selected = [self _selectedPaths];
++    if ([selected count] > 0)
++        return selected;
++    if ([_directory length] > 0)
++        return @[ _directory ];
++    return @[];
++}
++
++- (void) _setAcceptedPaths: (NSArray *) paths {
++    [self _setFilename: [paths firstObject]];
++}
++
++/* End the modal session this panel is running and take the panel off screen.
++ * The session is stopped only when this panel is the modal window, never
++ * another session that happens to be on top. The panel is hidden here as well
++ * so a dialog whose session cannot be stopped still goes away. */
++- (void) _dismissWithCode: (NSModalResponse) code {
++    if ([NSApp modalWindow] == self)
++        [NSApp stopModalWithCode: code];
++    [self orderOut: nil];
++}
++
+ - (IBAction) _selectFile: (id) sender {
+-    NSURL *url = [_outlineView itemAtRow: [_outlineView selectedRow]];
+-    [self _setFilename: [url path]];
+-
+-    [NSApp stopModalWithCode: NSOKButton];
++    NSArray *paths = [self _acceptablePaths];
++    if ([paths count] == 0)
++        return;
++    @try {
++        [self _setAcceptedPaths: paths];
++    } @catch (NSException *exception) {
++        NSLog(@"NSSavePanel: could not record the chosen file: %@", exception);
++        [self _dismissWithCode: NSCancelButton];
++        return;
++    }
++    [self _dismissWithCode: NSOKButton];
+ }
+ 
+ - (IBAction) _cancel: (id) sender {
+-    [NSApp stopModalWithCode: NSCancelButton];
+-}
+-
++    [self _dismissWithCode: NSCancelButton];
++}
++
++- (void) cancelOperation: (id) sender {
++    [self _cancel: sender];
++}
++
 +// Electron (Riot Client UI) opens its file dialogs through these and waits
 +// for the handler; run the panel modally and report the result.
  - (void) beginWithCompletionHandler: (void (^)(NSModalResponse result)) handler {
@@ -11368,6 +11435,57 @@ index 50c3d59e..5872e594 100644
 +    return NO;
  }
  
+ - (NSInteger) runModalForDirectory: (NSString *) directory
+diff --git a/AppKit/NSSavePanel.subproj/NSOpenPanel.m b/AppKit/NSSavePanel.subproj/NSOpenPanel.m
+index dfd33637..9388b186 100644
+--- a/AppKit/NSSavePanel.subproj/NSOpenPanel.m
++++ b/AppKit/NSSavePanel.subproj/NSOpenPanel.m
+@@ -23,6 +23,8 @@
+ 
+ @interface NSSavePanel (Private)
+ - (void) _setFilename: (NSString *) filename;
++- (NSArray *) _selectedPaths;
++- (void) _setAcceptedPaths: (NSArray *) paths;
+ @end
+ 
+ @implementation NSOpenPanel
+@@ -92,7 +94,36 @@
+ 
+ - (void) _setFilename: (NSString *) filename {
+     [super _setFilename: filename];
+-    _filenames = [@[ filename ] retain];
++    [self _setAcceptedPaths: [filename length] > 0 ? @[ filename ] : @[]];
++}
++
++- (void) _setAcceptedPaths: (NSArray *) paths {
++    if (!_allowsMultipleSelection && [paths count] > 1)
++        paths = @[ [paths firstObject] ];
++    @synchronized(self) {
++        NSArray *copy = [paths copy];
++        [_filenames release];
++        _filenames = copy;
++    }
++    [super _setFilename: [paths firstObject]];
++}
++
++/* Open only accepts what the panel was configured to choose. With nothing
++ * selected, a directory chooser accepts the directory being shown. */
++- (NSArray *) _acceptablePaths {
++    NSFileManager *manager = [NSFileManager defaultManager];
++    NSMutableArray *result = [NSMutableArray array];
++    for (NSString *path in [self _selectedPaths]) {
++        BOOL isDirectory = NO;
++        if (![manager fileExistsAtPath: path isDirectory: &isDirectory])
++            continue;
++        if (isDirectory ? _canChooseDirectories : _canChooseFiles)
++            [result addObject: path];
++    }
++    if ([result count] == 0 && [[self _selectedPaths] count] == 0 &&
++        _canChooseDirectories && [[self directory] length] > 0)
++        [result addObject: [self directory]];
++    return result;
+ }
+ 
  - (NSInteger) runModalForDirectory: (NSString *) directory
 diff --git a/AppKit/include/AppKit/NSSavePanel.h b/AppKit/include/AppKit/NSSavePanel.h
 index 53bacc46..3c6adfb7 100644

tests/test_nsopenpanel_close.py

@@ -0,0 +1,46 @@
+"""Check that the patched Cocotron file panels always end their modal session and never throw on Open."""
+import unittest
+import test_runtime_compatibility as sources
+
+
+def body(source, signature):
+    return source.split(signature + ' {', 1)[1].split('\n}', 1)[0]
+
+
+class FilePanelCloseChecks(unittest.TestCase):
+    patched = sources.CompatibilityChecks.patched
+
+    def save_panel(self):
+        return self.patched('cocotron', 'AppKit/NSSavePanel.subproj/NSSavePanel.m')
+
+    def open_panel(self):
+        return self.patched('cocotron', 'AppKit/NSSavePanel.subproj/NSOpenPanel.m')
+
+    def test_both_buttons_dismiss_through_one_helper(self):
+        source = self.save_panel()
+        dismiss = body(source, '- (void) _dismissWithCode: (NSModalResponse) code')
+        self.assertIn('[NSApp modalWindow] == self', dismiss)
+        self.assertIn('stopModalWithCode: code', dismiss)
+        self.assertIn('orderOut', dismiss)
+        self.assertIn('_dismissWithCode: NSCancelButton', body(source, '- (IBAction) _cancel: (id) sender'))
+        self.assertIn('_dismissWithCode: NSOKButton', body(source, '- (IBAction) _selectFile: (id) sender'))
+
+    def test_open_without_a_selection_cannot_throw(self):
+        # Nothing selected used to hand nil to -_setFilename:, and NSOpenPanel boxed it in @[ ... ].
+        self.assertNotIn('@[ filename ]];', self.open_panel().replace('[filename length] > 0 ? @[ filename ] : @[]', ''))
+        select = body(self.save_panel(), '- (IBAction) _selectFile: (id) sender')
+        self.assertNotIn('itemAtRow: [_outlineView selectedRow]', select)
+        self.assertIn('@catch', select)
+        self.assertIn('NSCancelButton', select.split('@catch', 1)[1])
+
+    def test_open_panel_honours_what_it_may_choose(self):
+        accept = body(self.open_panel(), '- (NSArray *) _acceptablePaths')
+        self.assertIn('_canChooseDirectories', accept)
+        self.assertIn('_canChooseFiles', accept)
+
+    def test_escape_cancels(self):
+        self.assertIn('[self _cancel: sender]', body(self.save_panel(), '- (void) cancelOperation: (id) sender'))
+
+
+if __name__ == '__main__':
+    unittest.main()