Commit 5bf1c6c2 authored by Marijn Kruisselbrink's avatar Marijn Kruisselbrink Committed by Chromium LUCI CQ

Ensure that showSaveFilePicker always shows the extension on Mac.

While not a very strong security boundary, making it possible for
users to know what extension a file will be saved with is a good idea.

This also fixes support for compound extensions with the File System
Access API (i.e. ".tar.gz"). The mac file dialog already had a
workaround if the default path ended in such an extension, but the
same problem occurs if the file type filters include a type with a
compound extension.

Bug: 1137247
Change-Id: I492bf36baced3de044b8fed5d57fc7b9b5b64400
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582842Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835803}
parent d3772e27
...@@ -206,6 +206,8 @@ void SelectFileDialogBridge::Show( ...@@ -206,6 +206,8 @@ void SelectFileDialogBridge::Show(
} }
} }
const bool keep_extension_visible =
file_types ? file_types->keep_extension_visible : false;
if (type_ != SelectFileDialogType::kFolder && if (type_ != SelectFileDialogType::kFolder &&
type_ != SelectFileDialogType::kUploadFolder && type_ != SelectFileDialogType::kUploadFolder &&
type_ != SelectFileDialogType::kExistingFolder) { type_ != SelectFileDialogType::kExistingFolder) {
...@@ -227,7 +229,7 @@ void SelectFileDialogBridge::Show( ...@@ -227,7 +229,7 @@ void SelectFileDialogBridge::Show(
// this by never hiding extensions in that case. // this by never hiding extensions in that case.
base::FilePath::StringType penultimate_extension = base::FilePath::StringType penultimate_extension =
default_path.RemoveFinalExtension().FinalExtension(); default_path.RemoveFinalExtension().FinalExtension();
if (!penultimate_extension.empty()) { if (!penultimate_extension.empty() || keep_extension_visible) {
[dialog setExtensionHidden:NO]; [dialog setExtensionHidden:NO];
} else { } else {
[dialog setExtensionHidden:YES]; [dialog setExtensionHidden:YES];
......
...@@ -28,6 +28,9 @@ struct SelectFileTypeInfo { ...@@ -28,6 +28,9 @@ struct SelectFileTypeInfo {
// Specifies whether or not there is be a filter added for all files. // Specifies whether or not there is be a filter added for all files.
bool include_all_files; bool include_all_files;
// Specifies whether the (save) file dialog should keep the extension visible.
bool keep_extension_visible;
}; };
// The interface to a file selection (Save As, Upload, etc) dialog. // The interface to a file selection (Save As, Upload, etc) dialog.
......
...@@ -173,6 +173,7 @@ ui::SelectFileDialog::FileTypeInfo ConvertAcceptsToFileTypeInfo( ...@@ -173,6 +173,7 @@ ui::SelectFileDialog::FileTypeInfo ConvertAcceptsToFileTypeInfo(
file_types.include_all_files = true; file_types.include_all_files = true;
file_types.allowed_paths = ui::SelectFileDialog::FileTypeInfo::ANY_PATH; file_types.allowed_paths = ui::SelectFileDialog::FileTypeInfo::ANY_PATH;
file_types.keep_extension_visible = true;
return file_types; return file_types;
} }
......
...@@ -26,8 +26,7 @@ ui::SelectFileDialogFactory* dialog_factory_ = NULL; ...@@ -26,8 +26,7 @@ ui::SelectFileDialogFactory* dialog_factory_ = NULL;
namespace ui { namespace ui {
SelectFileDialog::FileTypeInfo::FileTypeInfo() SelectFileDialog::FileTypeInfo::FileTypeInfo() = default;
: include_all_files(false), allowed_paths(NATIVE_PATH) {}
SelectFileDialog::FileTypeInfo::FileTypeInfo(const FileTypeInfo& other) = SelectFileDialog::FileTypeInfo::FileTypeInfo(const FileTypeInfo& other) =
default; default;
......
...@@ -135,7 +135,13 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog ...@@ -135,7 +135,13 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog
std::vector<base::string16> extension_description_overrides; std::vector<base::string16> extension_description_overrides;
// Specifies whether there will be a filter added for all files (i.e. *.*). // Specifies whether there will be a filter added for all files (i.e. *.*).
bool include_all_files; bool include_all_files = false;
// Some implementations by default hide the extension of a file, in
// particular in a save file dialog. If this is set to true, where
// supported, the save file dialog will instead keep the file extension
// visible.
bool keep_extension_visible = false;
// Specifies which type of paths the caller can handle. // Specifies which type of paths the caller can handle.
enum AllowedPaths { enum AllowedPaths {
...@@ -153,7 +159,7 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog ...@@ -153,7 +159,7 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog
// docs.google.com URL. // docs.google.com URL.
ANY_PATH_OR_URL ANY_PATH_OR_URL
}; };
AllowedPaths allowed_paths; AllowedPaths allowed_paths = NATIVE_PATH;
}; };
// Returns a file path with a base name at most 255 characters long. This // Returns a file path with a base name at most 255 characters long. This
......
...@@ -135,6 +135,8 @@ void SelectFileDialogImpl::SelectFileImpl( ...@@ -135,6 +135,8 @@ void SelectFileDialogImpl::SelectFileImpl(
mojo_file_types->extension_description_overrides = mojo_file_types->extension_description_overrides =
file_types->extension_description_overrides; file_types->extension_description_overrides;
mojo_file_types->include_all_files = file_types->include_all_files; mojo_file_types->include_all_files = file_types->include_all_files;
mojo_file_types->keep_extension_visible =
file_types->keep_extension_visible;
} }
auto callback = base::BindOnce(&SelectFileDialogImpl::FileWasSelected, auto callback = base::BindOnce(&SelectFileDialogImpl::FileWasSelected,
......
...@@ -503,8 +503,29 @@ TEST_F(SelectFileDialogMacTest, MultipleExtension) { ...@@ -503,8 +503,29 @@ TEST_F(SelectFileDialogMacTest, MultipleExtension) {
EXPECT_FALSE([panel isExtensionHidden]); EXPECT_FALSE([panel isExtensionHidden]);
} }
// Test to ensure lifetime is sound if a reference to the panel outlives the // Verify that the file dialog does not hide extension when the
// delegate. // `keep_extension_visible` flag is set to true.
TEST_F(SelectFileDialogMacTest, KeepExtensionVisible) {
const std::string extensions_arr[][2] = {{"html", "htm"}, {"jpeg", "jpg"}};
SelectFileDialog::FileTypeInfo file_type_info;
file_type_info.extensions.push_back(
GetVectorFromArray<std::string>(extensions_arr[0]));
file_type_info.extensions.push_back(
GetVectorFromArray<std::string>(extensions_arr[1]));
file_type_info.keep_extension_visible = true;
FileDialogArguments args(GetDefaultArguments());
args.file_types = &file_type_info;
SelectFileWithParams(args);
NSSavePanel* panel = GetPanel();
EXPECT_FALSE([panel canSelectHiddenExtension]);
EXPECT_FALSE([panel isExtensionHidden]);
}
// Test to ensure lifetime is sound if a reference to
// the panel outlives the delegate.
TEST_F(SelectFileDialogMacTest, Lifetime) { TEST_F(SelectFileDialogMacTest, Lifetime) {
base::scoped_nsobject<NSSavePanel> panel; base::scoped_nsobject<NSSavePanel> panel;
@autoreleasepool { @autoreleasepool {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment