Skip to content

Commit 80df660

Browse files
authored
fix: file-only picker incorrectly allowing some directories (electron#48198)
1 parent 8225af4 commit 80df660

2 files changed

Lines changed: 109 additions & 0 deletions

File tree

patches/chromium/.patches

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,4 @@ fix_resolve_dynamic_background_material_update_issue_on_windows_11.patch
138138
feat_add_support_for_embedder_snapshot_validation.patch
139139
chore_restore_some_deprecated_wrapper_utility_in_gin.patch
140140
chore_add_electron_objects_to_wrappablepointertag.patch
141+
band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Avi Drissman <avi@chromium.org>
3+
Date: Thu, 21 Aug 2025 07:33:53 -0700
4+
Subject: Band-aid over an issue with using deprecated NSOpenPanel API
5+
6+
Because deprecated and broken NSOpenPanel API is used, the open panel
7+
will sometimes incorrectly misunderstand a folder to be a package and
8+
return it as a user selection when folders are disallowed from
9+
selection. In that case, skip it.
10+
11+
Bug: 40861123
12+
Bug: 41275486
13+
Bug: 440106155
14+
Change-Id: Ia0459a2bb76a30f4e126bd83069d7e13894d62f6
15+
Fixed: 438779953
16+
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6867298
17+
Commit-Queue: Avi Drissman <avi@chromium.org>
18+
Reviewed-by: Christine Hollingsworth <christinesm@chromium.org>
19+
Cr-Commit-Position: refs/heads/main@{#1504534}
20+
21+
diff --git a/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm b/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm
22+
index f0b8108a7f8a63f66664c6c5ad3ada0bf60805b3..67380a76c699d1c2db0d3a96671bb92657c4a6d3 100644
23+
--- a/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm
24+
+++ b/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm
25+
@@ -225,7 +225,7 @@ - (void)popupAction:(id)sender {
26+
// Unfortunately, there's no great way to do strict type matching with
27+
// NSOpenPanel. Setting explicit extensions via -allowedFileTypes is
28+
// deprecated, and there's no way to specify that strict type equality should
29+
- // be used for -allowedContentTypes (FB13721802).
30+
+ // be used for -allowedContentTypes (https://crbug.com/41275486, FB13721802).
31+
//
32+
// -[NSOpenSavePanelDelegate panel:shouldEnableURL:] could be used to enforce
33+
// strict type matching, however its presence on the delegate means that all
34+
@@ -235,6 +235,10 @@ - (void)popupAction:(id)sender {
35+
//
36+
// Therefore, use the deprecated API, because it's the only way to remain
37+
// performant while achieving strict type matching.
38+
+ //
39+
+ // TODO(https://crbug.com/440106155): Possibly reconsider using
40+
+ // -panel:shouldEnableURL: if the speed impact is judged to be acceptable
41+
+ // nowadays.
42+
43+
#pragma clang diagnostic push
44+
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
45+
@@ -479,8 +483,8 @@ - (void)popupAction:(id)sender {
46+
47+
// See -[ExtensionDropdownHandler popupAction:] as to why file extensions
48+
// are collected here rather than being converted to UTTypes.
49+
- // TODO(FB13721802): Use UTTypes when strict type matching can be
50+
- // specified.
51+
+ // TODO(https://crbug.com/440106155, FB13721802): Use UTTypes when strict
52+
+ // type matching can be specified.
53+
NSString* ext_ns = base::SysUTF8ToNSString(ext);
54+
if (![file_extensions_array containsObject:ext_ns]) {
55+
[file_extensions_array addObject:ext_ns];
56+
@@ -571,18 +575,46 @@ - (void)popupAction:(id)sender {
57+
}
58+
NSString* path = url.path;
59+
60+
- // There is a bug in macOS where, despite a request to disallow file
61+
- // selection, files/packages are able to be selected. If indeed file
62+
- // selection was disallowed, drop any files selected.
63+
- // https://crbug.com/40861123, FB11405008
64+
- if (!open_panel.canChooseFiles) {
65+
+ if (base::mac::MacOSMajorVersion() < 14) {
66+
+ // There is a bug in macOS (https://crbug.com/40861123, FB11405008)
67+
+ // where, despite a request to disallow file selection, files/packages
68+
+ // are able to be selected. If indeed file selection was disallowed,
69+
+ // drop any files selected. This issue is fixed in macOS 14, so only
70+
+ // do the workaround on previous releases.
71+
+ if (!open_panel.canChooseFiles) {
72+
+ BOOL is_directory;
73+
+ BOOL exists =
74+
+ [NSFileManager.defaultManager fileExistsAtPath:path
75+
+ isDirectory:&is_directory];
76+
+ BOOL is_package =
77+
+ [NSWorkspace.sharedWorkspace isFilePackageAtPath:path];
78+
+ if (!exists || !is_directory || is_package) {
79+
+ continue;
80+
+ }
81+
+ }
82+
+ }
83+
+
84+
+ // As long as FB13721802 remains unfixed, this class uses extensions to
85+
+ // filter what files are available rather than UTTypes. This deprecated
86+
+ // API has a problem, however. If you specify an extension to be shown
87+
+ // as available, then the NSOpenPanel will assume that any directory
88+
+ // that has that extension is a package, and will offer it to the user
89+
+ // for selection even if directory selection isn't otherwise allowed.
90+
+ // Therefore, if directories are disallowed, filter out any that find
91+
+ // their way in if they're not actually packages.
92+
+ //
93+
+ // TODO(https://crbug.com/440106155, FB13721802): Possibly reconsider
94+
+ // using -panel:shouldEnableURL: if the speed impact is judged to be
95+
+ // acceptable nowadays, and drop this band-aid.
96+
+ if (!open_panel.canChooseDirectories) {
97+
BOOL is_directory;
98+
BOOL exists =
99+
[NSFileManager.defaultManager fileExistsAtPath:path
100+
isDirectory:&is_directory];
101+
BOOL is_package =
102+
[NSWorkspace.sharedWorkspace isFilePackageAtPath:path];
103+
- if (!exists || !is_directory || is_package) {
104+
+ if (!exists || (is_directory && !is_package)) {
105+
+ NSLog(@"dropping %@", path);
106+
continue;
107+
}
108+
}

0 commit comments

Comments
 (0)