Allow feedback form to be shown in an App Launcher feedback mode.

chrome/browser/ui/webui/feedback_ui.cc

 // Copyright (c) 2012 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/ui/webui/feedback_ui.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 185 matching lines @@
   // for this functionality since we need to make *sure* we never get
   // instantiated again while we are open - with singleton tabs, that can
   // happen).
   int feedback_tab_index = GetIndexOfFeedbackTab(browser);
   if (feedback_tab_index >= 0) {
     // Do not refresh screenshot, do not create a new tab.
     browser->tab_strip_model()->ActivateTabAt(feedback_tab_index, true);
     return;
   }
 
-  std::vector<unsigned char>* last_screenshot_png =
-      FeedbackUtil::GetScreenshotPng();
-  last_screenshot_png->clear();
+  if (category_tag != kAppLauncherCategoryTag) {
+    std::vector<unsigned char>* last_screenshot_png =
+        FeedbackUtil::GetScreenshotPng();
+    last_screenshot_png->clear();
 
-  gfx::NativeWindow native_window;
-  gfx::Rect snapshot_bounds;
+    gfx::NativeWindow native_window;
+    gfx::Rect snapshot_bounds;
 
 #if defined(OS_CHROMEOS)
-  // For ChromeOS, don't use the browser window but the root window
-  // instead to grab the screenshot. We want everything on the screen, not
-  // just the current browser.
-  native_window = ash::Shell::GetPrimaryRootWindow();
-  snapshot_bounds = gfx::Rect(native_window->bounds());
+    // For ChromeOS, don't use the browser window but the root window
+    // instead to grab the screenshot. We want everything on the screen, not
+    // just the current browser.
+    native_window = ash::Shell::GetPrimaryRootWindow();
+    snapshot_bounds = gfx::Rect(native_window->bounds());
 #else
-  native_window = browser->window()->GetNativeWindow();
-  snapshot_bounds = gfx::Rect(browser->window()->GetBounds().size());
+    native_window = browser->window()->GetNativeWindow();
+    snapshot_bounds = gfx::Rect(browser->window()->GetBounds().size());
 #endif
-  bool success = chrome::GrabWindowSnapshotForUser(native_window,
-                                                   last_screenshot_png,
-                                                   snapshot_bounds);
-  FeedbackUtil::SetScreenshotSize(success ? snapshot_bounds : gfx::Rect());
-
+    bool success = chrome::GrabWindowSnapshotForUser(native_window,
+                                                     last_screenshot_png,
+                                                     snapshot_bounds);
+    FeedbackUtil::SetScreenshotSize(success ? snapshot_bounds : gfx::Rect());
+  }
   std::string feedback_url = std::string(chrome::kChromeUIFeedbackURL) + "?" +
       kSessionIDParameter + base::IntToString(browser->session_id().id()) +
       "&" + kTabIndexParameter +
       base::IntToString(browser->tab_strip_model()->active_index()) +
       "&" + kDescriptionParameter +
       net::EscapeUrlEncodedData(description_template, false) + "&" +
       kCategoryTagParameter + net::EscapeUrlEncodedData(category_tag, false);
 
 #if defined(OS_CHROMEOS)
   feedback_url = feedback_url + "&" + kTimestampParameter +
@@ skipping 37 matching lines @@
   void ClobberScreenshotsSource();
 
   void CancelFeedbackCollection();
   void CloseFeedbackTab();
 
   WebContents* tab_;
   ScreenshotSource* screenshot_source_;
 
   FeedbackData* feedback_data_;
   std::string target_tab_url_;
+  std::string category_tag_;
 #if defined(OS_CHROMEOS)
   // Variables to track SyslogsProvider::RequestSyslogs.
   CancelableTaskTracker::TaskId syslogs_task_id_;
   CancelableTaskTracker syslogs_tracker_;
 
   // Timestamp of when the feedback request was initiated.
   std::string timestamp_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(FeedbackHandler);
@@ skipping 38 matching lines @@
 #endif
   source->AddLocalizedString("noscreenshot",
                              IDS_FEEDBACK_INCLUDE_NO_SCREENSHOT);
 
   source->AddLocalizedString("send-report", IDS_FEEDBACK_SEND_REPORT);
   source->AddLocalizedString("cancel", IDS_CANCEL);
 
   source->AddLocalizedString("no-description", IDS_FEEDBACK_NO_DESCRIPTION);
   source->AddLocalizedString("no-saved-screenshots",
                              IDS_FEEDBACK_NO_SAVED_SCREENSHOTS_HELP);
   source->AddLocalizedString("privacy-note", IDS_FEEDBACK_PRIVACY_NOTE);

[Dan Beam, 2013/03/15 15:29:51]

^ I guess you guy are already using i18n-keys-with-dashes, so my mention of
i18nKeysAsCamelCase isn't a huge deal... it does differ from much of the other
webui code, though, IIRC

+  source->AddLocalizedString("launcher-title", IDS_FEEDBACK_LAUNCHER_TITLE);
+  source->AddLocalizedString("launcher-description",
+                             IDS_FEEDBACK_LAUNCHER_DESCRIPTION_LABEL);
 
   source->SetJsonPath("strings.js");
   source->AddResourcePath("feedback.js", IDR_FEEDBACK_JS);
   source->SetDefaultResource(
       successful_init ? IDR_FEEDBACK_HTML : IDR_FEEDBACK_HTML_INVALID);
 
   return source;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ skipping 73 matching lines @@
         if (!base::StringToInt(query_str, &index))
           return false;
         continue;
       }
       if (StartsWithASCII(*it, std::string(kCustomPageUrlParameter), true)) {
         ReplaceFirstSubstringAfterOffset(
             &query_str, 0, kCustomPageUrlParameter, "");
         custom_page_url = query_str;
         continue;
       }
+      if (StartsWithASCII(*it, std::string(kCategoryTagParameter), true)) {
+        ReplaceFirstSubstringAfterOffset(
+            &query_str, 0, kCategoryTagParameter, "");
+        category_tag_ = query_str;
+        continue;
+      }

[Dan Beam, 2013/03/15 15:29:51]

what's the difference between if () { continue; } if () { continue; } and if ()
{ } else if () { }?

[benwells, 2013/03/18 05:21:19]

I'm not sure there is any difference. TBH I just followed the local pattern, but
this way uses more lines.

 #if defined(OS_CHROMEOS)
       if (StartsWithASCII(*it, std::string(kTimestampParameter), true)) {
         ReplaceFirstSubstringAfterOffset(
             &query_str, 0, kTimestampParameter, "");
         timestamp_ = query_str;
         continue;

[Dan Beam, 2013/03/18 06:09:50]

also, there's a continue at the very bottom of the loop... that makes no sense
to me...

[benwells, 2013/03/19 02:39:54]

It's another form of if / else. The continue at the end is equivalent to having
a comma after the last item in a list: it lets someone come later and copy
another block to the end, and it just works.

I've converted this all to an if / else block.

       }
 #endif
     }
   }
 
   // If we don't have a page url specified, get it from the tab index.
   if (custom_page_url.empty()) {
     if (session_id == -1)
       return false;
 
@@ skipping 43 matching lines @@
                  base::Unretained(this)));
 }
 
 void FeedbackHandler::HandleGetDialogDefaults(const ListValue*) {
   // Will delete itself when feedback_data_->SendReport() is called.
   feedback_data_ = new FeedbackData();
 
   // Send back values which the dialog js needs initially.
   DictionaryValue dialog_defaults;
 
+  if (category_tag_ == chrome::kAppLauncherCategoryTag)
+    dialog_defaults.SetBoolean("launcherFeedback", true);
+
   // Current url.
   dialog_defaults.SetString("currentUrl", target_tab_url_);
 
   // Are screenshots disabled?
   dialog_defaults.SetBoolean(
       "disableScreenshots",
       g_browser_process->local_state()->GetBoolean(prefs::kDisableScreenshots));
 
   // User e-mail
   std::string user_email = GetUserEmail();
@@ skipping 131 matching lines @@
         LOG(ERROR) << "Unable to attach file: " << attached_filename;
         // Clear the filename so feedback_util doesn't try to attach the file.
         attached_filename = "";
       }
     }
   }
 #endif
 
   // Update the data in feedback_data_ so it can be sent
   feedback_data_->UpdateData(Profile::FromWebUI(web_ui())
                              , std::string()

[rkc, 2013/03/15 16:58:40]

Might want to actually send the category tag :)

Looks like a bug that never got caught since on the feedback backend we really
don't do anything with the category tag.

[benwells, 2013/03/18 05:21:19]

Woops, good catch. Done.

                              , page_url
                              , description
                              , user_email
                              , image_ptr
 #if defined(OS_CHROMEOS)
                              , send_sys_info
                              , false  // sent_report
                              , timestamp_
                              , attached_filename
                              , attached_filedata
@@ skipping 93 matching lines @@
                     screenshot_filepaths.begin() + sort_size,
                     screenshot_filepaths.end(),
                     ScreenshotTimestampComp);
   for (size_t i = 0; i < sort_size; ++i)
     saved_screenshots->push_back(
         std::string(ScreenshotSource::kScreenshotUrlRoot) +
         std::string(ScreenshotSource::kScreenshotSaved) +
         screenshot_filepaths[i]);
 }
 #endif