autofill names - dont parse when calling SetRawInfo(FULL_NAME)

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

 // Copyright 2013 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/autofill/autofill_dialog_controller_impl.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
@@ skipping 183 matching lines @@
           field.Type().GetStorableType())));
 }
 
 // Initializes |form_group| from user-entered data.
 void FillFormGroupFromOutputs(const FieldValueMap& detail_outputs,
                               FormGroup* form_group) {
   for (FieldValueMap::const_iterator iter = detail_outputs.begin();
        iter != detail_outputs.end(); ++iter) {
     ServerFieldType type = iter->first;
     if (!iter->second.empty()) {
-      if (type == ADDRESS_HOME_COUNTRY || type == ADDRESS_BILLING_COUNTRY) {
-        form_group->SetInfo(AutofillType(type),
-                            iter->second,
-                            g_browser_process->GetApplicationLocale());
-      } else {
-        form_group->SetRawInfo(
-            AutofillType(type).GetStorableType(), iter->second);
-      }
+      form_group->SetInfo(AutofillType(type),
+                          iter->second,
+                          g_browser_process->GetApplicationLocale());

[Ilya Sherman, 2014/06/27 06:25:08]

Note that SetInfo() can override user-specified formatting, e.g. for phone
numbers.  Is that desirable?

[Evan Stade, 2014/06/28 01:01:35]

When is SetInfo meant to be called? Isn't it for data that a user has typed,
whereas SetRawInfo is more like "copy without modifying"?

It seems like PhoneNumber is broken if we want to retain user-specified
formatting, but SetInfo overrides that formatting. I just tested with web
autofill, and we don't retain user formatting there because PDM uses SetInfo. We
do retain formatting for data entered in chrome://settings/autofill. I'm not
sure if we want these to differ, but I think this UI is more like a web form
than the settings dialog (which is only for power users).

[Ilya Sherman, 2014/07/01 02:55:17]

SetInfo() is meant to be called on data that's coming from a webpage, where we
don't have control of the form.  It aggressively normalizes, to compensate for
that lack of control.

Either SetInfo() or SetRawInfo() is reasonable to call for input into our own
UIs.  However, for anything where we want to preserve user formatting -- which
historically has been at least phone numbers and credit card numbers -- we
should use SetRawInfo().
These do differ intentionally, as I described above.  If you think this UI is
not one where we want to preserve user formatting, I think that's a little
strange, but I won't push back too hard.

[Evan Stade, 2014/07/01 23:18:52]

I'm actually OK with preserving user formatting (when there is any ---
3105558293 was probably typed that way because it's faster and not because the
user thinks it's the prettiest format), but I also want consistency. It's
inconsistent to call SetInfo for one type and SetRawInfo for another, or to call
SetInfo for one UI and SetRawInfo for another. I'm not sure what the "lack of
control" of web forms actually translates to in a concrete sense, specifically
with the phone number example. Why would a user type differently into <input
type="text"> than this dialog?

[Ilya Sherman, 2014/07/02 00:02:15]

There are plenty of forms out there that impose their own formatting, overriding
anything that the user entered.  We only see the end state, so it's hard to
distinguish the formatting the user wanted from the formatting the site wanted.

[Evan Stade, 2014/07/07 16:20:52]

I see. And instead we impose the formatting that Chrome wants, even if it's
clobbering user-supplied formatting.

Let's consider 2 users, user A and user B, and two sites, site X and site Y.
User A cares about their formatting, user B does not. Site X has a freeform
input, site Y imposes formatting.

User A goes to site X. User provides formatting. Chrome overrides formatting.
User A is sad, unless she happens to like Chrome's formatting.
User A goes to site Y. Site imposes formatting. Chrome overrides formatting.
User A is sad, unless she happens to like Chrome's formatting.
(if smart enough, in these two cases, user A can go to
chrome://settings/autofill to fix it)

User B goes to site X. Chrome imposes formatting. User B is happy.
User B goes to site Y. Site imposes formatting. Chrome overrides formatting.
User B is happy.

So in the current world, user A is always sad, user B always happy. If Chrome
didn't override site formatting, user A would at least be happy on site X,
although we still couldn't fix site Y for them.

[Ilya Sherman, 2014/07/08 00:35:09]

Well, the most common case in the US is that the phone number is split across
multiple fields.  Hence, the most common case is that we add a pretty format
where there isn't one already.  I suppose we could change SetInfo() to only
write a pretty format if the incoming format is just a digit string.

     }
   }
 }
 
 // Get billing info from |output| and put it into |card|, |cvc|, and |profile|.
 // These outparams are required because |card|/|profile| accept different types
 // of raw info, and CreditCard doesn't save CVCs.
 void GetBillingInfoFromOutputs(const FieldValueMap& output,
                                CreditCard* card,
                                base::string16* cvc,
@@ skipping 2822 matching lines @@
             cards[i]->guid(),
             cards[i]->Label(),
             rb.GetImageNamed(CreditCard::IconResourceId(cards[i]->type())));
         suggested_cc_.SetEnabled(
             cards[i]->guid(),
             !ShouldDisallowCcType(cards[i]->TypeForDisplay()));
       }
 
       const std::vector<AutofillProfile*>& profiles = manager->GetProfiles();
       std::vector<base::string16> labels;
-      AutofillProfile::CreateDifferentiatingLabels(profiles, &labels);
+      AutofillProfile::CreateDifferentiatingLabels(
+          profiles,
+          g_browser_process->GetApplicationLocale(),
+          &labels);
       DCHECK_EQ(labels.size(), profiles.size());
       for (size_t i = 0; i < profiles.size(); ++i) {
         const AutofillProfile& profile = *profiles[i];
         if (!i18ninput::AddressHasCompleteAndVerifiedData(
                 profile, g_browser_process->GetApplicationLocale())) {
           continue;
         }
 
         // Don't add variants for addresses: name is part of credit card and
         // we'll just ignore email and phone number variants.
@@ skipping 1063 matching lines @@
   view_->UpdateButtonStrip();
 }
 
 void AutofillDialogControllerImpl::FetchWalletCookie() {
   net::URLRequestContextGetter* request_context = profile_->GetRequestContext();
   signin_helper_.reset(new wallet::WalletSigninHelper(this, request_context));
   signin_helper_->StartWalletCookieValueFetch();
 }
 
 }  // namespace autofill