Skip to content

Commit

Permalink
Revert "Filter files in the file picker for strict type conformance."
Browse files Browse the repository at this point in the history
This reverts commit 9b3c873.

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 <lgrey@chromium.org>
> Auto-Submit: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Commit-Queue: Leonard Grey <lgrey@chromium.org>
> 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 <pbommana@google.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/6420@{#9}
Cr-Branched-From: df576e0-refs/heads/main@{#1287100}
  • Loading branch information
Avi Drissman authored and Chromium LUCI CQ committed Apr 16, 2024
1 parent c702359 commit 7379d2b
Showing 1 changed file with 24 additions and 51 deletions.
75 changes: 24 additions & 51 deletions components/remote_cocoa/app_shim/select_file_dialog_bridge.mm
Expand Up @@ -28,6 +28,14 @@

const int kFileTypePopupTag = 1234;

// TODO(macOS 11): Remove this.
CFStringRef CreateUTIFromExtension(const base::FilePath::StringType& ext) {
base::apple::ScopedCFTypeRef<CFStringRef> 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 =
Expand All @@ -38,12 +46,7 @@
return description;
}
} else {
base::apple::ScopedCFTypeRef<CFStringRef> ext_cf =
base::SysUTF8ToCFStringRef(ext);
base::apple::ScopedCFTypeRef<CFStringRef> uti(
UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension,
ext_cf.get(),
/*inConformingToUTI=*/nullptr));
base::apple::ScopedCFTypeRef<CFStringRef> uti(CreateUTIFromExtension(ext));
NSString* description =
base::apple::CFToNSOwnershipCast(UTTypeCopyDescription(uti.get()));

Expand Down Expand Up @@ -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<UTType*>* 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
Expand Down Expand Up @@ -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];
Expand Down

0 comments on commit 7379d2b

Please sign in to comment.