Commit c8e0b726 authored by Marijn Kruisselbrink's avatar Marijn Kruisselbrink Committed by Commit Bot

[NativeFS] Slightly tweak accepted types in the file dialog.

Rather than sorting all extensions, keep the website provided extensions
first and append the mime-type derived extensions to that list. This
should give the website more control over what extension gets appended
to a file name if the user doesn't include an extension themselves.

Additionally pass 1 as file_type_index, to select the first type by
default. Most file dialog implementations did this anyway when passing
0, but it seems better to be explicit about what type to select by
default.

Bug: 1103133
Change-Id: I531f13e72bdaa4de8d7b3270da21a577ebe56e82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288429
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786556}
parent 3ac45db9
...@@ -47,30 +47,51 @@ void RecordFileSelectionResult(blink::mojom::ChooseFileSystemEntryType type, ...@@ -47,30 +47,51 @@ void RecordFileSelectionResult(blink::mojom::ChooseFileSystemEntryType type,
"NativeFileSystemAPI.FileChooserResult." + TypeToString(type), count); "NativeFileSystemAPI.FileChooserResult." + TypeToString(type), count);
} }
// Converts the accepted mime types and extensions from |option| into a list
// of just extensions to be passed to the file dialog implementation.
// The returned list will start with all the explicit website provided
// extensions in order, followed by (for each mime type) the preferred
// extension for that mime type (if any) and any other extensions associated
// with that mime type. Duplicates are filtered out so each extension only
// occurs once in the returned list.
bool GetFileTypesFromAcceptsOption( bool GetFileTypesFromAcceptsOption(
const blink::mojom::ChooseFileSystemEntryAcceptsOption& option, const blink::mojom::ChooseFileSystemEntryAcceptsOption& option,
std::vector<base::FilePath::StringType>* extensions, std::vector<base::FilePath::StringType>* extensions,
base::string16* description) { base::string16* description) {
std::set<base::FilePath::StringType> extension_set; std::set<base::FilePath::StringType> extension_set;
for (const std::string& extension : option.extensions) { for (const std::string& extension_string : option.extensions) {
base::FilePath::StringType extension;
#if defined(OS_WIN) #if defined(OS_WIN)
extension_set.insert(base::UTF8ToWide(extension)); extension = base::UTF8ToWide(extension_string);
#else #else
extension_set.insert(extension); extension = extension_string;
#endif #endif
if (extension_set.insert(extension).second) {
extensions->push_back(std::move(extension));
}
} }
for (const std::string& mime_type : option.mime_types) { for (const std::string& mime_type : option.mime_types) {
base::FilePath::StringType preferred_extension;
if (net::GetPreferredExtensionForMimeType(mime_type,
&preferred_extension)) {
if (extension_set.insert(preferred_extension).second) {
extensions->push_back(std::move(preferred_extension));
}
}
std::vector<base::FilePath::StringType> inner; std::vector<base::FilePath::StringType> inner;
net::GetExtensionsForMimeType(mime_type, &inner); net::GetExtensionsForMimeType(mime_type, &inner);
if (inner.empty()) if (inner.empty())
continue; continue;
extension_set.insert(inner.begin(), inner.end()); for (auto& extension : inner) {
if (extension_set.insert(extension).second) {
extensions->push_back(std::move(extension));
}
}
} }
extensions->assign(extension_set.begin(), extension_set.end());
if (extensions->empty()) if (extensions->empty())
return false; return false;
...@@ -149,7 +170,7 @@ void FileSystemChooser::CreateAndShow( ...@@ -149,7 +170,7 @@ void FileSystemChooser::CreateAndShow(
listener->dialog_->SelectFile( listener->dialog_->SelectFile(
dialog_type, /*title=*/base::string16(), dialog_type, /*title=*/base::string16(),
/*default_path=*/base::FilePath(), &options.file_type_info(), /*default_path=*/base::FilePath(), &options.file_type_info(),
/*file_type_index=*/0, /*file_type_index=*/1,
/*default_extension=*/base::FilePath::StringType(), /*default_extension=*/base::FilePath::StringType(),
web_contents ? web_contents->GetTopLevelNativeWindow() : nullptr, web_contents ? web_contents->GetTopLevelNativeWindow() : nullptr,
/*params=*/nullptr); /*params=*/nullptr);
......
...@@ -496,9 +496,9 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, AcceptsOptions) { ...@@ -496,9 +496,9 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, AcceptsOptions) {
EXPECT_TRUE(dialog_params.file_types->include_all_files); EXPECT_TRUE(dialog_params.file_types->include_all_files);
ASSERT_EQ(2u, dialog_params.file_types->extensions.size()); ASSERT_EQ(2u, dialog_params.file_types->extensions.size());
ASSERT_EQ(2u, dialog_params.file_types->extensions[0].size()); ASSERT_EQ(2u, dialog_params.file_types->extensions[0].size());
EXPECT_EQ(FILE_PATH_LITERAL("Js"),
dialog_params.file_types->extensions[0][0]);
EXPECT_EQ(FILE_PATH_LITERAL("txt"), EXPECT_EQ(FILE_PATH_LITERAL("txt"),
dialog_params.file_types->extensions[0][0]);
EXPECT_EQ(FILE_PATH_LITERAL("Js"),
dialog_params.file_types->extensions[0][1]); dialog_params.file_types->extensions[0][1]);
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[1], EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[1],
FILE_PATH_LITERAL("jpg"))); FILE_PATH_LITERAL("jpg")));
......
...@@ -125,11 +125,11 @@ TEST_F(FileSystemChooserTest, AcceptsExtensions) { ...@@ -125,11 +125,11 @@ TEST_F(FileSystemChooserTest, AcceptsExtensions) {
EXPECT_TRUE(dialog_params.file_types->include_all_files); EXPECT_TRUE(dialog_params.file_types->include_all_files);
ASSERT_EQ(1u, dialog_params.file_types->extensions.size()); ASSERT_EQ(1u, dialog_params.file_types->extensions.size());
EXPECT_EQ(2u, dialog_params.file_types->extensions[0].size()); ASSERT_EQ(2u, dialog_params.file_types->extensions[0].size());
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0], EXPECT_EQ(dialog_params.file_types->extensions[0][0],
FILE_PATH_LITERAL("text"))); FILE_PATH_LITERAL("text"));
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0], EXPECT_EQ(dialog_params.file_types->extensions[0][1],
FILE_PATH_LITERAL("js"))); FILE_PATH_LITERAL("js"));
ASSERT_EQ(1u, ASSERT_EQ(1u,
dialog_params.file_types->extension_description_overrides.size()); dialog_params.file_types->extension_description_overrides.size());
...@@ -151,8 +151,11 @@ TEST_F(FileSystemChooserTest, AcceptsExtensionsAndMimeTypes) { ...@@ -151,8 +151,11 @@ TEST_F(FileSystemChooserTest, AcceptsExtensionsAndMimeTypes) {
EXPECT_FALSE(dialog_params.file_types->include_all_files); EXPECT_FALSE(dialog_params.file_types->include_all_files);
ASSERT_EQ(1u, dialog_params.file_types->extensions.size()); ASSERT_EQ(1u, dialog_params.file_types->extensions.size());
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0], ASSERT_GE(dialog_params.file_types->extensions[0].size(), 4u);
FILE_PATH_LITERAL("text"))); EXPECT_EQ(dialog_params.file_types->extensions[0][0],
FILE_PATH_LITERAL("text"));
EXPECT_EQ(dialog_params.file_types->extensions[0][1],
FILE_PATH_LITERAL("jpg"));
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0], EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0],
FILE_PATH_LITERAL("gif"))); FILE_PATH_LITERAL("gif")));
EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0], EXPECT_TRUE(base::Contains(dialog_params.file_types->extensions[0],
......
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