Commit dcc14ad6 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Fix passwords filling on iOS.

When filling forms, Chrome identifies fields by FormFieldData::id on iOS and FormFieldData::name
on other platforms. The new FormData->PasswordForm parser failed to use 'id' on iOS. This CL
fixes that by introducing platform-dependent identifier selector, which returns 'id' on iOS and
'name' elsewhere.

The alternative solution of switching the iOS code to use 'name' (or the rest to use 'id') is not
good for various reasons:

 - non-iOS code will ultimately migrate to using FormFieldData::unique_renderer_id, which is
superior to both but not supported on iOS
 - iOS code contains significant improvements which ensure that FormFieldData::id is actually
unique; this has been neither done outside of iOS, nor for FormFieldData::name
 - the amount of code needing change would be large and the benefit virtually null (compared to
the solution from this CL).

Bug: 831123
Change-Id: I24960064bb3f792ef4e0ebe6fbf769509481bad1
Reviewed-on: https://chromium-review.googlesource.com/1142151
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarVaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576489}
parent d999c0d9
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "components/autofill/core/common/autofill_regex_constants.h" #include "components/autofill/core/common/autofill_regex_constants.h"
#include "components/autofill/core/common/autofill_regexes.h" #include "components/autofill/core/common/autofill_regexes.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
...@@ -27,6 +28,7 @@ ...@@ -27,6 +28,7 @@
using autofill::FieldPropertiesFlags; using autofill::FieldPropertiesFlags;
using autofill::FormFieldData; using autofill::FormFieldData;
using autofill::PasswordForm; using autofill::PasswordForm;
using base::string16;
namespace password_manager { namespace password_manager {
...@@ -497,33 +499,45 @@ void ParseUsingBaseHeuristics( ...@@ -497,33 +499,45 @@ void ParseUsingBaseHeuristics(
return; return;
} }
string16 GetPlatformSpecificIdentifier(const FormFieldData& field) {
#if defined(OS_IOS)
return field.id;
#else
return field.name;
#endif
}
// Set username and password fields in |password_form| based on // Set username and password fields in |password_form| based on
// |significant_fields| . // |significant_fields| .
void SetFields(const SignificantFields& significant_fields, void SetFields(const SignificantFields& significant_fields,
PasswordForm* password_form) { PasswordForm* password_form) {
password_form->has_renderer_ids = true; password_form->has_renderer_ids = true;
if (significant_fields.username) { if (significant_fields.username) {
password_form->username_element = significant_fields.username->name; password_form->username_element =
GetPlatformSpecificIdentifier(*significant_fields.username);
password_form->username_value = significant_fields.username->value; password_form->username_value = significant_fields.username->value;
password_form->username_element_renderer_id = password_form->username_element_renderer_id =
significant_fields.username->unique_renderer_id; significant_fields.username->unique_renderer_id;
} }
if (significant_fields.password) { if (significant_fields.password) {
password_form->password_element = significant_fields.password->name; password_form->password_element =
GetPlatformSpecificIdentifier(*significant_fields.password);
password_form->password_value = significant_fields.password->value; password_form->password_value = significant_fields.password->value;
password_form->password_element_renderer_id = password_form->password_element_renderer_id =
significant_fields.password->unique_renderer_id; significant_fields.password->unique_renderer_id;
} }
if (significant_fields.new_password) { if (significant_fields.new_password) {
password_form->new_password_element = significant_fields.new_password->name; password_form->new_password_element =
GetPlatformSpecificIdentifier(*significant_fields.new_password);
password_form->new_password_value = significant_fields.new_password->value; password_form->new_password_value = significant_fields.new_password->value;
} }
if (significant_fields.confirmation_password) { if (significant_fields.confirmation_password) {
password_form->confirmation_password_element = password_form->confirmation_password_element =
significant_fields.confirmation_password->name; GetPlatformSpecificIdentifier(
*significant_fields.confirmation_password);
} }
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h" #include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/password_form.h" #include "components/autofill/core/common/password_form.h"
...@@ -241,7 +242,15 @@ void CheckField(const std::vector<FormFieldData>& fields, ...@@ -241,7 +242,15 @@ void CheckField(const std::vector<FormFieldData>& fields,
}); });
ASSERT_TRUE(field_it != fields.end()) ASSERT_TRUE(field_it != fields.end())
<< "Could not find a field with renderer ID " << renderer_id; << "Could not find a field with renderer ID " << renderer_id;
// On iOS |id| is used for identifying DOM elements, so the parser should return
// it.
#if defined(OS_IOS)
EXPECT_EQ(element_name, field_it->id);
#else
EXPECT_EQ(element_name, field_it->name); EXPECT_EQ(element_name, field_it->name);
#endif
if (element_value) if (element_value)
EXPECT_EQ(*element_value, field_it->value); EXPECT_EQ(*element_value, field_it->value);
} }
......
...@@ -62,11 +62,13 @@ class NewPasswordFormManagerTest : public testing::Test { ...@@ -62,11 +62,13 @@ class NewPasswordFormManagerTest : public testing::Test {
observed_form_.fields.push_back(field); observed_form_.fields.push_back(field);
field.name = ASCIIToUTF16("username"); field.name = ASCIIToUTF16("username");
field.id = field.name;
field.form_control_type = "text"; field.form_control_type = "text";
field.unique_renderer_id = 2; field.unique_renderer_id = 2;
observed_form_.fields.push_back(field); observed_form_.fields.push_back(field);
field.name = ASCIIToUTF16("password"); field.name = ASCIIToUTF16("password");
field.id = field.name;
field.form_control_type = "password"; field.form_control_type = "password";
field.unique_renderer_id = 3; field.unique_renderer_id = 3;
observed_form_.fields.push_back(field); observed_form_.fields.push_back(field);
......
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