Skip to content

Commit

Permalink
Fallback to infer text behind <input>s for assigned labels.
Browse files Browse the repository at this point in the history
A developer can associate a <label> to an input in two ways:
1) Using the for-attribute: <label for=fieldID>
2) By making the <input> a child of the <label>.

This CL addresses the second case. Right now, FindChildText() is used
to extract the content of the label. However, FindChildText() stops at
autofillable elements, which causes us to miss the label in cases like
this:
<label>
  <input>
  text
</label>
This causes problems e.g. on citilink.ru (see https://chromium-review.googlesource.com/c/chromium/src/+/4079457/comment/de5c03ad_a1d9971a/).

Generally, this behaviour of FindChildText() seems reasonable, since
text behind an input is oftentimes only relevant for error handling etc.
However, in case the input is explicitly associated with the label, it
might make sense to be more lenient.

This CL adds a fallback mechanism: If extracting a label using
FindChildText() for an associated label of the second kind fails,
we instead look at the text after the input.

Change-Id: I20a69ede53f65ce3f255a2fd7450f4a0e8365d26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4106611
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Florian Leimgruber <fleimgruber@google.com>
Cr-Commit-Position: refs/heads/main@{#1084877}
  • Loading branch information
florianleimgruber authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 1615725 commit 86bff38
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 15 deletions.
17 changes: 17 additions & 0 deletions chrome/renderer/autofill/form_autofill_browsertest.cc
Expand Up @@ -3592,6 +3592,23 @@ TEST_F(FormAutofillTest, LabelForAttribute) {
testing::UnorderedElementsAre(base::Bucket(AssignedLabelSource::kId, 2)));
}

// Tests that when a label is assigned to an input, text behind it is considered
// as a fallback.
// The label is assigned to the input without the for-attribute, by declaring it
// it inside the label.
TEST_F(FormAutofillTest, LabelTextBehindInput) {
ExpectLabels(R"(
<form name=TestForm action=http://cnn.com>
<label>
<input>
label
</label>
</form>
)",
/*id_attributes=*/{u""}, /*name_attributes=*/{u""},
/*labels=*/{u"label"}, /*names=*/{u""}, /*values=*/{u""});
}

TEST_F(FormAutofillTest, LabelsWithSpans) {
ExpectJohnSmithLabelsAndIdAttributes(
"<FORM name='TestForm' action='http://cnn.com' method='post'>"
Expand Down
25 changes: 23 additions & 2 deletions components/autofill/content/renderer/form_autofill_util.cc
Expand Up @@ -1396,12 +1396,33 @@ void MatchLabelsAndFields(
field_data = iter->first;
}

// Skip `label` if we could not find an associated form control.
if (!field_data)
continue;

std::u16string label_text = FindChildText(label);
if (label_text.empty())
continue;
if (label_text.empty()) {
if (label.HasAttribute(*kFor)) {
continue;
}
DCHECK(!control.IsNull() && control.IsFormControlElement());
// An associated form control was found, but the `label` does not have a
// for-attribute, so the form control must be a descendant of the `label`.
// Since `FindChildText()` stops at autofillable elements, the
// `label_text` can be empty if the "text" is declared behind the <input>.
// For example:
// <label>
// <input>
// text
// </label>
// Thus, consider text behind the <input> as a fallback.
// Since associated labels are counted as `kFor`, the source is ignored.
FormFieldData::LabelSource irrelevant_source;
if (!InferLabelFromNext(control.To<WebFormControlElement>(), label_text,
irrelevant_source)) {
continue;
}
}

// Concatenate labels because some sites might have multiple label
// candidates.
Expand Down
5 changes: 4 additions & 1 deletion components/autofill/ios/form_util/resources/fill.js
Expand Up @@ -729,7 +729,10 @@ function matchLabelsAndFields_(
if (!('label' in fieldData)) {
fieldData['label'] = '';
}
const labelText = __gCrWeb.fill.findChildText(label);
let labelText = __gCrWeb.fill.findChildText(label);
if (labelText.length === 0 && !label.htmlFor) {
labelText = __gCrWeb.fill.inferLabelFromNext(fieldElement);
}
// Concatenate labels because some sites might have multiple label
// candidates.
if (fieldData['label'].length > 0 && labelText.length > 0) {
Expand Down
Expand Up @@ -8,8 +8,8 @@ NAME_FIRST | ctl00$ContentInfo$Name$FName$txtFName | First Name: | | ctl00$Cust
NAME_MIDDLE_INITIAL | ctl00$ContentInfo$Name$MName$txtMName | Middle Initial (optional): | | ctl00$CustomerHeader$ddlCountries_1
NAME_LAST | ctl00$ContentInfo$Name$LName$txtLName | Last Name: | | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$Name$suffix$txtSuffix | Suffix (optional): | | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$MemberAge$EnrollmentAge | This OnePass account is for an adult 18 years of age or older. | rdoAdult | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$MemberAge$EnrollmentAge | This OnePass account is for a minor under the age of 18. | rdoMinor | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$MemberAge$EnrollmentAge | This OnePass account is for an adult 18 years of age or older. This OnePass account is for an adult 18 years of age or older. | rdoAdult | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$MemberAge$EnrollmentAge | This OnePass account is for a minor under the age of 18. This OnePass account is for a minor under the age of 18. | rdoMinor | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$BirthDate$txtDOB | Minor’s Date of Birth: | mm/dd/yyyy | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$AddressType$AddressType | Home | rdoAddTypeHome | ctl00$CustomerHeader$ddlCountries_1
UNKNOWN_TYPE | ctl00$ContentInfo$AddressType$AddressType | Business/Other | rdoAddTypeBusiness | ctl00$CustomerHeader$ddlCountries_1
Expand Down
@@ -1,10 +1,10 @@
UNKNOWN_TYPE | creditCard | American Express Discover | | creditCard_1
CREDIT_CARD_NAME_FULL | cardholderName | Cardholder's name | | cardholderName_2
CREDIT_CARD_EXP_MONTH | expireMonth | Expiration Date | | cardholderName_2
CREDIT_CARD_EXP_4_DIGIT_YEAR | expireYear | Expiration Date | | cardholderName_2
CREDIT_CARD_VERIFICATION_CODE | securityNumber | CVV Number 3 or 4 digit number | | cardholderName_2
UNKNOWN_TYPE | nickname | Card's nickname e.g. My Visa, Corporate card, etc | | creditCard_1
UNKNOWN_TYPE | useAddress | Use stored address | use-stored-address | creditCard_1
UNKNOWN_TYPE | stored-address | | ? | creditCard_1
UNKNOWN_TYPE | useAddress | Add new address | add-new-address | creditCard_1
UNKNOWN_TYPE | shouldSave | Save credit card info | on | creditCard_1
CREDIT_CARD_NUMBER | creditCard | Invalid card # | | creditCard_1
CREDIT_CARD_NAME_FULL | cardholderName | Cardholder's name | | creditCard_1
CREDIT_CARD_EXP_MONTH | expireMonth | Expiration Date | | creditCard_1
CREDIT_CARD_EXP_4_DIGIT_YEAR | expireYear | Expiration Date | | creditCard_1
CREDIT_CARD_VERIFICATION_CODE | securityNumber | CVV Number 3 or 4 digit number | | creditCard_1
UNKNOWN_TYPE | nickname | Card's nickname e.g. My Visa, Corporate card, etc | | nickname_2
UNKNOWN_TYPE | useAddress | Use stored address | use-stored-address | nickname_2
UNKNOWN_TYPE | stored-address | | ? | nickname_2
UNKNOWN_TYPE | useAddress | Add new address | add-new-address | nickname_2
UNKNOWN_TYPE | shouldSave | Save credit card info | on | nickname_2

0 comments on commit 86bff38

Please sign in to comment.