Skip to content

GitLab

T
tangled
Commit e39d1178 authored by sebsg's avatar sebsg Committed by Commit Bot
Browse files
Options

[AF] Only try to replace the triggering element during a re-fill. 

There is an issue with the replacing logic if the forms and fields
have no identifier. This CL does not fix this directly, but greatly
reduces the cases where it will happen (only dynamic forms with more
than one form with no id).

Rogerm@ is working on a better way of identifying fields that
should help resolve this: crbug.com/896689

Bug: 898346
Change-Id: I7e17a6a2009be9e376088640bb14a81549ebbef2
Reviewed-on: https://chromium-review.googlesource.com/c/1319851
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: default avatarFabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606224}
parent 54fc3498
Hide whitespace changes
Inline Side-by-side
Showing with 73 additions and 1 deletion
chrome/browser/autofill/autofill_interactive_uitest.cc
View file @ e39d1178
...@@ -2173,6 +2173,64 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, DynamicChangingFormFill) { ...@@ -2173,6 +2173,64 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, DynamicChangingFormFill) {
ExpectFieldValue("phone_form1", ""); ExpectFieldValue("phone_form1", "");
} }
// Test that a page with 2 forms with no name and id containing fields with no
// name or if get filled properly.
IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
FillFormAndFieldWithNoNameOrId) {
CreateTestProfile();
GURL url = embedded_test_server()->GetURL(
"/autofill/forms_without_identifiers.html");
ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(browser(), url));
// Focus on the first field of the second form.
bool result = false;
std::string script =
R"( function onFocusHandler(e) {
e.target.removeEventListener(e.type, arguments.callee);
domAutomationController.send(true);
}
if (document.readyState === 'complete') {
var target = document.forms[1].elements[0];
target.addEventListener('focus', onFocusHandler);
target.focus();
} else {
domAutomationController.send(false);
})";
ASSERT_TRUE(
content::ExecuteScriptAndExtractBool(GetWebContents(), script, &result));
ASSERT_TRUE(result);
// Start filling the first name field with "M" and wait for the popup to be
// shown.
SendKeyToPageAndWait(ui::DomKey::FromCharacter('M'), ui::DomCode::US_M,
ui::VKEY_M, {ObservedUiEvents::kSuggestionShown});
// Press the down arrow to select the suggestion and preview the autofilled
// form.
SendKeyToPopupAndWait(ui::DomKey::ARROW_DOWN,
{ObservedUiEvents::kPreviewFormData});
// Press Enter to accept the autofill suggestions.
SendKeyToPopupAndWait(ui::DomKey::ENTER, {ObservedUiEvents::kFormDataFilled});
// Make sure that the form was filled.
std::string value;
ASSERT_TRUE(content::ExecuteScriptAndExtractString(
GetWebContents(),
"window.domAutomationController.send("
" document.forms[1].elements[0].value);",
&value));
EXPECT_EQ("Milton C. Waddams", value) << "for first field";
ASSERT_TRUE(content::ExecuteScriptAndExtractString(
GetWebContents(),
"window.domAutomationController.send("
" document.forms[1].elements[1].value);",
&value));
EXPECT_EQ("red.swingline@initech.com", value) << "for second field";
}
// Test that we can Autofill forms where some fields name change during the // Test that we can Autofill forms where some fields name change during the
// fill. // fill.
IN_PROC_BROWSER_TEST_P(AutofillCompanyInteractiveTest, FieldsChangeName) { IN_PROC_BROWSER_TEST_P(AutofillCompanyInteractiveTest, FieldsChangeName) {
......
chrome/test/data/autofill/forms_without_identifiers.html 0 → 100644
View file @ e39d1178
<!-- A page that is used to test that a page with 2 forms without idenfiers that contain fields without identifiers get filled properly. -->
<body>
<form>
<button type="button"></button>
</form>
<form>
<input type="text" autocomplete="name">
<input type="text" autocomplete="email">
</form>
</body>
\ No newline at end of file
components/autofill/content/renderer/autofill_agent.cc
View file @ e39d1178
...@@ -450,7 +450,9 @@ void AutofillAgent::FillForm(int32_t id, const FormData& form) { ...@@ -450,7 +450,9 @@ void AutofillAgent::FillForm(int32_t id, const FormData& form) {
was_last_action_fill_ = true; was_last_action_fill_ = true;
if (base::FeatureList::IsEnabled(features::kAutofillDynamicForms)) // If this is a re-fill, replace the triggering element if it's invalid.
if (base::FeatureList::IsEnabled(features::kAutofillDynamicForms) &&
id == kNoQueryId)
ReplaceElementIfNowInvalid(form); ReplaceElementIfNowInvalid(form);
query_node_autofill_state_ = element_.GetAutofillState(); query_node_autofill_state_ = element_.GetAutofillState();
...@@ -1129,6 +1131,8 @@ bool AutofillAgent::FindTheUniqueNewVersionOfOldElement( ...@@ -1129,6 +1131,8 @@ bool AutofillAgent::FindTheUniqueNewVersionOfOldElement(
return true; return true;
} }
// TODO(crbug.com/896689): Update this method to use the unique ids once they
// are implemented.
void AutofillAgent::ReplaceElementIfNowInvalid(const FormData& original_form) { void AutofillAgent::ReplaceElementIfNowInvalid(const FormData& original_form) {
// If the document is invalid, bail out. // If the document is invalid, bail out.
if (element_.GetDocument().IsNull()) if (element_.GetDocument().IsNull())
......
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