Commit 164dbb35 authored by sandromaggi's avatar sandromaggi Committed by Commit Bot

[Autofill Assistant] Allow to clear RequiredFields

This CL allows to clear a field instead of filling it. It will behave
the same way as filling a field in regards to the |forced| flag, i.e.
if the field is already empty (and we don't force it) we don't do
anything.

The field is, in the end, checked for emptiness. Non empty fields with
the |clear_value| flag will fail the action.

The |clear_value| flag should only really be used for <input> fields.
It can work with <select> as well (setting the value attribute) but
this is not guaranteed to actually empty the field (if the default has
a different value). The flag does not work in combination with
|fallback_click_element| at all.

Bug: b/160850266
Change-Id: Ib170ee0f3e6931ad5f09a1664548c492e260e52a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2289779Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#787680}
parent b7f40445
...@@ -13,7 +13,10 @@ RequiredField::~RequiredField() = default; ...@@ -13,7 +13,10 @@ RequiredField::~RequiredField() = default;
RequiredField::RequiredField(const RequiredField& copy) = default; RequiredField::RequiredField(const RequiredField& copy) = default;
bool RequiredField::ShouldFallback(bool apply_fallback) const { bool RequiredField::ShouldFallback(bool apply_fallback) const {
return (status == EMPTY && !fallback_click_element.has_value()) || return (status == EMPTY && !value_expression.empty() &&
!fallback_click_element.has_value()) ||
(status != EMPTY && value_expression.empty() &&
!fallback_click_element.has_value()) ||
(forced && apply_fallback) || (forced && apply_fallback) ||
(fallback_click_element.has_value() && apply_fallback); (fallback_click_element.has_value() && apply_fallback);
} }
......
...@@ -54,6 +54,12 @@ void FillStatusDetailsWithEmptyField(const RequiredField& required_field, ...@@ -54,6 +54,12 @@ void FillStatusDetailsWithEmptyField(const RequiredField& required_field,
field_error->set_empty_after_fallback(true); field_error->set_empty_after_fallback(true);
} }
void FillStatusDetailsWithNotClearedField(const RequiredField& required_field,
ClientStatus* client_status) {
auto* field_error = AddAutofillError(required_field, client_status);
field_error->set_filled_after_clear(true);
}
} // namespace } // namespace
RequiredFieldsFallbackHandler::~RequiredFieldsFallbackHandler() = default; RequiredFieldsFallbackHandler::~RequiredFieldsFallbackHandler() = default;
...@@ -139,9 +145,15 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone( ...@@ -139,9 +145,15 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone(
if (required_field.ShouldFallback(apply_fallback)) { if (required_field.ShouldFallback(apply_fallback)) {
should_fallback = true; should_fallback = true;
if (!apply_fallback) { if (!apply_fallback) {
VLOG(1) << "Field was empty after applying fallback: " if (required_field.value_expression.empty()) {
<< required_field.selector; VLOG(1) << "Field was filled after attempting to clear it: "
FillStatusDetailsWithEmptyField(required_field, &client_status_); << required_field.selector;
FillStatusDetailsWithNotClearedField(required_field, &client_status_);
} else {
VLOG(1) << "Field was empty after applying fallback: "
<< required_field.selector;
FillStatusDetailsWithEmptyField(required_field, &client_status_);
}
} }
break; break;
} }
...@@ -168,9 +180,11 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone( ...@@ -168,9 +180,11 @@ void RequiredFieldsFallbackHandler::OnCheckRequiredFieldsDone(
continue; continue;
} }
if (field_formatter::FormatString(required_field.value_expression, if (required_field.value_expression.empty()) {
fallback_values_) has_fallbacks = true;
.has_value()) { } else if (field_formatter::FormatString(required_field.value_expression,
fallback_values_)
.has_value()) {
has_fallbacks = true; has_fallbacks = true;
} else { } else {
VLOG(3) << "Field has no fallback data: " << required_field.selector VLOG(3) << "Field has no fallback data: " << required_field.selector
...@@ -206,14 +220,25 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially( ...@@ -206,14 +220,25 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
return; return;
} }
// Set the next field to its fallback value. // Treat the next field.
const RequiredField& required_field = required_fields_[required_fields_index]; const RequiredField& required_field = required_fields_[required_fields_index];
if (required_field.value_expression.empty()) {
ActionDelegateUtil::SetFieldValue(
action_delegate_, required_field.selector, "",
required_field.fill_strategy, required_field.delay_in_millisecond,
base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
weak_ptr_factory_.GetWeakPtr(), required_fields_index));
return;
}
auto fallback_value = field_formatter::FormatString( auto fallback_value = field_formatter::FormatString(
required_field.value_expression, fallback_values_); required_field.value_expression, fallback_values_);
if (!fallback_value.has_value()) { if (!fallback_value.has_value()) {
VLOG(3) << "No fallback for " << required_field.selector; VLOG(3) << "No fallback for " << required_field.selector;
// If there is no fallback value, we skip this failed field. // If there is no fallback value, we skip this failed field.
return SetFallbackFieldValuesSequentially(++required_fields_index); SetFallbackFieldValuesSequentially(++required_fields_index);
return;
} }
if (required_field.fallback_click_element.has_value()) { if (required_field.fallback_click_element.has_value()) {
......
...@@ -601,5 +601,56 @@ TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) { ...@@ -601,5 +601,56 @@ TEST_F(RequiredFieldsFallbackHandlerTest, CustomDropdownClicksStopOnError) {
std::move(callback)); std::move(callback));
} }
TEST_F(RequiredFieldsFallbackHandlerTest, ClearsFilledFields) {
Selector full_field_selector({"#full_field"});
Selector empty_field_selector({"#empty_field"});
EXPECT_CALL(mock_web_controller_, OnGetFieldValue(full_field_selector, _))
.WillOnce(RunOnceCallback<1>(OkClientStatus(), "value"));
EXPECT_CALL(mock_web_controller_, OnGetFieldValue(empty_field_selector, _))
.Times(0);
Expectation clear_full_value =
EXPECT_CALL(
mock_action_delegate_,
OnSetFieldValue(EqualsElement(test_util::MockFindElement(
mock_action_delegate_, full_field_selector)),
"", _))
.WillOnce(RunOnceCallback<2>(OkClientStatus()));
EXPECT_CALL(mock_web_controller_, OnGetFieldValue(full_field_selector, _))
.After(clear_full_value)
.WillOnce(RunOnceCallback<1>(OkClientStatus(), ""));
Expectation clear_empty_value =
EXPECT_CALL(
mock_action_delegate_,
OnSetFieldValue(EqualsElement(test_util::MockFindElement(
mock_action_delegate_, empty_field_selector)),
"", _))
.WillOnce(RunOnceCallback<2>(OkClientStatus()));
EXPECT_CALL(mock_web_controller_, OnGetFieldValue(empty_field_selector, _))
.After(clear_empty_value)
.WillOnce(RunOnceCallback<1>(OkClientStatus(), ""));
auto non_forced_field = CreateRequiredField("", {"#full_field"});
auto forced_field = CreateRequiredField("", {"#empty_field"});
forced_field.forced = true;
std::vector<RequiredField> required_fields = {non_forced_field, forced_field};
std::map<std::string, std::string> fallback_values;
RequiredFieldsFallbackHandler fallback_handler(
required_fields, fallback_values, &mock_action_delegate_);
base::OnceCallback<void(const ClientStatus&,
const base::Optional<ClientStatus>&)>
callback =
base::BindOnce([](const ClientStatus& status,
const base::Optional<ClientStatus>& detail_status) {
EXPECT_EQ(status.proto_status(), ACTION_APPLIED);
});
fallback_handler.CheckAndFallbackRequiredFields(OkClientStatus(),
std::move(callback));
}
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -161,7 +161,7 @@ void UseAddressAction::ExecuteFallback(const ClientStatus& status) { ...@@ -161,7 +161,7 @@ void UseAddressAction::ExecuteFallback(const ClientStatus& status) {
std::vector<RequiredField> required_fields; std::vector<RequiredField> required_fields;
for (const auto& required_field_proto : for (const auto& required_field_proto :
proto_.use_address().required_fields()) { proto_.use_address().required_fields()) {
if (required_field_proto.value_expression().empty()) { if (!required_field_proto.has_value_expression()) {
continue; continue;
} }
......
...@@ -142,7 +142,7 @@ void UseCreditCardAction::OnGetFullCard( ...@@ -142,7 +142,7 @@ void UseCreditCardAction::OnGetFullCard(
std::vector<RequiredField> required_fields; std::vector<RequiredField> required_fields;
for (const auto& required_field_proto : proto_.use_card().required_fields()) { for (const auto& required_field_proto : proto_.use_card().required_fields()) {
if (required_field_proto.value_expression().empty()) { if (!required_field_proto.has_value_expression()) {
continue; continue;
} }
......
...@@ -668,6 +668,10 @@ message AutofillErrorInfoProto { ...@@ -668,6 +668,10 @@ message AutofillErrorInfoProto {
// The field was required and expected to be filled during the fallback // The field was required and expected to be filled during the fallback
// flow but was empty in the end. // flow but was empty in the end.
bool empty_after_fallback = 6; bool empty_after_fallback = 6;
// The field was expected to be cleared during the fallback flow but
// still had a value in the end.
bool filled_after_clear = 7;
} }
reserved 2; reserved 2;
...@@ -984,6 +988,7 @@ message UseAddressProto { ...@@ -984,6 +988,7 @@ message UseAddressProto {
// e.g., (+41) (79) (1234567) // e.g., (+41) (79) (1234567)
// Note that the set of actually available fields are outside of our // Note that the set of actually available fields are outside of our
// control and are retrieved automatically from the provided profile. // control and are retrieved automatically from the provided profile.
// An value expression set to an empty string will clear the field.
optional string value_expression = 6; optional string value_expression = 6;
optional SelectorProto element = 2; optional SelectorProto element = 2;
...@@ -1048,6 +1053,7 @@ message UseCreditCardProto { ...@@ -1048,6 +1053,7 @@ message UseCreditCardProto {
// * "${53}/${55}" -> expiration month / expiration year // * "${53}/${55}" -> expiration month / expiration year
// Note that the set of actually available fields are outside of our // Note that the set of actually available fields are outside of our
// control and are retrieved automatically from the provided credit card. // control and are retrieved automatically from the provided credit card.
// An value expression set to an empty string will clear the field.
optional string value_expression = 6; optional string value_expression = 6;
optional SelectorProto element = 2; optional SelectorProto element = 2;
......
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