Commit c97cb6fa authored by Raphael Kubo da Costa's avatar Raphael Kubo da Costa Committed by Commit Bot

libgtkui: Use application/octet-stream as the last option in the KDE code

We were previously using an std::set<std::string> to filter out mime type
duplicates, optionally adding the "application/octet-stream" mime type to
the set before joining all items as a single string to pass to the
kdialog invocation.

This used to work fine with the KDE4-based kdialog, whose underlying
KFileDialog ended up creating a custom file type filter with all entries and
suggesting the proper file extension. The KDE Frameworks 5-based kdialog
that was released a few months ago uses a simple QFileDialog, and the
filter's entries are added in the order they are passed to kdialog.

In practice, this means that downloading a PDF file (or any file whose mime
type ended up coming after "application/octet-stream" when iterating our
std::set) causes kdialog to be invoked like this:

    kdialog [...] --getsavefilename /path/to/Downloads/foo.pdf \
                  application/octet-stream application/pdf

KDE4 kdialog suggests "foo.pdf" as the name and adds "unknown, PDF
document", "unknown" and "PDF document" to the filter list. KF5 kdialog
suggests "foo.bin" as the name and adds "unknown" and "PDF document" to the
filter list.

We now make sure that "application/octet-stream" is the last mime type we
pass to kdialog so that any other more specific mime type is chosen as the
default and we do not always try to add the ".bin" extension to the files we
are saving.

Bug: 752375
Change-Id: I56a458042823c52beada9c1819c2ee4b8b8e5e30
Reviewed-on: https://chromium-review.googlesource.com/891160
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: default avatarElliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532556}
parent 1f38693c
...@@ -285,7 +285,6 @@ bool SelectFileDialogImplKDE::HasMultipleFileTypeChoicesImpl() { ...@@ -285,7 +285,6 @@ bool SelectFileDialogImplKDE::HasMultipleFileTypeChoicesImpl() {
std::string SelectFileDialogImplKDE::GetMimeTypeFilterString() { std::string SelectFileDialogImplKDE::GetMimeTypeFilterString() {
DCHECK(pipe_task_runner_->RunsTasksInCurrentSequence()); DCHECK(pipe_task_runner_->RunsTasksInCurrentSequence());
std::string filter_string;
// We need a filter set because the same mime type can appear multiple times. // We need a filter set because the same mime type can appear multiple times.
std::set<std::string> filter_set; std::set<std::string> filter_set;
for (size_t i = 0; i < file_types_.extensions.size(); ++i) { for (size_t i = 0; i < file_types_.extensions.size(); ++i) {
...@@ -297,17 +296,16 @@ std::string SelectFileDialogImplKDE::GetMimeTypeFilterString() { ...@@ -297,17 +296,16 @@ std::string SelectFileDialogImplKDE::GetMimeTypeFilterString() {
} }
} }
} }
std::vector<std::string> filter_vector(filter_set.cbegin(),
filter_set.cend());
// Add the *.* filter, but only if we have added other filters (otherwise it // Add the *.* filter, but only if we have added other filters (otherwise it
// is implied). // is implied). It needs to be added last to avoid being picked as the default
if (file_types_.include_all_files && !file_types_.extensions.empty()) // filter.
filter_set.insert("application/octet-stream"); if (file_types_.include_all_files && !file_types_.extensions.empty()) {
// Create the final output string. DCHECK(filter_set.find("application/octet-stream") == filter_set.end());
filter_string.clear(); filter_vector.push_back("application/octet-stream");
for (std::set<std::string>::iterator it = filter_set.begin();
it != filter_set.end(); ++it) {
filter_string.append(*it + " ");
} }
return filter_string; return base::JoinString(filter_vector, " ");
} }
std::unique_ptr<SelectFileDialogImplKDE::KDialogOutputParams> std::unique_ptr<SelectFileDialogImplKDE::KDialogOutputParams>
......
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