rAc: fix international address formatting (mostly)

chrome/browser/ui/autofill/data_model_wrapper.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/autofill/data_model_wrapper.h"
 
 #include "base/callback.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 38 matching lines @@
 }
 
 gfx::Image DataModelWrapper::GetIcon() {
   return gfx::Image();
 }
 
 #if !defined(OS_ANDROID)
 bool DataModelWrapper::GetDisplayText(
     base::string16* vertically_compact,
     base::string16* horizontally_compact) {
+  base::string16 phone =
+      GetInfoForDisplay(AutofillType(PHONE_HOME_WHOLE_NUMBER));
+  if (phone.empty())
+    return false;

[Dan Beam, 2014/01/29 04:15:09]

^ but out of curiosity, why return false for only phone?

+
   // Format the address.
   ::i18n::addressinput::AddressData address_data;
   address_data.recipient = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(NAME_FULL)));
-  address_data.country_code = UTF16ToUTF8(
-      GetInfoForDisplay(AutofillType(ADDRESS_HOME_COUNTRY)));
+  address_data.country_code = UTF16ToASCII(
+      GetInfoForDisplay(AutofillType(HTML_TYPE_COUNTRY_CODE,
+                                     HTML_MODE_SHIPPING)));

[Dan Beam, 2014/01/29 03:26:59]

^ what's the effective difference here?

[Evan Stade, 2014/01/29 04:11:00]

returns country code instead of human-readable name

[Dan Beam, 2014/01/29 07:14:45]

not lgtm -- this breaks FieldMapWrapper (which assumes a server_field())

see here for an alternative fix:
https://codereview.chromium.org/145553009/diff/270001/chrome/browser/ui/autof...

   address_data.administrative_area = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_STATE)));
   address_data.locality = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_CITY)));
   address_data.dependent_locality = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_DEPENDENT_LOCALITY)));
   address_data.sorting_code = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_SORTING_CODE)));
   address_data.postal_code = UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_ZIP)));
 
   address_data.address_lines.push_back(UTF16ToUTF8(
       GetInfoForDisplay(AutofillType(ADDRESS_HOME_LINE1))));
   base::string16 address2 = GetInfoForDisplay(AutofillType(ADDRESS_HOME_LINE2));
   if (!address2.empty())
     address_data.address_lines.push_back(UTF16ToUTF8(address2));
 
   std::vector<std::string> lines;
   address_data.FormatForDisplay(&lines);
 
   // Email and phone number aren't part of address formatting.
   base::string16 non_address_info;
   base::string16 email = GetInfoForDisplay(AutofillType(EMAIL_ADDRESS));
   if (!email.empty())
     non_address_info += ASCIIToUTF16("\n") + email;
 
-  non_address_info += ASCIIToUTF16("\n") +
-      GetInfoForDisplay(AutofillType(PHONE_HOME_WHOLE_NUMBER));
+  non_address_info += ASCIIToUTF16("\n") + phone;
 
   // The separator is locale-specific.
   std::string compact_separator =
       ::i18n::addressinput::GetCompactAddressLinesSeparator(
           g_browser_process->GetApplicationLocale());
   *vertically_compact =
       base::UTF8ToUTF16(JoinString(lines, compact_separator)) +
           non_address_info;
   *horizontally_compact = base::UTF8ToUTF16(JoinString(lines, "\n")) +
       non_address_info;
@@ skipping 166 matching lines @@
   if (type.GetStorableType() == PHONE_HOME_WHOLE_NUMBER)
     return address_->DisplayPhoneNumber();
 
   return DataModelWrapper::GetInfoForDisplay(type);
 }
 
 #if !defined(OS_ANDROID)
 bool WalletAddressWrapper::GetDisplayText(
     base::string16* vertically_compact,
     base::string16* horizontally_compact) {
-  if (!address_->is_complete_address() ||
-      GetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER)).empty()) {
+  if (!address_->is_complete_address())
     return false;
-  }
 
   return DataModelWrapper::GetDisplayText(vertically_compact,
                                           horizontally_compact);
 }
 #endif
 
 // WalletInstrumentWrapper
 
 WalletInstrumentWrapper::WalletInstrumentWrapper(
     const wallet::WalletItems::MaskedInstrument* instrument)
@@ skipping 24 matching lines @@
 gfx::Image WalletInstrumentWrapper::GetIcon() {
   return instrument_->CardIcon();
 }
 
 #if !defined(OS_ANDROID)
 bool WalletInstrumentWrapper::GetDisplayText(
     base::string16* vertically_compact,
     base::string16* horizontally_compact) {
   // TODO(dbeam): handle other instrument statuses? http://crbug.com/233048
   if (instrument_->status() == wallet::WalletItems::MaskedInstrument::EXPIRED ||
-      !instrument_->address().is_complete_address() ||
-      GetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER)).empty()) {
+      !instrument_->address().is_complete_address()) {
     return false;
   }
 
-  DataModelWrapper::GetDisplayText(vertically_compact, horizontally_compact);
+  if (!DataModelWrapper::GetDisplayText(vertically_compact,
+                                        horizontally_compact)) {
+    return false;
+  }
+
   // TODO(estade): descriptive_name() is user-provided. Should we use it or
   // just type + last 4 digits?
   base::string16 line1 = instrument_->descriptive_name() + ASCIIToUTF16("\n");
   *vertically_compact = line1 + *vertically_compact;
   *horizontally_compact = line1 + *horizontally_compact;
   return true;
 }
 #endif
 
 // FullWalletBillingWrapper
@@ skipping 51 matching lines @@
     : field_map_(field_map) {}
 
 FieldMapWrapper::~FieldMapWrapper() {}
 
 base::string16 FieldMapWrapper::GetInfo(const AutofillType& type) const {
   FieldValueMap::const_iterator it = field_map_.find(type.server_type());
   return it != field_map_.end() ? it->second : base::string16();
 }
 
 }  // namespace autofill