From 7379d2b0a549c98d33bf03b451c5320738295094 Mon Sep 17 00:00:00 2001 From: Avi Drissman Date: Tue, 16 Apr 2024 16:39:31 +0000 Subject: [PATCH] Revert "Filter files in the file picker for strict type conformance." This reverts commit 9b3c873bd5edd4e364ff8e2c05b96ee953d0a5a6. Reason for revert: https://crbug.com/335171119 Original change's description: > Filter files in the file picker for strict type conformance. > > Fixed: 41275486 > Change-Id: I77605e1b0727afdb1ba8e4c3dc54c7f3d89920b8 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5448139 > Reviewed-by: Leonard Grey > Auto-Submit: Avi Drissman > Commit-Queue: Avi Drissman > Commit-Queue: Leonard Grey > Cr-Commit-Position: refs/heads/main@{#1286553} Change-Id: I2d7cbe4da4f13f578441d06a1e8d329a3c89996d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5457774 Reviewed-by: Prudhvikumar Bommana Commit-Queue: Prudhvikumar Bommana Owners-Override: Prudhvikumar Bommana Cr-Commit-Position: refs/branch-heads/6420@{#9} Cr-Branched-From: df576e0dad8b00dea2749a08f0849c518fa28949-refs/heads/main@{#1287100} --- .../app_shim/select_file_dialog_bridge.mm | 75 ++++++------------- 1 file changed, 24 insertions(+), 51 deletions(-) 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 306ae9656a4c0c..a1a6a83779a9e9 100644 --- a/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm +++ b/components/remote_cocoa/app_shim/select_file_dialog_bridge.mm @@ -28,6 +28,14 @@ const int kFileTypePopupTag = 1234; +// TODO(macOS 11): Remove this. +CFStringRef CreateUTIFromExtension(const base::FilePath::StringType& ext) { + base::apple::ScopedCFTypeRef ext_cf( + base::SysUTF8ToCFStringRef(ext)); + return UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension, + ext_cf.get(), nullptr); +} + NSString* GetDescriptionFromExtension(const base::FilePath::StringType& ext) { if (@available(macOS 11, *)) { UTType* type = @@ -38,12 +46,7 @@ return description; } } else { - base::apple::ScopedCFTypeRef ext_cf = - base::SysUTF8ToCFStringRef(ext); - base::apple::ScopedCFTypeRef uti( - UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension, - ext_cf.get(), - /*inConformingToUTI=*/nullptr)); + base::apple::ScopedCFTypeRef uti(CreateUTIFromExtension(ext)); NSString* description = base::apple::CFToNSOwnershipCast(UTTypeCopyDescription(uti.get())); @@ -216,47 +219,6 @@ - (BOOL)panel:(id)sender validateURL:(NSURL*)url error:(NSError**)outError { return YES; } -- (BOOL)panel:(id)sender shouldEnableURL:(NSURL*)url { - // When provided UTTypes, NSOpenPanel determines whether files are selectable - // by conformance, not by strict type equality. For example, with - // public.plain-text, not only .txt files will be selectable, but .js, .m3u, - // and .csv files will be as well. With public.zip-archive, not only .zip - // files will be selectable, but also .jar files and .xlsb files. - // - // While this can be great for normal viewing/editing apps, this is not - // desirable for Chromium, where the web platform requires strict type - // matching on the provided extensions, and files that have a conforming file - // type by accident of their implementation shouldn't qualify for selection. - // - // 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). Therefore, as inefficient as - // it is, this use of the delegate is required. - - if (@available(macOS 11, *)) { - NSArray* allowedTypes = [sender allowedContentTypes]; - if (!allowedTypes.count) { - // An empty array is "all types"; - return YES; - } - - UTType* type; - if (![url getResourceValue:&type forKey:NSURLContentTypeKey error:nil]) { - return NO; - } - - return [allowedTypes containsObject:type]; - } else { - // It is not documented, but -panel:shouldEnableURL: is a filter. Files for - // which NO is returned are disabled, but files for which YES is returned - // are further processed and filtered by the specified allowedFileTypes. - // Return YES and allow the further filtering by the file types already - // specified. - return YES; - } -} - @end @implementation ExtensionDropdownHandler @@ -528,10 +490,21 @@ - (void)popupAction:(id)sender { [file_uttype_array addObject:type]; } } else { - // Do not include any UTIs in this file type list. UTIs, if included, - // are compared with conformance, while strict type equality is - // required. See the -panel:shouldEnableURL: delegate callback above for - // more information. + // Crash reports suggest that CreateUTIFromExtension may return nil. + // Hence we nil check before adding to |file_type_set|. See + // crbug.com/630101 and rdar://27490414. + NSString* uti = + base::apple::CFToNSOwnershipCast(CreateUTIFromExtension(ext)); + if (uti) { + if (![file_type_array containsObject:uti]) { + [file_type_array addObject:uti]; + } + } + + // Always allow the extension itself, in case the UTI doesn't map + // back to the original extension correctly. This occurs with dynamic + // UTIs on 10.7 and 10.8. + // See https://crbug.com/148840, https://openradar.appspot.com/12316273 NSString* ext_ns = base::SysUTF8ToNSString(ext); if (![file_type_array containsObject:ext_ns]) { [file_type_array addObject:ext_ns];