From dcf18e23f6b1406fd4210016b51aac1953d76ca4 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Mon, 8 Sep 2025 09:51:45 +0200 Subject: [PATCH] fix: file-only picker incorrectly allowing some directories (#48229) * fix: file-only picker incorrectly allowing some directories Co-authored-by: Shelley Vohr * chore: update patches --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr Co-authored-by: John Kleinschmidt --- patches/chromium/.patches | 1 + ...ith_using_deprecated_nsopenpanel_api.patch | 108 ++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 patches/chromium/band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index a4f7394d6710..f6c10444b1ca 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -150,3 +150,4 @@ make_focus_methods_in_webcontentsviewchildframe_notimplemented.patch cherry-pick-f1e6422a355c.patch fix_resolve_dynamic_background_material_update_issue_on_windows_11.patch feat_add_support_for_embedder_snapshot_validation.patch +band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch diff --git a/patches/chromium/band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch b/patches/chromium/band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch new file mode 100644 index 000000000000..f7b44b9a75b4 --- /dev/null +++ b/patches/chromium/band-aid_over_an_issue_with_using_deprecated_nsopenpanel_api.patch @@ -0,0 +1,108 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Avi Drissman +Date: Thu, 21 Aug 2025 07:33:53 -0700 +Subject: Band-aid over an issue with using deprecated NSOpenPanel API + +Because deprecated and broken NSOpenPanel API is used, the open panel +will sometimes incorrectly misunderstand a folder to be a package and +return it as a user selection when folders are disallowed from +selection. In that case, skip it. + +Bug: 40861123 +Bug: 41275486 +Bug: 440106155 +Change-Id: Ia0459a2bb76a30f4e126bd83069d7e13894d62f6 +Fixed: 438779953 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6867298 +Commit-Queue: Avi Drissman +Reviewed-by: Christine Hollingsworth +Cr-Commit-Position: refs/heads/main@{#1504534} + +diff --git a/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm b/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm +index 8e5fcc911dd28b99786ae2708184a7c47d3b1c3c..1f9977ffb1d7fd5d8bcc22f6f1ca2b840af909d6 100644 +--- a/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm ++++ b/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm +@@ -220,7 +220,7 @@ - (void)popupAction:(id)sender { + // Unfortunately, there's no great way to do strict type matching with + // NSOpenPanel. Setting explicit extensions via -allowedFileTypes is + // deprecated, and there's no way to specify that strict type equality should +- // be used for -allowedContentTypes (FB13721802). ++ // be used for -allowedContentTypes (https://crbug.com/41275486, FB13721802). + // + // -[NSOpenSavePanelDelegate panel:shouldEnableURL:] could be used to enforce + // strict type matching, however its presence on the delegate means that all +@@ -230,6 +230,10 @@ - (void)popupAction:(id)sender { + // + // Therefore, use the deprecated API, because it's the only way to remain + // performant while achieving strict type matching. ++ // ++ // TODO(https://crbug.com/440106155): Possibly reconsider using ++ // -panel:shouldEnableURL: if the speed impact is judged to be acceptable ++ // nowadays. + + #pragma clang diagnostic push + #pragma clang diagnostic ignored "-Wdeprecated-declarations" +@@ -462,8 +466,8 @@ - (void)popupAction:(id)sender { + + // See -[ExtensionDropdownHandler popupAction:] as to why file extensions + // are collected here rather than being converted to UTTypes. +- // TODO(FB13721802): Use UTTypes when strict type matching can be +- // specified. ++ // TODO(https://crbug.com/440106155, FB13721802): Use UTTypes when strict ++ // type matching can be specified. + NSString* ext_ns = base::SysUTF8ToNSString(ext); + if (![file_extensions_array containsObject:ext_ns]) { + [file_extensions_array addObject:ext_ns]; +@@ -554,18 +558,46 @@ - (void)popupAction:(id)sender { + } + NSString* path = url.path; + +- // There is a bug in macOS where, despite a request to disallow file +- // selection, files/packages are able to be selected. If indeed file +- // selection was disallowed, drop any files selected. +- // https://crbug.com/40861123, FB11405008 +- if (!open_panel.canChooseFiles) { ++ if (base::mac::MacOSMajorVersion() < 14) { ++ // There is a bug in macOS (https://crbug.com/40861123, FB11405008) ++ // where, despite a request to disallow file selection, files/packages ++ // are able to be selected. If indeed file selection was disallowed, ++ // drop any files selected. This issue is fixed in macOS 14, so only ++ // do the workaround on previous releases. ++ if (!open_panel.canChooseFiles) { ++ BOOL is_directory; ++ BOOL exists = ++ [NSFileManager.defaultManager fileExistsAtPath:path ++ isDirectory:&is_directory]; ++ BOOL is_package = ++ [NSWorkspace.sharedWorkspace isFilePackageAtPath:path]; ++ if (!exists || !is_directory || is_package) { ++ continue; ++ } ++ } ++ } ++ ++ // As long as FB13721802 remains unfixed, this class uses extensions to ++ // filter what files are available rather than UTTypes. This deprecated ++ // API has a problem, however. If you specify an extension to be shown ++ // as available, then the NSOpenPanel will assume that any directory ++ // that has that extension is a package, and will offer it to the user ++ // for selection even if directory selection isn't otherwise allowed. ++ // Therefore, if directories are disallowed, filter out any that find ++ // their way in if they're not actually packages. ++ // ++ // TODO(https://crbug.com/440106155, FB13721802): Possibly reconsider ++ // using -panel:shouldEnableURL: if the speed impact is judged to be ++ // acceptable nowadays, and drop this band-aid. ++ if (!open_panel.canChooseDirectories) { + BOOL is_directory; + BOOL exists = + [NSFileManager.defaultManager fileExistsAtPath:path + isDirectory:&is_directory]; + BOOL is_package = + [NSWorkspace.sharedWorkspace isFilePackageAtPath:path]; +- if (!exists || !is_directory || is_package) { ++ if (!exists || (is_directory && !is_package)) { ++ NSLog(@"dropping %@", path); + continue; + } + }