diff options
5 files changed, 132 insertions, 42 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index db22340..6c268f7 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -10693,9 +10693,6 @@ Would you like to start <ph name="CONTROL_PANEL_APPLET_NAME">$1<ex>Add/Remove Pr <message name="IDS_AUTOFILL_DIALOG_VALIDATION_INVALID_PHONE_NUMBER" desc="Message displayed to user when phone number validation fails."> Invalid phone number. Please check and try again. </message> - <message name="IDS_AUTOFILL_DIALOG_VALIDATION_MISSING_VALUE" desc="Message displayed to user when one of the required fields is left empty."> - Field is required - </message> <!-- Autofill dialog post-submit bubbles --> <message name="IDS_AUTOFILL_GENERATED_CREDIT_CARD_BUBBLE_TITLE" desc="Title of a bubble shown directly after a new card has been generated via Online Wallet using the Autofill dialog."> diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc index 9744d89..673986d 100644 --- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc +++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc @@ -77,6 +77,7 @@ #include "grit/chromium_strings.h" #include "grit/component_strings.h" #include "grit/generated_resources.h" +#include "grit/libaddressinput_strings.h" #include "grit/platform_locale_settings.h" #include "grit/theme_resources.h" #include "grit/webkit_resources.h" @@ -1721,14 +1722,14 @@ base::string16 AutofillDialogControllerImpl::InputValidityMessage( case CREDIT_CARD_EXP_MONTH: if (!InputWasEdited(CREDIT_CARD_EXP_MONTH, value)) { return l10n_util::GetStringUTF16( - IDS_AUTOFILL_DIALOG_VALIDATION_MISSING_VALUE); + IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD); } break; case CREDIT_CARD_EXP_4_DIGIT_YEAR: if (!InputWasEdited(CREDIT_CARD_EXP_4_DIGIT_YEAR, value)) { return l10n_util::GetStringUTF16( - IDS_AUTOFILL_DIALOG_VALIDATION_MISSING_VALUE); + IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD); } break; @@ -1790,9 +1791,9 @@ base::string16 AutofillDialogControllerImpl::InputValidityMessage( break; } - return value.empty() ? - l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_VALIDATION_MISSING_VALUE) : - base::string16(); + return value.empty() ? l10n_util::GetStringUTF16( + IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD) : + base::string16(); } // TODO(groby): Also add tests. @@ -1803,6 +1804,7 @@ ValidityMessages AutofillDialogControllerImpl::InputsAreValid( if (inputs.empty()) return messages; + AddressValidator::Status status = AddressValidator::SUCCESS; if (i18ninput::Enabled() && section != SECTION_CC) { AddressData address_data; i18ninput::CreateAddressData( @@ -1810,21 +1812,17 @@ ValidityMessages AutofillDialogControllerImpl::InputsAreValid( &address_data); AddressProblems problems; - if (GetValidator()->ValidateAddress( - address_data, - AddressProblemFilter(), - &problems) == AddressValidator::SUCCESS) { - common::AddressType address_type = section == SECTION_SHIPPING ? - common::ADDRESS_TYPE_SHIPPING : common::ADDRESS_TYPE_BILLING; - for (size_t i = 0; i < problems.size(); ++i) { - const AddressProblem& problem = problems[i]; - bool sure = problem.type != AddressProblem::MISSING_REQUIRED_FIELD; - base::string16 text = l10n_util::GetStringUTF16(problem.description_id); - messages.Set(i18ninput::TypeForField(problem.field, address_type), - ValidityMessage(text, sure)); - } - } else { - // TODO(dbeam): disable submit button until able to successfully validate. + status = GetValidator()->ValidateAddress(address_data, + AddressProblemFilter(), + &problems); + common::AddressType address_type = section == SECTION_SHIPPING ? + common::ADDRESS_TYPE_SHIPPING : common::ADDRESS_TYPE_BILLING; + for (size_t i = 0; i < problems.size(); ++i) { + const AddressProblem& problem = problems[i]; + bool sure = problem.type != AddressProblem::MISSING_REQUIRED_FIELD; + base::string16 text = l10n_util::GetStringUTF16(problem.description_id); + messages.Set(i18ninput::TypeForField(problem.field, address_type), + ValidityMessage(text, sure)); } } @@ -1838,6 +1836,18 @@ ValidityMessages AutofillDialogControllerImpl::InputsAreValid( // it to be valid unless later proven otherwise. bool sure = InputWasEdited(type, iter->second) || AutofillType(type).GetStorableType() == ADDRESS_HOME_COUNTRY; + + if (status != AddressValidator::SUCCESS && + InputWasEdited(type, iter->second) && + (AutofillType(type).group() == ADDRESS_HOME || + AutofillType(type).group() == ADDRESS_BILLING)) { + DCHECK(text.empty()); + // TODO(estade): string translation or remove this (sweet) hack. + text = base::ASCIIToUTF16("Sorry, Chrome failed to validate this field. " + "Please wait and try again."); + sure = false; + } + messages.Set(type, ValidityMessage(text, sure)); } diff --git a/third_party/libaddressinput/chromium/cpp/res/messages.grdp b/third_party/libaddressinput/chromium/cpp/res/messages.grdp index a09c6d9..ce7b442 100644 --- a/third_party/libaddressinput/chromium/cpp/res/messages.grdp +++ b/third_party/libaddressinput/chromium/cpp/res/messages.grdp @@ -116,7 +116,7 @@ https://code.google.com/p/libaddressinput/source/browse/trunk/java/res/values/ad <message name="IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD" desc="Message to be shown when a required field is empty"> - Required + Field is required </message> <message name="IDS_LIBADDRESSINPUT_I18N_INVALID_ENTRY" diff --git a/third_party/libaddressinput/chromium/cpp/src/address_validator.cc b/third_party/libaddressinput/chromium/cpp/src/address_validator.cc index b08dc83..0657d30 100644 --- a/third_party/libaddressinput/chromium/cpp/src/address_validator.cc +++ b/third_party/libaddressinput/chromium/cpp/src/address_validator.cc @@ -32,6 +32,7 @@ #include "country_rules_aggregator.h" #include "grit/libaddressinput_strings.h" +#include "region_data_constants.h" #include "retriever.h" #include "rule.h" #include "ruleset.h" @@ -111,7 +112,17 @@ class AddressValidatorImpl : public AddressValidator { AddressProblems* problems) const { std::map<std::string, const Ruleset*>::const_iterator ruleset_it = rules_.find(address.country_code); + + // We can still validate the required fields even if the full ruleset isn't + // ready. if (ruleset_it == rules_.end()) { + Rule rule; + rule.CopyFrom(Rule::GetDefault()); + if (rule.ParseSerializedRule( + RegionDataConstants::GetRegionData(address.country_code))) { + EnforceRequiredFields(rule, address, filter, problems); + } + return loading_rules_.find(address.country_code) != loading_rules_.end() ? RULES_NOT_READY : RULES_UNAVAILABLE; @@ -121,24 +132,7 @@ class AddressValidatorImpl : public AddressValidator { assert(ruleset != NULL); const Rule& country_rule = ruleset->GetLanguageCodeRule(address.language_code); - - // Validate required fields. - for (std::vector<AddressField>::const_iterator - field_it = country_rule.GetRequired().begin(); - field_it != country_rule.GetRequired().end(); - ++field_it) { - bool field_empty = *field_it != STREET_ADDRESS - ? address.GetFieldValue(*field_it).empty() - : IsEmptyStreetAddress(address.address_lines); - if (field_empty && - FilterAllows( - filter, *field_it, AddressProblem::MISSING_REQUIRED_FIELD)) { - problems->push_back(AddressProblem( - *field_it, - AddressProblem::MISSING_REQUIRED_FIELD, - IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD)); - } - } + EnforceRequiredFields(country_rule, address, filter, problems); // Validate general postal code format. A country-level rule specifies the // regular expression for the whole postal code. @@ -218,6 +212,29 @@ class AddressValidatorImpl : public AddressValidator { } } + // Adds problems for just the required fields portion of |country_rule|. + void EnforceRequiredFields(const Rule& country_rule, + const AddressData& address, + const AddressProblemFilter& filter, + AddressProblems* problems) const { + for (std::vector<AddressField>::const_iterator + field_it = country_rule.GetRequired().begin(); + field_it != country_rule.GetRequired().end(); + ++field_it) { + bool field_empty = *field_it != STREET_ADDRESS + ? address.GetFieldValue(*field_it).empty() + : IsEmptyStreetAddress(address.address_lines); + if (field_empty && + FilterAllows( + filter, *field_it, AddressProblem::MISSING_REQUIRED_FIELD)) { + problems->push_back(AddressProblem( + *field_it, + AddressProblem::MISSING_REQUIRED_FIELD, + IDS_LIBADDRESSINPUT_I18N_MISSING_REQUIRED_FIELD)); + } + } + } + // Loads the ruleset for a country code. CountryRulesAggregator aggregator_; diff --git a/third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc b/third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc index 258b488..519517e 100644 --- a/third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc +++ b/third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc @@ -60,5 +60,71 @@ TEST_F(AddressValidatorTest, EmptyAddressNoFatalFailure) { validator_->ValidateAddress(address, AddressProblemFilter(), &problems)); } +TEST_F(AddressValidatorTest, BasicValidation) { + // US rules should always be available, even though this load call fails. + validator_->LoadRules("US"); + AddressData address; + address.country_code = "US"; + address.language_code = "en"; + address.administrative_area = "TX"; + address.locality = "Paris"; + address.postal_code = "75461"; + address.address_lines.push_back("123 Main St"); + AddressProblems problems; + EXPECT_EQ( + AddressValidator::SUCCESS, + validator_->ValidateAddress(address, AddressProblemFilter(), &problems)); + + // TODO(estade): this should be EXPECT_TRUE(problems.empty()). Postal code + // validation is broken at the moment. + EXPECT_EQ(1U, problems.size()); +} + +TEST_F(AddressValidatorTest, BasicValidationFailure) { + // US rules should always be available, even though this load call fails. + validator_->LoadRules("US"); + AddressData address; + address.country_code = "US"; + address.language_code = "en"; + address.administrative_area = "XT"; + address.locality = "Paris"; + address.postal_code = "75461"; + address.address_lines.push_back("123 Main St"); + AddressProblems problems; + EXPECT_EQ( + AddressValidator::SUCCESS, + validator_->ValidateAddress(address, AddressProblemFilter(), &problems)); + + ASSERT_EQ(1U, problems.size()); + EXPECT_EQ(AddressProblem::UNKNOWN_VALUE, problems[0].type); + EXPECT_EQ(ADMIN_AREA, problems[0].field); +} + +TEST_F(AddressValidatorTest, ValidationFailureWithNoRulesPresent) { + // The fake downloader/fake storage will fail to get these rules. + validator_->LoadRules("CA"); + AddressData address; + address.country_code = "CA"; + address.language_code = "en"; + address.administrative_area = "Never never land"; + address.locality = "grassy knoll"; + address.postal_code = "1234"; + address.address_lines.push_back("123 Main St"); + AddressProblems problems; + EXPECT_EQ( + AddressValidator::RULES_UNAVAILABLE, + validator_->ValidateAddress(address, AddressProblemFilter(), &problems)); + EXPECT_TRUE(problems.empty()); + + // Field requirements are still enforced even if the rules aren't downloaded. + address.administrative_area = ""; + problems.clear(); + EXPECT_EQ( + AddressValidator::RULES_UNAVAILABLE, + validator_->ValidateAddress(address, AddressProblemFilter(), &problems)); + ASSERT_EQ(1U, problems.size()); + EXPECT_EQ(AddressProblem::MISSING_REQUIRED_FIELD, problems[0].type); +} + } // namespace addressinput } // namespace i18n |