From 20a659fa662c78be7832876b13c7ea6e30f947bd Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 29 May 2024 16:38:00 -0400 Subject: [PATCH] Pass mime type values as file dialog accept filters (see #3314) File dialogs that specify mime type (e.g. "image/*") accept filters will pass those values unchanged to the OnFileDialog |accept_filters| parameter. The default dialog implementation will show those filters in addition to a combined "Custom Files" filter. This is a change from preexisting Google Chrome behavior where only the combined "Custom Files" filter is displayed, and restores CEF behavior that existed prior to 2ea7459a89. Document the fact that OnFileDialog may be called twice, once before MIME type expansion and once afterwards. Add new OnFileDialog |accept_extensions| and |accept_descriptions| parameters for MIME type extensions and descriptions. Details: This change adds a SelectFileDialog::FileTypeInfo::extension_mimetypes member and improves the logic in FileSelectHelper::GetFileTypesFromAcceptType and file_dialog_manager.cc SelectFileToFileChooserParams to support recall of the source mime type when populating the FileChooserParams structure. To test: - Run `ceftests --gtest_filter=DialogTest.*` - Run `cefclient --url=https://tests/dialogs` --- include/capi/cef_dialog_handler_capi.h | 25 ++-- include/cef_api_hash.h | 8 +- include/cef_dialog_handler.h | 25 ++-- libcef/browser/file_dialog_manager.cc | 78 ++++++++--- libcef/browser/file_dialog_manager.h | 4 + libcef_dll/cpptoc/dialog_handler_cpptoc.cc | 17 ++- libcef_dll/ctocpp/dialog_handler_ctocpp.cc | 31 +++- libcef_dll/ctocpp/dialog_handler_ctocpp.h | 4 +- .../chrome_browser_dialogs_native.patch | 105 +++++++------- tests/cefclient/browser/dialog_handler_gtk.cc | 74 ++++++---- tests/cefclient/browser/dialog_handler_gtk.h | 4 + tests/cefclient/browser/dialog_test.cc | 7 +- tests/ceftests/dialog_unittest.cc | 132 +++++++++++++++--- tests/ceftests/test_util.cc | 3 + tests/shared/common/string_util.cc | 14 ++ tests/shared/common/string_util.h | 4 + 16 files changed, 389 insertions(+), 146 deletions(-) diff --git a/include/capi/cef_dialog_handler_capi.h b/include/capi/cef_dialog_handler_capi.h index ef92835ee..f884bcefb 100644 --- a/include/capi/cef_dialog_handler_capi.h +++ b/include/capi/cef_dialog_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=5644fdc2453dd083079bf9e3616b687eeb49f250$ +// $hash=bf7208a86ee17f63fd7163cef8c3a13373a1f1c8$ // #ifndef CEF_INCLUDE_CAPI_CEF_DIALOG_HANDLER_CAPI_H_ @@ -86,13 +86,20 @@ typedef struct _cef_dialog_handler_t { /// to show the default title ("Open" or "Save" depending on the mode). /// |default_file_path| is the path with optional directory and/or file name /// component that should be initially selected in the dialog. - /// |accept_filters| are used to restrict the selectable file types and may - /// any combination of (a) valid lower-cased MIME types (e.g. "text/*" or - /// "image/*"), (b) individual file extensions (e.g. ".txt" or ".png"), or (c) - /// combined description and file extension delimited using "|" and ";" (e.g. - /// "Image Types|.png;.gif;.jpg"). To display a custom dialog return true (1) - /// and execute |callback| either inline or at a later time. To display the - /// default dialog return false (0). + /// |accept_filters| are used to restrict the selectable file types and may be + /// any combination of valid lower-cased MIME types (e.g. "text/*" or + /// "image/*") and individual file extensions (e.g. ".txt" or ".png"). + /// |accept_extensions| provides the semicolon-delimited expansion of MIME + /// types to file extensions (if known, or NULL string otherwise). + /// |accept_descriptions| provides the descriptions for MIME types (if known, + /// or NULL string otherwise). For example, the "image/*" mime type might have + /// extensions ".png;.jpg;.bmp;..." and description "Image Files". + /// |accept_filters|, |accept_extensions| and |accept_descriptions| will all + /// be the same size. To display a custom dialog return true (1) and execute + /// |callback| either inline or at a later time. To display the default dialog + /// return false (0). If this function returns false (0) it may be called an + /// additional time for the same dialog (both before and after MIME type + /// expansion). /// int(CEF_CALLBACK* on_file_dialog)( struct _cef_dialog_handler_t* self, @@ -101,6 +108,8 @@ typedef struct _cef_dialog_handler_t { const cef_string_t* title, const cef_string_t* default_file_path, cef_string_list_t accept_filters, + cef_string_list_t accept_extensions, + cef_string_list_t accept_descriptions, struct _cef_file_dialog_callback_t* callback); } cef_dialog_handler_t; diff --git a/include/cef_api_hash.h b/include/cef_api_hash.h index 3376655d1..0dcbced41 100644 --- a/include/cef_api_hash.h +++ b/include/cef_api_hash.h @@ -42,13 +42,13 @@ // way that may cause binary incompatibility with other builds. The universal // hash value will change if any platform is affected whereas the platform hash // values will change only if that particular platform is affected. -#define CEF_API_HASH_UNIVERSAL "100bd8428968266a67128c02f63e2fd5aaa90d28" +#define CEF_API_HASH_UNIVERSAL "3ecad1c27a9f10720824f4c4be478dab28f1258d" #if defined(OS_WIN) -#define CEF_API_HASH_PLATFORM "d5cba94f734fb1966f098fad5cc5f342a5d14afe" +#define CEF_API_HASH_PLATFORM "551fbcb8ec2687c554be82cc10f471201c88727b" #elif defined(OS_MAC) -#define CEF_API_HASH_PLATFORM "f5554ef4857432df5d22a14a9df7f3b1248c3265" +#define CEF_API_HASH_PLATFORM "4ca22671083a799f3c8c09905804882988ebd97b" #elif defined(OS_LINUX) -#define CEF_API_HASH_PLATFORM "5c283fac1f398ae4a5a5c2f16a4984267034950f" +#define CEF_API_HASH_PLATFORM "fb827bbeab0a16044f5cee11912d7dfc2ad44999" #endif #ifdef __cplusplus diff --git a/include/cef_dialog_handler.h b/include/cef_dialog_handler.h index 30fd3a16d..d922885bf 100644 --- a/include/cef_dialog_handler.h +++ b/include/cef_dialog_handler.h @@ -77,21 +77,30 @@ class CefDialogHandler : public virtual CefBaseRefCounted { /// empty to show the default title ("Open" or "Save" depending on the mode). /// |default_file_path| is the path with optional directory and/or file name /// component that should be initially selected in the dialog. - /// |accept_filters| are used to restrict the selectable file types and may - /// any combination of (a) valid lower-cased MIME types (e.g. "text/*" or - /// "image/*"), (b) individual file extensions (e.g. ".txt" or ".png"), or (c) - /// combined description and file extension delimited using "|" and ";" (e.g. - /// "Image Types|.png;.gif;.jpg"). To display a custom dialog return true and - /// execute |callback| either inline or at a later time. To display the - /// default dialog return false. + /// |accept_filters| are used to restrict the selectable file types and may be + /// any combination of valid lower-cased MIME types (e.g. "text/*" or + /// "image/*") and individual file extensions (e.g. ".txt" or ".png"). + /// |accept_extensions| provides the semicolon-delimited expansion of MIME + /// types to file extensions (if known, or empty string otherwise). + /// |accept_descriptions| provides the descriptions for MIME types (if known, + /// or empty string otherwise). For example, the "image/*" mime type might + /// have extensions ".png;.jpg;.bmp;..." and description "Image Files". + /// |accept_filters|, |accept_extensions| and |accept_descriptions| will all + /// be the same size. To display a custom dialog return true and execute + /// |callback| either inline or at a later time. To display the default dialog + /// return false. If this method returns false it may be called an additional + /// time for the same dialog (both before and after MIME type expansion). /// /*--cef(optional_param=title,optional_param=default_file_path, - optional_param=accept_filters)--*/ + optional_param=accept_filters,optional_param=accept_extensions, + optional_param=accept_descriptions)--*/ virtual bool OnFileDialog(CefRefPtr browser, FileDialogMode mode, const CefString& title, const CefString& default_file_path, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) { return false; } diff --git a/libcef/browser/file_dialog_manager.cc b/libcef/browser/file_dialog_manager.cc index b94260af7..a7ad14278 100644 --- a/libcef/browser/file_dialog_manager.cc +++ b/libcef/browser/file_dialog_manager.cc @@ -141,16 +141,19 @@ FileChooserParams SelectFileToFileChooserParams( params.title = title; params.default_file_name = default_path; - // Note that this translation will lose any mime-type based filters that - // may have existed in the original FileChooserParams::accept_types if this - // dialog was created via FileSelectHelper::RunFileChooser. if (file_types) { - // A list of allowed extensions. For example, it might be + // |file_types| comes from FileSelectHelper::GetFileTypesFromAcceptType. + // |extensions| is a list of allowed extensions. For example, it might be // { { "htm", "html" }, { "txt" } } - for (auto& vec : file_types->extensions) { - for (auto& ext : vec) { - params.accept_types.push_back( - FilePathTypeToString16(FILE_PATH_LITERAL(".") + ext)); + for (size_t i = 0; i < file_types->extensions.size(); ++i) { + if (!file_types->extension_mimetypes[i].empty()) { + // Use the original mime type. + params.accept_types.push_back(file_types->extension_mimetypes[i]); + } else if (file_types->extensions[i].size() == 1) { + // Use the single file extension. We ignore the "Custom Files" filter + // which is the only instance of multiple file extensions. + params.accept_types.push_back(FilePathTypeToString16( + FILE_PATH_LITERAL(".") + file_types->extensions[i][0])); } } } @@ -328,8 +331,10 @@ void CefFileDialogManager::RunFileChooser( // handled here there will be another call to the delegate from RunSelectFile. // It might be better to execute the delegate only the single time here, but // we don't currently have sufficient state in RunSelectFile to know that the - // delegate has already been executed. - callback = MaybeRunDelegate(params, std::move(callback)); + // delegate has already been executed. Also, we haven't retrieved file + // extension data at this point. + callback = MaybeRunDelegate(params, Extensions(), Descriptions(), + std::move(callback)); if (callback.is_null()) { // The delegate kept the callback. return; @@ -377,14 +382,15 @@ void CefFileDialogManager::RunSelectFile( active_listeners_.insert(listener); - // This will not be an exact representation of the original params. auto chooser_params = SelectFileToFileChooserParams(type, title, default_path, file_types); auto callback = base::BindOnce(&CefFileDialogManager::SelectFileDoneByDelegateCallback, weak_ptr_factory_.GetWeakPtr(), base::Unretained(listener), base::Unretained(params)); - callback = MaybeRunDelegate(chooser_params, std::move(callback)); + callback = MaybeRunDelegate(chooser_params, file_types->extensions, + file_types->extension_description_overrides, + std::move(callback)); if (callback.is_null()) { // The delegate kept the callback. return; @@ -456,7 +462,15 @@ void CefFileDialogManager::SelectFileListenerDestroyed( CefFileDialogManager::RunFileChooserCallback CefFileDialogManager::MaybeRunDelegate( const blink::mojom::FileChooserParams& params, + const Extensions& extensions, + const Descriptions& descriptions, RunFileChooserCallback callback) { + // |extensions| and |descriptions| may be empty, or may contain 1 additional + // entry for the "Custom Files" filter. + DCHECK(extensions.empty() || extensions.size() >= params.accept_types.size()); + DCHECK(descriptions.empty() || + descriptions.size() >= params.accept_types.size()); + if (auto client = browser_->client()) { if (auto handler = browser_->client()->GetDialogHandler()) { int mode = FILE_DIALOG_OPEN; @@ -478,12 +492,37 @@ CefFileDialogManager::MaybeRunDelegate( break; } - std::vector::const_iterator it; - std::vector accept_filters; - it = params.accept_types.begin(); - for (; it != params.accept_types.end(); ++it) { - accept_filters.push_back(*it); + for (auto& accept_type : params.accept_types) { + accept_filters.push_back(accept_type); + } + + std::vector accept_extensions; + std::vector accept_descriptions; + if (extensions.empty()) { + // We don't know the expansion of mime type values at this time, + // so only include the single file extensions. + for (auto& accept_type : params.accept_types) { + accept_extensions.push_back( + accept_type.ends_with(u"/*") ? std::u16string() : accept_type); + } + // Empty descriptions. + accept_descriptions.resize(params.accept_types.size()); + } else { + // There may be 1 additional entry in |extensions| and |descriptions| + // that we want to ignore (for the "Custom Files" filter). + for (size_t i = 0; i < params.accept_types.size(); ++i) { + const auto& extension_list = extensions[i]; + std::u16string ext_str; + for (auto& ext : extension_list) { + if (!ext_str.empty()) { + ext_str += u";"; + } + ext_str += FilePathTypeToString16(FILE_PATH_LITERAL(".") + ext); + } + accept_extensions.push_back(ext_str); + accept_descriptions.push_back(descriptions[i]); + } } CefRefPtr callbackImpl( @@ -491,10 +530,13 @@ CefFileDialogManager::MaybeRunDelegate( const bool handled = handler->OnFileDialog( browser_.get(), static_cast(mode), params.title, params.default_file_name.value(), accept_filters, - callbackImpl.get()); + accept_extensions, accept_descriptions, callbackImpl.get()); if (!handled) { // May return nullptr if the client has already executed the callback. callback = callbackImpl->Disconnect(); + LOG_IF(ERROR, callback.is_null()) + << "Should return true from OnFileDialog when executing the " + "callback"; } } } diff --git a/libcef/browser/file_dialog_manager.h b/libcef/browser/file_dialog_manager.h index 78ee27fa0..504136ba0 100644 --- a/libcef/browser/file_dialog_manager.h +++ b/libcef/browser/file_dialog_manager.h @@ -73,8 +73,12 @@ class CefFileDialogManager { void SelectFileListenerDestroyed(ui::SelectFileDialog::Listener* listener); private: + using Extensions = std::vector>; + using Descriptions = std::vector; [[nodiscard]] RunFileChooserCallback MaybeRunDelegate( const blink::mojom::FileChooserParams& params, + const Extensions& extensions, + const Descriptions& descriptions, RunFileChooserCallback callback); void SelectFileDoneByDelegateCallback( diff --git a/libcef_dll/cpptoc/dialog_handler_cpptoc.cc b/libcef_dll/cpptoc/dialog_handler_cpptoc.cc index 9d430ab28..0d2fc6a14 100644 --- a/libcef_dll/cpptoc/dialog_handler_cpptoc.cc +++ b/libcef_dll/cpptoc/dialog_handler_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=1bb6115ab6501d8d006585cede00970e59af9868$ +// $hash=3c95f12406eee58308a2c019e8081e0710769e0d$ // #include "libcef_dll/cpptoc/dialog_handler_cpptoc.h" @@ -30,6 +30,8 @@ dialog_handler_on_file_dialog(struct _cef_dialog_handler_t* self, const cef_string_t* title, const cef_string_t* default_file_path, cef_string_list_t accept_filters, + cef_string_list_t accept_extensions, + cef_string_list_t accept_descriptions, cef_file_dialog_callback_t* callback) { shutdown_checker::AssertNotShutdown(); @@ -49,17 +51,24 @@ dialog_handler_on_file_dialog(struct _cef_dialog_handler_t* self, if (!callback) { return 0; } - // Unverified params: title, default_file_path, accept_filters + // Unverified params: title, default_file_path, accept_filters, + // accept_extensions, accept_descriptions // Translate param: accept_filters; type: string_vec_byref_const std::vector accept_filtersList; transfer_string_list_contents(accept_filters, accept_filtersList); + // Translate param: accept_extensions; type: string_vec_byref_const + std::vector accept_extensionsList; + transfer_string_list_contents(accept_extensions, accept_extensionsList); + // Translate param: accept_descriptions; type: string_vec_byref_const + std::vector accept_descriptionsList; + transfer_string_list_contents(accept_descriptions, accept_descriptionsList); // Execute bool _retval = CefDialogHandlerCppToC::Get(self)->OnFileDialog( CefBrowserCToCpp::Wrap(browser), mode, CefString(title), - CefString(default_file_path), accept_filtersList, - CefFileDialogCallbackCToCpp::Wrap(callback)); + CefString(default_file_path), accept_filtersList, accept_extensionsList, + accept_descriptionsList, CefFileDialogCallbackCToCpp::Wrap(callback)); // Return type: bool return _retval; diff --git a/libcef_dll/ctocpp/dialog_handler_ctocpp.cc b/libcef_dll/ctocpp/dialog_handler_ctocpp.cc index 3cdb94b12..bf55be1b2 100644 --- a/libcef_dll/ctocpp/dialog_handler_ctocpp.cc +++ b/libcef_dll/ctocpp/dialog_handler_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=40bd7cc8a8fd361aceda9bbd426f912ac140ac3d$ +// $hash=bd2c8c9be10525d69d072dacfb3cfd215486e077$ // #include "libcef_dll/ctocpp/dialog_handler_ctocpp.h" @@ -28,6 +28,8 @@ bool CefDialogHandlerCToCpp::OnFileDialog( const CefString& title, const CefString& default_file_path, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) { shutdown_checker::AssertNotShutdown(); @@ -48,7 +50,8 @@ bool CefDialogHandlerCToCpp::OnFileDialog( if (!callback.get()) { return false; } - // Unverified params: title, default_file_path, accept_filters + // Unverified params: title, default_file_path, accept_filters, + // accept_extensions, accept_descriptions // Translate param: accept_filters; type: string_vec_byref_const cef_string_list_t accept_filtersList = cef_string_list_alloc(); @@ -56,17 +59,37 @@ bool CefDialogHandlerCToCpp::OnFileDialog( if (accept_filtersList) { transfer_string_list_contents(accept_filters, accept_filtersList); } + // Translate param: accept_extensions; type: string_vec_byref_const + cef_string_list_t accept_extensionsList = cef_string_list_alloc(); + DCHECK(accept_extensionsList); + if (accept_extensionsList) { + transfer_string_list_contents(accept_extensions, accept_extensionsList); + } + // Translate param: accept_descriptions; type: string_vec_byref_const + cef_string_list_t accept_descriptionsList = cef_string_list_alloc(); + DCHECK(accept_descriptionsList); + if (accept_descriptionsList) { + transfer_string_list_contents(accept_descriptions, accept_descriptionsList); + } // Execute int _retval = _struct->on_file_dialog( _struct, CefBrowserCppToC::Wrap(browser), mode, title.GetStruct(), - default_file_path.GetStruct(), accept_filtersList, - CefFileDialogCallbackCppToC::Wrap(callback)); + default_file_path.GetStruct(), accept_filtersList, accept_extensionsList, + accept_descriptionsList, CefFileDialogCallbackCppToC::Wrap(callback)); // Restore param:accept_filters; type: string_vec_byref_const if (accept_filtersList) { cef_string_list_free(accept_filtersList); } + // Restore param:accept_extensions; type: string_vec_byref_const + if (accept_extensionsList) { + cef_string_list_free(accept_extensionsList); + } + // Restore param:accept_descriptions; type: string_vec_byref_const + if (accept_descriptionsList) { + cef_string_list_free(accept_descriptionsList); + } // Return type: bool return _retval ? true : false; diff --git a/libcef_dll/ctocpp/dialog_handler_ctocpp.h b/libcef_dll/ctocpp/dialog_handler_ctocpp.h index 0df7d03a6..bcd10d8f7 100644 --- a/libcef_dll/ctocpp/dialog_handler_ctocpp.h +++ b/libcef_dll/ctocpp/dialog_handler_ctocpp.h @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=b9efac38dfb7834bd87b6c0dfd29e0fc3179b6ee$ +// $hash=48e8da3e0a0ce27bf61f24f601d0ca179d232b90$ // #ifndef CEF_LIBCEF_DLL_CTOCPP_DIALOG_HANDLER_CTOCPP_H_ @@ -42,6 +42,8 @@ class CefDialogHandlerCToCpp const CefString& title, const CefString& default_file_path, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) override; }; diff --git a/patch/patches/chrome_browser_dialogs_native.patch b/patch/patches/chrome_browser_dialogs_native.patch index 2421bed62..e9ce38e1b 100644 --- a/patch/patches/chrome_browser_dialogs_native.patch +++ b/patch/patches/chrome_browser_dialogs_native.patch @@ -1,5 +1,5 @@ diff --git chrome/browser/file_select_helper.cc chrome/browser/file_select_helper.cc -index 447a91b9ac380..cf5ad1b907977 100644 +index 447a91b9ac380..0452f0a8b57cf 100644 --- chrome/browser/file_select_helper.cc +++ chrome/browser/file_select_helper.cc @@ -20,6 +20,7 @@ @@ -43,30 +43,23 @@ index 447a91b9ac380..cf5ad1b907977 100644 #if BUILDFLAG(ENTERPRISE_CLOUD_CONTENT_ANALYSIS) enterprise_connectors::ContentAnalysisDelegate::Data data; if (enterprise_connectors::ContentAnalysisDelegate::IsEnabled( -@@ -459,7 +479,8 @@ void FileSelectHelper::DontAbortOnMissingWebContentsForTesting() { - - std::unique_ptr - FileSelectHelper::GetFileTypesFromAcceptType( -- const std::vector& accept_types) { -+ const std::vector& accept_types, -+ bool run_from_cef) { - auto base_file_type = std::make_unique(); - if (accept_types.empty()) - return base_file_type; -@@ -472,17 +493,24 @@ FileSelectHelper::GetFileTypesFromAcceptType( +@@ -472,31 +492,51 @@ FileSelectHelper::GetFileTypesFromAcceptType( std::vector* extensions = &file_type->extensions.back(); + // Create individual filters for each accept type. + std::vector> all_extensions; + std::vector all_overrides; ++ std::vector all_mimetypes; + // Find the corresponding extensions. int valid_type_count = 0; int description_id = 0; ++ std::string ascii_type; for (const auto& accept_type : accept_types) { + std::vector current_extensions; + description_id = 0; ++ ascii_type.clear(); + size_t old_extension_size = extensions->size(); if (accept_type[0] == '.') { @@ -79,7 +72,11 @@ index 447a91b9ac380..cf5ad1b907977 100644 } else { if (!base::IsStringASCII(accept_type)) continue; -@@ -493,10 +521,18 @@ FileSelectHelper::GetFileTypesFromAcceptType( +- std::string ascii_type = base::UTF16ToASCII(accept_type); ++ ascii_type = base::UTF16ToASCII(accept_type); + if (ascii_type == "image/*") + description_id = IDS_IMAGE_FILES; + else if (ascii_type == "audio/*") description_id = IDS_AUDIO_FILES; else if (ascii_type == "video/*") description_id = IDS_VIDEO_FILES; @@ -93,6 +90,8 @@ index 447a91b9ac380..cf5ad1b907977 100644 + all_overrides.push_back(description_id != 0 ? + l10n_util::GetStringUTF16(description_id) : + std::u16string()); ++ all_mimetypes.push_back(ascii_type.empty() ? ++ std::u16string() : accept_type); + + extensions->insert(extensions->end(), current_extensions.begin(), + current_extensions.end()); @@ -100,23 +99,40 @@ index 447a91b9ac380..cf5ad1b907977 100644 if (extensions->size() > old_extension_size) valid_type_count++; } -@@ -521,6 +557,15 @@ FileSelectHelper::GetFileTypesFromAcceptType( - l10n_util::GetStringUTF16(description_id)); - } +@@ -513,12 +553,28 @@ FileSelectHelper::GetFileTypesFromAcceptType( + // dialog uses the first extension in the list to form the description, + // like "EHTML Files". This is not what we want. + if (valid_type_count > 1 || +- (valid_type_count == 1 && description_id == 0 && extensions->size() > 1)) ++ (valid_type_count == 1 && description_id == 0 && extensions->size() > 1)) { + description_id = IDS_CUSTOM_FILES; ++ ascii_type.clear(); ++ } -+ if (run_from_cef && all_extensions.size() > 1) { +- if (description_id) { +- file_type->extension_description_overrides.push_back( +- l10n_util::GetStringUTF16(description_id)); ++ file_type->extension_description_overrides.push_back( ++ description_id != 0 ? ++ l10n_util::GetStringUTF16(description_id) : ++ std::u16string()); ++ file_type->extension_mimetypes.push_back( ++ ascii_type.empty() ? std::u16string() : base::ASCIIToUTF16(ascii_type)); ++ ++ if (all_extensions.size() > 1) { + // Insert filters for the specific accept types at the beginning. + file_type->extensions.insert(file_type->extensions.begin(), + all_extensions.begin(), all_extensions.end()); + file_type->extension_description_overrides.insert( + file_type->extension_description_overrides.begin(), + all_overrides.begin(), all_overrides.end()); -+ } -+ - return file_type; - } ++ file_type->extension_mimetypes.insert( ++ file_type->extension_mimetypes.begin(), ++ all_mimetypes.begin(), all_mimetypes.end()); + } -@@ -528,7 +573,8 @@ FileSelectHelper::GetFileTypesFromAcceptType( + return file_type; +@@ -528,7 +584,8 @@ FileSelectHelper::GetFileTypesFromAcceptType( void FileSelectHelper::RunFileChooser( content::RenderFrameHost* render_frame_host, scoped_refptr listener, @@ -126,7 +142,7 @@ index 447a91b9ac380..cf5ad1b907977 100644 Profile* profile = Profile::FromBrowserContext( render_frame_host->GetProcess()->GetBrowserContext()); -@@ -547,6 +593,7 @@ void FileSelectHelper::RunFileChooser( +@@ -547,6 +604,7 @@ void FileSelectHelper::RunFileChooser( // message. scoped_refptr file_select_helper( new FileSelectHelper(profile)); @@ -134,18 +150,8 @@ index 447a91b9ac380..cf5ad1b907977 100644 file_select_helper->RunFileChooser(render_frame_host, std::move(listener), params.Clone()); } -@@ -598,7 +645,8 @@ void FileSelectHelper::RunFileChooser( - } - - void FileSelectHelper::GetFileTypesInThreadPool(FileChooserParamsPtr params) { -- select_file_types_ = GetFileTypesFromAcceptType(params->accept_types); -+ select_file_types_ = GetFileTypesFromAcceptType(params->accept_types, -+ run_from_cef_); - select_file_types_->allowed_paths = - params->need_local_path ? ui::SelectFileDialog::FileTypeInfo::NATIVE_PATH - : ui::SelectFileDialog::FileTypeInfo::ANY_PATH; diff --git chrome/browser/file_select_helper.h chrome/browser/file_select_helper.h -index 7194323c36956..903cdf0c294fc 100644 +index 7194323c36956..fd4a5361798f0 100644 --- chrome/browser/file_select_helper.h +++ chrome/browser/file_select_helper.h @@ -62,7 +62,8 @@ class FileSelectHelper : public base::RefCountedThreadSafe< @@ -158,17 +164,7 @@ index 7194323c36956..903cdf0c294fc 100644 // Enumerates all the files in directory. static void EnumerateDirectory( -@@ -264,7 +265,8 @@ class FileSelectHelper : public base::RefCountedThreadSafe< - // |accept_types| contains only valid lowercased MIME types or file extensions - // beginning with a period (.). - static std::unique_ptr -- GetFileTypesFromAcceptType(const std::vector& accept_types); -+ GetFileTypesFromAcceptType(const std::vector& accept_types, -+ bool run_from_cef); - - // Check the accept type is valid. It is expected to be all lower case with - // no whitespace. -@@ -325,6 +327,9 @@ class FileSelectHelper : public base::RefCountedThreadSafe< +@@ -325,6 +326,9 @@ class FileSelectHelper : public base::RefCountedThreadSafe< // Set to false in unit tests since there is no WebContents. bool abort_on_missing_web_contents_in_tests_ = true; @@ -261,7 +257,7 @@ index 68dd62159b686..e94831cd44a2d 100644 return CreateSelectFileDialog(listener, std::move(policy)); } diff --git ui/shell_dialogs/select_file_dialog.h ui/shell_dialogs/select_file_dialog.h -index 9b12fae59c3cc..dfb534a6f06e6 100644 +index 9b12fae59c3cc..a80873f421c8d 100644 --- ui/shell_dialogs/select_file_dialog.h +++ ui/shell_dialogs/select_file_dialog.h @@ -102,7 +102,8 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog @@ -274,7 +270,18 @@ index 9b12fae59c3cc..dfb534a6f06e6 100644 SelectFileDialog(const SelectFileDialog&) = delete; SelectFileDialog& operator=(const SelectFileDialog&) = delete; -@@ -199,6 +200,19 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog +@@ -126,6 +127,10 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog + // be used. + std::vector extension_description_overrides; + ++ // Original mime types for the specified extensions. Entries correspond to ++ // |extensions|; if left blank then there was no mime type. ++ std::vector extension_mimetypes; ++ + // Specifies whether there will be a filter added for all files (i.e. *.*). + bool include_all_files = false; + +@@ -199,6 +204,19 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog const GURL* caller = nullptr); bool HasMultipleFileTypeChoices(); @@ -294,7 +301,7 @@ index 9b12fae59c3cc..dfb534a6f06e6 100644 protected: friend class base::RefCountedThreadSafe; -@@ -224,6 +238,11 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog +@@ -224,6 +242,11 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog // The listener to be notified of selection completion. raw_ptr listener_; @@ -306,7 +313,7 @@ index 9b12fae59c3cc..dfb534a6f06e6 100644 private: // Tests if the file selection dialog can be displayed by // testing if the AllowFileSelectionDialogs-Policy is -@@ -236,8 +255,6 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog +@@ -236,8 +259,6 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog // Returns true if the dialog has multiple file type choices. virtual bool HasMultipleFileTypeChoicesImpl() = 0; diff --git a/tests/cefclient/browser/dialog_handler_gtk.cc b/tests/cefclient/browser/dialog_handler_gtk.cc index 33e687bb3..bb2c990d5 100644 --- a/tests/cefclient/browser/dialog_handler_gtk.cc +++ b/tests/cefclient/browser/dialog_handler_gtk.cc @@ -7,11 +7,13 @@ #include #include +#include "include/base/cef_logging.h" #include "include/cef_browser.h" #include "include/cef_parser.h" #include "include/wrapper/cef_helpers.h" #include "tests/cefclient/browser/root_window.h" #include "tests/cefclient/browser/util_gtk.h" +#include "tests/shared/common/string_util.h" namespace client { @@ -49,11 +51,14 @@ std::string GetDescriptionFromMimeType(const std::string& mime_type) { } } + LOG(WARNING) << "Unrecognized mime type: " << mime_type; return std::string(); } void AddFilters(GtkFileChooser* chooser, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, bool include_all_files, std::vector* filters) { bool has_filter = false; @@ -64,33 +69,13 @@ void AddFilters(GtkFileChooser* chooser, continue; } - std::vector extensions; - std::string description; - - size_t sep_index = filter.find('|'); - if (sep_index != std::string::npos) { - // Treat as a filter of the form "Filter Name|.ext1;.ext2;.ext3". - description = filter.substr(0, sep_index); - - const std::string& exts = filter.substr(sep_index + 1); - size_t last = 0; - size_t size = exts.size(); - for (size_t i = 0; i <= size; ++i) { - if (i == size || exts[i] == ';') { - std::string ext(exts, last, i - last); - if (!ext.empty() && ext[0] == '.') { - extensions.push_back(ext); - } - last = i + 1; - } - } - } else if (filter[0] == '.') { - // Treat as an extension beginning with the '.' character. - extensions.push_back(filter); - } else { - // Otherwise convert mime type to one or more extensions. - description = GetDescriptionFromMimeType(filter); + // Extensions and descriptions may be provided. + std::vector extensions = + AsciiStrSplit(accept_extensions[j], ';'); + std::string description = accept_descriptions[j]; + if (extensions.empty()) { + // Try to convert mime type to one or more extensions. std::vector ext; CefGetExtensionsForMimeType(filter, ext); for (size_t x = 0; x < ext.size(); ++x) { @@ -102,6 +87,10 @@ void AddFilters(GtkFileChooser* chooser, continue; } + if (description.empty() && filter[0] != '.') { + description = GetDescriptionFromMimeType(filter); + } + GtkFileFilter* gtk_filter = gtk_file_filter_new(); std::string ext_str; @@ -154,6 +143,24 @@ GtkWindow* GetWindow(CefRefPtr browser) { return nullptr; } +// Returns true if |accept_filters| contains a MIME type value without matching +// extensions in |accept_extensions|. +bool MissingMimeTypeData(const std::vector& accept_filters, + const std::vector& accept_extensions) { + if (accept_filters.empty()) { + return false; + } + + for (size_t i = 0; i < accept_filters.size(); ++i) { + const std::string& filter = accept_filters[i]; + if (filter[0] != '.' && accept_extensions[i].empty()) { + return true; + } + } + + return false; +} + } // namespace ClientDialogHandlerGtk::ClientDialogHandlerGtk() : gtk_dialog_(nullptr) {} @@ -164,8 +171,17 @@ bool ClientDialogHandlerGtk::OnFileDialog( const CefString& title, const CefString& default_file_path, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) { CEF_REQUIRE_UI_THREAD(); + DCHECK(accept_filters.size() == accept_extensions.size() == + accept_descriptions.size()); + + if (MissingMimeTypeData(accept_filters, accept_extensions)) { + // Wait for the 2nd call that provides MIME type data. + return false; + } OnFileDialogParams params; params.browser = browser; @@ -173,6 +189,8 @@ bool ClientDialogHandlerGtk::OnFileDialog( params.title = title; params.default_file_path = default_file_path; params.accept_filters = accept_filters; + params.accept_extensions = accept_extensions; + params.accept_descriptions = accept_descriptions; params.callback = callback; GetWindowAndContinue( @@ -309,7 +327,9 @@ void ClientDialogHandlerGtk::OnFileDialogContinue( } std::vector filters; - AddFilters(GTK_FILE_CHOOSER(dialog), params.accept_filters, true, &filters); + AddFilters(GTK_FILE_CHOOSER(dialog), params.accept_filters, + params.accept_extensions, params.accept_descriptions, true, + &filters); bool success = false; diff --git a/tests/cefclient/browser/dialog_handler_gtk.h b/tests/cefclient/browser/dialog_handler_gtk.h index 1ad967155..d28a1b5a4 100644 --- a/tests/cefclient/browser/dialog_handler_gtk.h +++ b/tests/cefclient/browser/dialog_handler_gtk.h @@ -25,6 +25,8 @@ class ClientDialogHandlerGtk : public CefDialogHandler, const CefString& title, const CefString& default_file_path, const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) override; // CefJSDialogHandler methods. @@ -48,6 +50,8 @@ class ClientDialogHandlerGtk : public CefDialogHandler, CefString title; CefString default_file_path; std::vector accept_filters; + std::vector accept_extensions; + std::vector accept_descriptions; CefRefPtr callback; }; void OnFileDialogContinue(const OnFileDialogParams& params, diff --git a/tests/cefclient/browser/dialog_test.cc b/tests/cefclient/browser/dialog_test.cc index 2222e1633..c337c4474 100644 --- a/tests/cefclient/browser/dialog_test.cc +++ b/tests/cefclient/browser/dialog_test.cc @@ -140,16 +140,11 @@ class Handler : public CefMessageRouterBrowserSide::Handler { if (accept_filters.empty() && dialog_state_->mode_ != FILE_DIALOG_OPEN_FOLDER) { // Build filters based on mime time. - accept_filters.push_back("text/*"); + accept_filters.push_back("image/*"); // Build filters based on file extension. accept_filters.push_back(".log"); accept_filters.push_back(".patch"); - - // Add specific filters as-is. - accept_filters.push_back("Document Files|.doc;.odt"); - accept_filters.push_back("Image Files|.png;.jpg;.gif"); - accept_filters.push_back("PDF Files|.pdf"); } dialog_state_->pending_ = true; diff --git a/tests/ceftests/dialog_unittest.cc b/tests/ceftests/dialog_unittest.cc index 947abe4bc..aa9cf0b2c 100644 --- a/tests/ceftests/dialog_unittest.cc +++ b/tests/ceftests/dialog_unittest.cc @@ -19,15 +19,22 @@ class DialogTestHandler : public TestHandler { : mode(dialog_mode), title("Test Title"), default_file_name("Test File Name") { - accept_types.push_back("text/*"); - accept_types.push_back(".js"); - accept_types.push_back(".css"); + accept_filters.push_back("image/*"); + // We're handling the dialog before MIME type expansion. + accept_extensions.push_back(CefString()); + accept_filters.push_back(".js"); + accept_extensions.push_back(".js"); + accept_filters.push_back(".css"); + accept_extensions.push_back(".css"); } FileDialogMode mode; CefString title; CefString default_file_name; - std::vector accept_types; + std::vector accept_filters; + std::vector accept_extensions; + + bool skip_first_callback = false; // True if the callback should execute asynchronously. bool callback_async = false; @@ -76,9 +83,9 @@ class DialogTestHandler : public TestHandler { void OnLoadEnd(CefRefPtr browser, CefRefPtr frame, int httpStatusCode) override { - browser->GetHost()->RunFileDialog(config_.mode, config_.title, - config_.default_file_name, - config_.accept_types, new Callback(this)); + browser->GetHost()->RunFileDialog( + config_.mode, config_.title, config_.default_file_name, + config_.accept_filters, new Callback(this)); } void ExecuteCallback(CefRefPtr callback) { @@ -94,18 +101,58 @@ class DialogTestHandler : public TestHandler { FileDialogMode mode, const CefString& title, const CefString& default_file_name, - const std::vector& accept_types, + const std::vector& accept_filters, + const std::vector& accept_extensions, + const std::vector& accept_descriptions, CefRefPtr callback) override { - got_onfiledialog_.yes(); + got_onfiledialog_ct_++; std::string url = browser->GetMainFrame()->GetURL(); EXPECT_STREQ(kTestUrl, url.c_str()); EXPECT_EQ(config_.mode, mode); EXPECT_STREQ(config_.title.ToString().c_str(), title.ToString().c_str()); - EXPECT_STREQ(config_.default_file_name.ToString().c_str(), - default_file_name.ToString().c_str()); - TestStringVectorEqual(config_.accept_types, accept_types); + + EXPECT_EQ(accept_filters.size(), accept_extensions.size()); + EXPECT_EQ(accept_filters.size(), accept_descriptions.size()); + TestStringVectorEqual(config_.accept_filters, accept_filters); + + if (got_onfiledialog_ct_ == 1U) { + // On the 2nd+ call this will be set to the last opened path value + // (possibly leftover from a different test). + EXPECT_STREQ(config_.default_file_name.ToString().c_str(), + default_file_name.ToString().c_str()); + + TestStringVectorEqual(config_.accept_extensions, accept_extensions); + + // All descriptions should be empty. + for (auto& desc : accept_descriptions) { + EXPECT_TRUE(desc.empty()); + } + + if (config_.skip_first_callback) { + return false; + } + } else if (got_onfiledialog_ct_ == 2U) { + // All MIME types should be resolved to file extensions. + // A description should be provided for MIME types only. + for (size_t i = 0; i < accept_filters.size(); ++i) { + const std::string filter = accept_filters[i]; + const std::string ext = accept_extensions[i]; + const std::string desc = accept_descriptions[i]; + if (filter == "image/*") { + EXPECT_TRUE(ext.find(".png") > 0); + EXPECT_TRUE(ext.find(".jpg") > 0); + EXPECT_FALSE(desc.empty()); + } else { + // Single file extensions should match. + EXPECT_STREQ(filter.data(), ext.data()); + EXPECT_TRUE(desc.empty()); + } + } + } else { + NOTREACHED(); + } if (config_.callback_async) { CefPostTask(TID_UI, base::BindOnce(&DialogTestHandler::ExecuteCallback, @@ -118,15 +165,19 @@ class DialogTestHandler : public TestHandler { } void DestroyTest() override { - EXPECT_TRUE(got_onfiledialog_); + if (config_.skip_first_callback) { + EXPECT_EQ(2U, got_onfiledialog_ct_); + } else { + EXPECT_EQ(1U, got_onfiledialog_ct_); + } EXPECT_TRUE(got_onfiledialogdismissed_); TestHandler::DestroyTest(); } - TestConfig config_; + const TestConfig& config_; - TrackCallback got_onfiledialog_; + size_t got_onfiledialog_ct_ = 0; TrackCallback got_onfiledialogdismissed_; IMPLEMENT_REFCOUNTING(DialogTestHandler); @@ -139,7 +190,8 @@ TEST(DialogTest, FileEmptyParams) { DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); config.title.clear(); config.default_file_name.clear(); - config.accept_types.clear(); + config.accept_filters.clear(); + config.accept_extensions.clear(); config.callback_async = false; config.callback_cancel = false; @@ -159,6 +211,18 @@ TEST(DialogTest, FileOpen) { ReleaseAndWaitForDestructor(handler); } +TEST(DialogTest, FileOpenSkipFirstCallback) { + DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); + config.callback_async = false; + config.callback_cancel = false; + config.callback_paths.push_back("/path/to/file1.txt"); + config.skip_first_callback = true; + + CefRefPtr handler = new DialogTestHandler(config); + handler->ExecuteTest(); + ReleaseAndWaitForDestructor(handler); +} + TEST(DialogTest, FileOpenCancel) { DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); config.callback_async = false; @@ -169,6 +233,17 @@ TEST(DialogTest, FileOpenCancel) { ReleaseAndWaitForDestructor(handler); } +TEST(DialogTest, FileOpenCancelSkipFirstCallback) { + DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); + config.callback_async = false; + config.callback_cancel = true; + config.skip_first_callback = true; + + CefRefPtr handler = new DialogTestHandler(config); + handler->ExecuteTest(); + ReleaseAndWaitForDestructor(handler); +} + TEST(DialogTest, FileOpenAsync) { DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); config.callback_async = true; @@ -180,9 +255,21 @@ TEST(DialogTest, FileOpenAsync) { ReleaseAndWaitForDestructor(handler); } +TEST(DialogTest, FileOpenAsyncSkipFirstCallback) { + DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); + config.callback_async = true; + config.callback_cancel = false; + config.callback_paths.push_back("/path/to/file1.txt"); + config.skip_first_callback = true; + + CefRefPtr handler = new DialogTestHandler(config); + handler->ExecuteTest(); + ReleaseAndWaitForDestructor(handler); +} + TEST(DialogTest, FileOpenAsyncCancel) { DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); - config.callback_async = false; + config.callback_async = true; config.callback_cancel = true; CefRefPtr handler = new DialogTestHandler(config); @@ -190,6 +277,17 @@ TEST(DialogTest, FileOpenAsyncCancel) { ReleaseAndWaitForDestructor(handler); } +TEST(DialogTest, FileOpenAsyncCancelSkipFirstCallback) { + DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN); + config.callback_async = true; + config.callback_cancel = true; + config.skip_first_callback = true; + + CefRefPtr handler = new DialogTestHandler(config); + handler->ExecuteTest(); + ReleaseAndWaitForDestructor(handler); +} + TEST(DialogTest, FileOpenMultiple) { DialogTestHandler::TestConfig config(FILE_DIALOG_OPEN_MULTIPLE); config.callback_async = false; diff --git a/tests/ceftests/test_util.cc b/tests/ceftests/test_util.cc index 50b1513d5..2dffb0d6c 100644 --- a/tests/ceftests/test_util.cc +++ b/tests/ceftests/test_util.cc @@ -277,6 +277,9 @@ void TestProcessMessageEqual(CefRefPtr val1, void TestStringVectorEqual(const std::vector& val1, const std::vector& val2) { EXPECT_EQ(val1.size(), val2.size()); + if (val1.size() != val2.size()) { + return; + } for (size_t i = 0; i < val1.size(); ++i) { EXPECT_STREQ(val1[i].ToString().c_str(), val2[i].ToString().c_str()); diff --git a/tests/shared/common/string_util.cc b/tests/shared/common/string_util.cc index 7bd5ada83..3a1cc017d 100644 --- a/tests/shared/common/string_util.cc +++ b/tests/shared/common/string_util.cc @@ -5,6 +5,8 @@ #include "tests/shared/common/string_util.h" #include +#include +#include namespace client { @@ -31,4 +33,16 @@ std::string AsciiStrReplace(const std::string& str, return result; } +std::vector AsciiStrSplit(const std::string& str, char delim) { + std::vector result; + std::stringstream ss(str); + std::string item; + + while (getline(ss, item, delim)) { + result.push_back(item); + } + + return result; +} + } // namespace client diff --git a/tests/shared/common/string_util.h b/tests/shared/common/string_util.h index 5ae4e2c31..d244b02cf 100644 --- a/tests/shared/common/string_util.h +++ b/tests/shared/common/string_util.h @@ -7,6 +7,7 @@ #pragma once #include +#include namespace client { @@ -18,6 +19,9 @@ std::string AsciiStrReplace(const std::string& str, const std::string& from, const std::string& to); +// Split |str| at character |delim|. +std::vector AsciiStrSplit(const std::string& str, char delim); + } // namespace client #endif // CEF_TESTS_SHARED_COMMON_STRING_UTIL_H_