Add the user email field to Chrome.

Add the user email field for Chrome on all other platforms (ChromeOS already has it); also make the field editable.

R=estade@chromium.org
BUG=94149
TEST=Send feedback reports from all 4 platforms, from both incognito and regular modes, making sure that the user e-mail is populated correctly and reported to the Feedback server correctly.


Review URL: https://chromiumcodereview.appspot.com/10832078

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149346 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/feedback/feedback_data.cc

@@ -31,9 +31,9 @@ void FeedbackData::UpdateData(Profile* profile,
                                const std::string& category_tag,
                                const std::string& page_url,
                                const std::string& description,
+                               const std::string& user_email,
                                ScreenshotDataPtr image
 #if defined(OS_CHROMEOS)
-                               , const std::string& user_email
                                , const bool send_sys_info
                                , const bool sent_report
                                , const std::string& timestamp
@@ -45,9 +45,9 @@ void FeedbackData::UpdateData(Profile* profile,
   category_tag_ = category_tag;
   page_url_ = page_url;
   description_ = description;
+  user_email_ = user_email;
   image_ = image;
 #if defined(OS_CHROMEOS)
-  user_email_ = user_email;
   send_sys_info_ = send_sys_info;
   sent_report_ = sent_report;
   timestamp_ = timestamp;
@@ -69,11 +69,11 @@ void FeedbackData::SendReport() {
                             , category_tag_
                             , page_url_
                             , description_
+                            , user_email_
                             , image_
                             , screen_size.width()
                             , screen_size.height()
 #if defined(OS_CHROMEOS)
-                            , user_email_
                             , zip_content_ ? zip_content_->c_str() : NULL
                             , zip_content_ ? zip_content_->length() : 0
                             , send_sys_info_ ? sys_info_ : NULL

chrome/browser/feedback/feedback_data.h

@@ -29,9 +29,9 @@ class FeedbackData {
                   const std::string& category_tag,
                   const std::string& page_url,
                   const std::string& description,
+                  const std::string& user_email,
                   ScreenshotDataPtr image
 #if defined(OS_CHROMEOS)
-                  , const std::string& user_email
                   , const bool send_sys_info
                   , const bool sent_report
                   , const std::string& timestamp
@@ -48,9 +48,9 @@ class FeedbackData {
   const std::string& category_tag() const { return category_tag_; }
   const std::string& page_url() const { return page_url_; }
   const std::string& description() const { return description_; }
+  const std::string& user_email() const { return user_email_; }
   ScreenshotDataPtr image() const { return image_; }
 #if defined(OS_CHROMEOS)
-  const std::string& user_email() const { return user_email_; }
   chromeos::system::LogDictionaryType* sys_info() const { return sys_info_; }
   bool send_sys_info() const { return send_sys_info_; }
   bool sent_report() const { return sent_report_; }
@@ -67,11 +67,11 @@ class FeedbackData {
   std::string category_tag_;
   std::string page_url_;
   std::string description_;
+  std::string user_email_;
   ScreenshotDataPtr image_;
 
 #if defined(OS_CHROMEOS)
   // Chromeos specific values for SendReport.
-  std::string user_email_;
   chromeos::system::LogDictionaryType* sys_info_;
   // Content of the compressed system logs.
   std::string* zip_content_;

chrome/browser/feedback/feedback_util.cc

@@ -246,11 +246,11 @@ void FeedbackUtil::SendReport(
     , const std::string& category_tag
     , const std::string& page_url_text
     , const std::string& description
+    , const std::string& user_email_text
     , ScreenshotDataPtr image_data_ptr
     , int png_width
     , int png_height
 #if defined(OS_CHROMEOS)
-    , const std::string& user_email_text
     , const char* zipped_logs_data
     , int zipped_logs_length
     , const chromeos::system::LogDictionaryType* const sys_info
@@ -271,10 +271,8 @@ void FeedbackUtil::SendReport(
   // they wish
   common_data->set_gaia_id(0);
 
-#if defined(OS_CHROMEOS)
   // Add the user e-mail to the feedback object
   common_data->set_user_email(user_email_text);
-#endif
 
   // Add the description to the feedback object
   common_data->set_description(description);

chrome/browser/feedback/feedback_util.h

@@ -68,11 +68,11 @@ class FeedbackUtil {
       , const std::string& category_tag
       , const std::string& page_url_text
       , const std::string& description
+      , const std::string& user_email_text
       , ScreenshotDataPtr png_data
       , int png_width
       , int png_height
 #if defined(OS_CHROMEOS)
-      , const std::string& user_email_text
       , const char* zipped_logs_data
       , int zipped_logs_length
       , const chromeos::system::LogDictionaryType* const sys_info

chrome/browser/resources/feedback.css

@@ -30,23 +30,21 @@
   width: 100%;
 }
 
-#page-url {
+.input-text-container {
   -webkit-box-align: baseline;
   display: -webkit-box;
 }
 
-#page-url-text {
-  /* Don't push checkboxes out of alignment with bulky input. */
-  margin-bottom: -0.25em;
-}
-
-#page-url > label {
+.input-text-container > label {
   -webkit-margin-end: 1em;
+  width: 150px;
 }
 
-#page-url-text {
+.input-text-container > input {
   -webkit-box-flex: 1;
   display: block;
+  /* Don't push checkboxes out of alignment with bulky input. */
+  margin-bottom: -0.25em;
 }
 
 .thumbnail-list {

chrome/browser/resources/feedback.html

@@ -18,7 +18,7 @@
   <div id="content">
     <span id="description" colspan="2" i18n-content="description"></span>
     <textarea id="description-text" rows="10"></textarea>
-    <div id="page-url" class="checkbox">
+    <div id="page-url" class="input-text-container checkbox">
       <label>
         <input id="page-url-checkbox" type="checkbox"
             value="pageurl" checked>
@@ -26,19 +26,17 @@
       </label>
       <input id="page-url-text">
     </div>
-<if expr="pp_ifdef('chromeos')">
     <!-- User e-mail -->
-    <div id="user-email-table">
-      <div class="checkbox">
-        <label>
-          <input id="user-email-checkbox" type="checkbox" checked>
-          <span id="user-email-label" i18n-content="user-email"></span>
-        </label>
-        <span id="user-email-text"></span>
-      </div>
+    <div id="user-email" class="input-text-container checkbox">
+      <label>
+        <input id="user-email-checkbox" type="checkbox" checked>
+        <span id="user-email-label" i18n-content="user-email"></span>
+      </label>
+      <input id="user-email-text">
     </div>
+<if expr="pp_ifdef('chromeos')">
     <!-- System Information -->
-    <div class="feedback-fieldlabel checkbox">
+    <div class="checkbox">
       <label>
         <input id="sys-info-checkbox" type="checkbox" checked>
         <span id="sysinfo-label">
@@ -49,7 +47,7 @@
 </if>
     <!-- Screenshot -->
     <div id="screenshot-row" hidden>
-      <div class="feedback-fieldlabel checkbox">
+      <div class="checkbox">
         <label>
           <input id="screenshot-checkbox" type="checkbox">
           <span id="screenshot-label-current"
@@ -59,7 +57,7 @@
               i18n-content="saved-screenshot" hidden></span>
         </label>
         <a id="screenshot-link-tosaved" href="#"
-            i18n-content="choose-different-screenshot">
+            i18n-content="choose-different-screenshot" hidden>
         </a>
         <a id="screenshot-link-tocurrent" href="#"
             i18n-content="choose-original-screenshot" hidden>

chrome/browser/resources/feedback.js

@@ -101,19 +101,19 @@ function sendReport() {
   var pageUrl = $('page-url-text').value;
   if (!$('page-url-checkbox').checked)
     pageUrl = '';
+  var userEmail = $('user-email-text').value;
+  if (!$('user-email-checkbox').checked)
+    userEmail = '';
 
   var reportArray = [pageUrl,
                      categoryTag,
                      $('description-text').value,
+                     userEmail,
                      imagePath];
 
   // Add chromeos data if it exists.
-  if ($('user-email-text') && $('sys-info-checkbox')) {
-    var userEmail = $('user-email-text').textContent;
-    if (!$('user-email-checkbox').checked)
-      userEmail = '';
-    reportArray = reportArray.concat([userEmail,
-                                      String($('sys-info-checkbox').checked)]);
+  if ($('sys-info-checkbox')) {
+    reportArray = reportArray.concat([String($('sys-info-checkbox').checked)]);
   }
 
   // open the landing page in a new tab, sendReport will close this one.
@@ -282,23 +282,15 @@ function setupDialogDefaults(defaults) {
     $('page-url-text').value = defaults.currentUrl;
   if (defaults.currentUrl == '')
     $('page-url-checkbox').checked = false;
+  // User e-mail.
+  $('user-email-text').value = defaults.userEmail;
 
   // Are screenshots disabled?
   if (!defaults.disableScreenshots)
     enableScreenshots();
 
-  // User e-mail.
-  if ($('user-email-text')) {
-    // We're in Chromium OS.
-    $('user-email-text').textContent = defaults.userEmail;
-    if (defaults.userEmail == '') {
-      // if we didn't get an e-mail address from cros,
-      // disable the user email display totally.
-      $('user-email-table').hidden = true;
-
-      // this also means we are in privacy mode, so no saved screenshots.
-      $('screenshot-link-tosaved').hidden = true;
-    }
+  if (defaults.useSaved) {
+    $('screenshot-link-tosaved').hidden = false;
   }
 }
 

chrome/browser/ui/webui/feedback_ui.cc

@@ -22,6 +22,9 @@
 #include "chrome/browser/feedback/feedback_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/signin/signin_manager.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -114,6 +117,20 @@ std::string GetUserEmail() {
     return manager->GetLoggedInUser().display_email();
 }
 
+#else
+
+std::string GetUserEmail() {
+  Profile* profile = ProfileManager::GetLastUsedProfile();
+  if (!profile)
+    return std::string();
+
+  SigninManager* signin = SigninManagerFactory::GetForProfile(profile);
+  if (!signin)
+    return std::string();
+
+  return signin->GetAuthenticatedUsername();
+}
+
 #endif  // OS_CHROMEOS
 
 // Returns the index of the feedback tab if already open, -1 otherwise
@@ -252,8 +269,9 @@ ChromeWebUIDataSource* CreateFeedbackUIHTMLSource(bool successful_init) {
                              IDS_FEEDBACK_SCREENSHOT_LABEL);
   source->AddLocalizedString("saved-screenshot",
                              IDS_FEEDBACK_SAVED_SCREENSHOT_LABEL);
-#if defined(OS_CHROMEOS)
   source->AddLocalizedString("user-email", IDS_FEEDBACK_USER_EMAIL_LABEL);
+
+#if defined(OS_CHROMEOS)
   source->AddLocalizedString("sysinfo",
                              IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX);
   source->AddLocalizedString("currentscreenshots",
@@ -453,6 +471,10 @@ void FeedbackHandler::HandleGetDialogDefaults(const ListValue*) {
       "disableScreenshots",
       g_browser_process->local_state()->GetBoolean(prefs::kDisableScreenshots));
 
+  // User e-mail
+  std::string user_email = GetUserEmail();
+  dialog_defaults.SetString("userEmail", user_email);
+
 #if defined(OS_CHROMEOS)
   // Trigger the request for system information here.
   chromeos::system::SyslogsProvider* provider =
@@ -465,8 +487,10 @@ void FeedbackHandler::HandleGetDialogDefaults(const ListValue*) {
         base::Bind(&FeedbackData::SyslogsComplete,
                    base::Unretained(feedback_data_)));
   }
-  // User e-mail
-  dialog_defaults.SetString("userEmail", GetUserEmail());
+
+  // On ChromeOS if the user's email is blank, it means we don't
+  // have a logged in user, hence don't use saved screenshots.
+  dialog_defaults.SetBoolean("useSaved", !user_email.empty());
 #endif
 
   web_ui()->CallJavascriptFunction("setupDialogDefaults", dialog_defaults);
@@ -508,7 +532,7 @@ void FeedbackHandler::HandleSendReport(const ListValue* list_value) {
 #if defined(OS_CHROMEOS)
   if (list_value->GetSize() != 6) {
 #else
-  if (list_value->GetSize() != 4) {
+  if (list_value->GetSize() != 5) {
 #endif
     LOG(ERROR) << "Feedback data corrupt! Feedback not sent.";
     return;
@@ -521,6 +545,8 @@ void FeedbackHandler::HandleSendReport(const ListValue* list_value) {
   (*i++)->GetAsString(&category_tag);
   std::string description;
   (*i++)->GetAsString(&description);
+  std::string user_email;
+  (*i++)->GetAsString(&user_email);
   std::string screenshot_path;
   (*i++)->GetAsString(&screenshot_path);
   screenshot_path.erase(0, strlen(kScreenshotBaseUrl));
@@ -531,8 +557,6 @@ void FeedbackHandler::HandleSendReport(const ListValue* list_value) {
     image_ptr = screenshot_source_->GetCachedScreenshot(screenshot_path);
 
 #if defined(OS_CHROMEOS)
-  std::string user_email;
-  (*i++)->GetAsString(&user_email);
   std::string sys_info_checkbox;
   (*i++)->GetAsString(&sys_info_checkbox);
   bool send_sys_info = (sys_info_checkbox == "true");
@@ -548,9 +572,9 @@ void FeedbackHandler::HandleSendReport(const ListValue* list_value) {
                                , std::string()
                                , page_url
                                , description
+                               , user_email
                                , image_ptr
 #if defined(OS_CHROMEOS)
-                               , user_email
                                , send_sys_info
                                , false  // sent_report
                                , timestamp_