Save Page As MHTML

chrome/browser/download/save_package_file_picker.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/save_package_file_picker.h"
 
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/prefs/pref_member.h"
@@ skipping 15 matching lines @@
 
 // If false, we don't prompt the user as to where to save the file.  This
 // exists only for testing.
 bool g_should_prompt_for_filename = true;
 
 // Used for mapping between SavePageType constants and the indexes above.
 const SavePageType kIndexToSaveType[] = {
   content::SAVE_PAGE_TYPE_UNKNOWN,
   content::SAVE_PAGE_TYPE_AS_ONLY_HTML,
   content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML,
+  content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML_SINGLE_FILE_MHTML,

[achuithb, 2012/04/12 19:40:02]

Just asking here - does this need to be so long? Isn't SAVE_PAGE_TYPE_AS_MHTML
sufficient?

[benjhayden, 2012/04/13 20:44:11]

Done.

 };
 
 int SavePackageTypeToIndex(SavePageType type) {
   for (size_t i = 0; i < arraysize(kIndexToSaveType); ++i) {
     if (kIndexToSaveType[i] == type)
       return i;
   }
   NOTREACHED();
   return -1;
 }
 
 // Indexes used for specifying which element in the extensions dropdown
 // the user chooses when picking a save type.
 const int kSelectFileHtmlOnlyIndex = 1;
 const int kSelectFileCompleteIndex = 2;
+const int kSelectFileCompleteSingleFileIndex = 3;
 
 // Used for mapping between the IDS_ string identifiers and the indexes above.
 const int kIndexToIDS[] = {
-  0, IDS_SAVE_PAGE_DESC_HTML_ONLY, IDS_SAVE_PAGE_DESC_COMPLETE,
+  0,
+  IDS_SAVE_PAGE_DESC_HTML_ONLY,
+  IDS_SAVE_PAGE_DESC_COMPLETE,
+  IDS_SAVE_PAGE_DESC_COMPLETE_SINGLE_FILE,
 };
 
 }
 
 SavePackageFilePicker::SavePackageFilePicker(
     content::WebContents* web_contents,
     const FilePath& suggested_path,
     const FilePath::StringType& default_extension,
     bool can_save_as_complete,
     DownloadPrefs* download_prefs,
     content::SaveFilePathPickedCallback callback)
     : render_process_id_(web_contents->GetRenderProcessHost()->GetID()),
       callback_(callback) {
   int file_type_index = SavePackageTypeToIndex(
       static_cast<SavePageType>(download_prefs->save_file_type()));
   DCHECK_NE(-1, file_type_index);
 
   SelectFileDialog::FileTypeInfo file_type_info;
 
   // If the contents can not be saved as complete-HTML, do not show the
   // file filters.
   if (can_save_as_complete) {
     bool add_extra_extension = false;
     FilePath::StringType extra_extension;
     if (!suggested_path.Extension().empty() &&
         suggested_path.Extension().compare(FILE_PATH_LITERAL("htm")) &&
         suggested_path.Extension().compare(FILE_PATH_LITERAL("html"))) {

[Randy Smith (Not in Mondays), 2012/04/12 18:00:02]

I may be being naive, but doesn't the logic as written mean that if the
suggested file name is .htm/.html that'll show up as an option for the .mht
save?  I'd think you'd want the .mht options as exclusions here as well?

[benjhayden, 2012/04/13 20:44:11]

Sorry, I don't quite understand.
I don't think that suggested_path will ever be mht.
To be sure, there are a few bugs in the most recent patch set's file picker's
handling of mhtml. I just don't know how to deal with them.

[Randy Smith (Not in Mondays), 2012/04/16 17:45:47]

I guess the question is whether you're sure that gun was really fired six times
:-}.  The suggested_path logic is convoluted, the internet is a cesspool, and I
suspect the behavior is "undefined" if the file picker is passed two separate
extension types where there's an overlap in strings between them.  So it would
seem worth protecting against to me.  
Well, you've got me curious.  What's up?  (Feel free to ignore this question if
you'd rather spend them time, say, fixing the bugs :-}).

[benjhayden, 2012/04/16 19:08:05]

Sorry, you're right, the suggested_path can be .mht/.mhtml if the URL ends with
.mht/.mhtml.

However, it appears that the extensions vector is only used (at least on gtk) to
filter which existing files are displayed. So I'm still not sure what you mean
by "if the suggested file name is .htm/.html that'll show up as an option for
the .mht save".
I think that hiding the extension will effectively deal with the issues.

Ah, however, it breaks the deduplication...

       add_extra_extension = true;
       extra_extension = suggested_path.Extension().substr(1);
     }
 
-    file_type_info.extensions.resize(2);
+    file_type_info.extensions.resize(3);
+
+    file_type_info.extension_description_overrides.push_back(
+        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileHtmlOnlyIndex]));

[Randy Smith (Not in Mondays), 2012/04/12 18:00:02]

Why did you need to add an override here?  (To be fair, if I look at the code
the way it used to be, it looks like the select file complete override was added
at the select file html only index location--did it used to be broken?  If so,
that answers my question.)

[benjhayden, 2012/04/13 20:44:11]

I didn't actually change any functionality here.
I just moved this override up a few lines and changed how it's spelled: from
'kSelectFileCompleteIndex - 1' to 'kSelectFileHtmlOnlyIndex'.

     file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
         FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
         FILE_PATH_LITERAL("html"));
-
     if (add_extra_extension) {
       file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
           extra_extension);
     }
 
+

[Randy Smith (Not in Mondays), 2012/04/12 18:00:02]

nit: Remove extra blank line.

[benjhayden, 2012/04/13 20:44:11]

Done.

     file_type_info.extension_description_overrides.push_back(
-        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileCompleteIndex - 1]));
+        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileCompleteIndex]));
     file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
         FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
         FILE_PATH_LITERAL("html"));
-
     if (add_extra_extension) {
       file_type_info.extensions[kSelectFileCompleteIndex - 1].push_back(
           extra_extension);
     }
 
     file_type_info.extension_description_overrides.push_back(
-        l10n_util::GetStringUTF16(kIndexToIDS[kSelectFileCompleteIndex]));
+        l10n_util::GetStringUTF16(kIndexToIDS[
+          kSelectFileCompleteSingleFileIndex]));
+    file_type_info.extensions[kSelectFileCompleteSingleFileIndex - 1].push_back(
+        FILE_PATH_LITERAL("mht"));
+    file_type_info.extensions[kSelectFileCompleteSingleFileIndex - 1].push_back(
+        FILE_PATH_LITERAL("mhtml"));
+    if (add_extra_extension) {
+      file_type_info.extensions[
+        kSelectFileCompleteSingleFileIndex - 1].push_back(extra_extension);
+    }
+
     file_type_info.include_all_files = false;
   } else {
     file_type_info.extensions.resize(1);
     file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1].push_back(
         suggested_path.Extension());
 
     if (!file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1][0].empty()) {
       // Drop the .
       file_type_info.extensions[kSelectFileHtmlOnlyIndex - 1][0].erase(0, 1);
     }
@@ skipping 25 matching lines @@
 
 void SavePackageFilePicker::SetShouldPromptUser(bool should_prompt) {
   g_should_prompt_for_filename = should_prompt;
 }
 
 void SavePackageFilePicker::FileSelected(const FilePath& path,
                                          int index,
                                          void* params) {
   // The option index is not zero-based.
   DCHECK(index >= kSelectFileHtmlOnlyIndex &&
-         index <= kSelectFileCompleteIndex);
+         index <= kSelectFileCompleteSingleFileIndex);
 
   RenderProcessHost* process = RenderProcessHost::FromID(render_process_id_);
   if (process) {
     SavePageType save_type = kIndexToSaveType[index];
     Profile* profile =
         Profile::FromBrowserContext(process->GetBrowserContext());
     PrefService* prefs = profile->GetPrefs();
     if (select_file_dialog_ &&
         select_file_dialog_->HasMultipleFileTypeChoices())
       prefs->SetInteger(prefs::kSaveFileType, save_type);
@@ skipping 14 matching lines @@
 
     callback_.Run(path, save_type);
   }
 
   delete this;
 }
 
 void SavePackageFilePicker::FileSelectionCanceled(void* params) {
   delete this;
 }