Commit 1cc6bf89 authored by Olivier Robin's avatar Olivier Robin Committed by Commit Bot

Do not override autofill suggestion with empty value.

If a form contains multiple fields with the same name, autofill
suggestions often fills only one.
The current resolution process keeps the value of the last element with
a given name (each value overrides the previous value).
This CLs make the resolution process to favor the non-empty value.
The filling will try to fill every fields with the same name with this
value.

Bug: 728374, 732360
Change-Id: I5c5c1635c59679a9823589e914c65bb734b3b505
Reviewed-on: https://chromium-review.googlesource.com/806162Reviewed-by: default avatarRoger McFarlane <rogerm@chromium.org>
Reviewed-by: default avatarPeter Lee <pkl@chromium.org>
Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524721}
parent 1e934065
...@@ -197,7 +197,8 @@ void GetFormAndField(autofill::FormData* form, ...@@ -197,7 +197,8 @@ void GetFormAndField(autofill::FormData* form,
// AutofillManagerDelegate. // AutofillManagerDelegate.
base::WeakPtr<autofill::AutofillPopupDelegate> popupDelegate_; base::WeakPtr<autofill::AutofillPopupDelegate> popupDelegate_;
// The autofill data that needs to be send when the |webState_| is shown. // The autofill data that needs to be send when the |webState_| is shown.
NSString* pendingFormData_; // The string is in JSON format.
NSString* pendingFormJSON_;
} }
- (instancetype)initWithPrefService:(PrefService*)prefService - (instancetype)initWithPrefService:(PrefService*)prefService
...@@ -654,10 +655,10 @@ void GetFormAndField(autofill::FormData* form, ...@@ -654,10 +655,10 @@ void GetFormAndField(autofill::FormData* form,
- (void)webStateWasShown:(web::WebState*)webState { - (void)webStateWasShown:(web::WebState*)webState {
DCHECK_EQ(webState_, webState); DCHECK_EQ(webState_, webState);
if (pendingFormData_) { if (pendingFormJSON_) {
[self sendDataToWebState:pendingFormData_]; [self sendDataToWebState:pendingFormJSON_];
} }
pendingFormData_ = nil; pendingFormJSON_ = nil;
} }
- (void)webState:(web::WebState*)webState - (void)webState:(web::WebState*)webState
...@@ -870,32 +871,35 @@ void GetFormAndField(autofill::FormData* form, ...@@ -870,32 +871,35 @@ void GetFormAndField(autofill::FormData* form,
} }
- (void)onFormDataFilled:(const autofill::FormData&)form { - (void)onFormDataFilled:(const autofill::FormData&)form {
std::unique_ptr<base::DictionaryValue> formData(new base::DictionaryValue); std::unique_ptr<base::DictionaryValue> JSONForm(new base::DictionaryValue);
formData->SetString("formName", base::UTF16ToUTF8(form.name)); JSONForm->SetString("formName", base::UTF16ToUTF8(form.name));
// Note: Destruction of all child base::Value types is handled by the root // Note: Destruction of all child base::Value types is handled by the root
// formData object on its own destruction. // formData object on its own destruction.
auto fieldsData = base::MakeUnique<base::DictionaryValue>(); auto JSONFields = base::MakeUnique<base::DictionaryValue>();
const std::vector<autofill::FormFieldData>& fields = form.fields; const std::vector<autofill::FormFieldData>& autofillFields = form.fields;
for (const auto& fieldData : fields) { for (const auto& autofillField : autofillFields) {
fieldsData->SetKey(base::UTF16ToUTF8(fieldData.name), if (JSONFields->HasKey(base::UTF16ToUTF8(autofillField.name)) &&
base::Value(fieldData.value)); autofillField.value.empty())
continue;
JSONFields->SetKey(base::UTF16ToUTF8(autofillField.name),
base::Value(autofillField.value));
} }
formData->Set("fields", std::move(fieldsData)); JSONForm->Set("fields", std::move(JSONFields));
// Stringify the JSON data and send it to the UIWebView-side fillForm method. // Stringify the JSON data and send it to the UIWebView-side fillForm method.
std::string dataString; std::string JSONString;
base::JSONWriter::Write(*formData.get(), &dataString); base::JSONWriter::Write(*JSONForm.get(), &JSONString);
NSString* nsDataString = base::SysUTF8ToNSString(dataString); NSString* nsJSONString = base::SysUTF8ToNSString(JSONString);
if (!webState_->IsVisible()) { if (!webState_->IsVisible()) {
pendingFormData_ = nsDataString; pendingFormJSON_ = nsJSONString;
return; return;
} }
[self sendDataToWebState:nsDataString]; [self sendDataToWebState:nsJSONString];
} }
- (void)sendDataToWebState:(NSString*)formData { - (void)sendDataToWebState:(NSString*)JSONData {
DCHECK(webState_->IsVisible()); DCHECK(webState_->IsVisible());
// It is possible that the fill was not initiated by selecting a suggestion. // It is possible that the fill was not initiated by selecting a suggestion.
// In this case we provide an empty callback. // In this case we provide an empty callback.
...@@ -903,7 +907,7 @@ void GetFormAndField(autofill::FormData* form, ...@@ -903,7 +907,7 @@ void GetFormAndField(autofill::FormData* form,
suggestionHandledCompletion_ = [^{ suggestionHandledCompletion_ = [^{
} copy]; } copy];
[jsAutofillManager_ [jsAutofillManager_
fillForm:formData fillForm:JSONData
forceFillFieldName:base::SysUTF16ToNSString(pendingAutocompleteField_) forceFillFieldName:base::SysUTF16ToNSString(pendingAutocompleteField_)
completionHandler:suggestionHandledCompletion_]; completionHandler:suggestionHandledCompletion_];
suggestionHandledCompletion_ = nil; suggestionHandledCompletion_ = nil;
......
...@@ -137,7 +137,7 @@ TEST_F(AutofillAgentTests, OnFormDataFilledWithNameCollisionTest) { ...@@ -137,7 +137,7 @@ TEST_F(AutofillAgentTests, OnFormDataFilledWithNameCollisionTest) {
[[mock_js_autofill_manager_ expect] fillForm: [[mock_js_autofill_manager_ expect] fillForm:
@"{\"fields\":{" @"{\"fields\":{"
"\"field1\":\"value 2\"," "\"field1\":\"value 2\","
"\"region\":\"\"" "\"region\":\"California\""
"},\"formName\":\"\"}" "},\"formName\":\"\"}"
forceFillFieldName:@"" forceFillFieldName:@""
completionHandler:[OCMArg any]]; completionHandler:[OCMArg any]];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment