mirror of
https://github.com/bitwarden/clients.git
synced 2026-07-07 21:10:55 +08:00
[PM-31646] stop label collection from crossing into sibling form-field containers (#19452)
Adds a containsChildFormElement guard to left-label, right-label, and
recursive sibling traversal so that text inside a sibling DOM container
that holds its own input/select/textarea is not absorbed as label text
for the target field. This prevents Yahoo's country-code dropdown
container ("Enter Country Code") from contaminating the username field's
label-left, which caused a false TOTP classification via the word "code".
This commit is contained in:
parent
877598ec9c
commit
f48647e2f1
@ -98,6 +98,9 @@ export class AutoFillConstants {
|
||||
...AutoFillConstants.ExcludedAutofillTypes,
|
||||
];
|
||||
|
||||
/** HTML elements for form fields */
|
||||
static readonly FieldElements: string[] = ["input", "select", "textarea"];
|
||||
|
||||
static readonly ExcludedIdentityAutocompleteTypes: Set<string> = new Set([
|
||||
"current-password",
|
||||
"new-password",
|
||||
|
||||
@ -1503,6 +1503,34 @@ describe("CollectAutofillContentService", () => {
|
||||
|
||||
expect(labelTag).toEqual("Username");
|
||||
});
|
||||
|
||||
it("does not collect text from a sibling that contains a form field", () => {
|
||||
document.body.innerHTML = `
|
||||
<div>
|
||||
<input type="text" name="username" id="username-id">
|
||||
<div>Enter Country Code <select><option>US</option></select></div>
|
||||
</div>
|
||||
`;
|
||||
const element = document.querySelector("#username-id") as FillableFormFieldElement;
|
||||
|
||||
const labelTag = collectAutofillContentService["createAutofillFieldRightLabel"](element);
|
||||
|
||||
expect(labelTag).toEqual("");
|
||||
});
|
||||
|
||||
it("does not stop traversal at a sibling div that has no form field descendant", () => {
|
||||
document.body.innerHTML = `
|
||||
<div>
|
||||
<input type="text" name="username" id="username-id">
|
||||
<div>Helper text</div>
|
||||
</div>
|
||||
`;
|
||||
const element = document.querySelector("#username-id") as FillableFormFieldElement;
|
||||
|
||||
const labelTag = collectAutofillContentService["createAutofillFieldRightLabel"](element);
|
||||
|
||||
expect(labelTag).toEqual("Helper text");
|
||||
});
|
||||
});
|
||||
|
||||
describe("createAutofillFieldLeftLabel", () => {
|
||||
@ -1520,6 +1548,53 @@ describe("CollectAutofillContentService", () => {
|
||||
|
||||
expect(labelTag).toEqual("Text ContentUsername");
|
||||
});
|
||||
|
||||
it("does not collect text from a direct sibling that contains a form field", () => {
|
||||
document.body.innerHTML = `
|
||||
<div>
|
||||
<div>Enter Country Code <select><option>US</option></select></div>
|
||||
<input type="text" name="username" id="username-id">
|
||||
</div>
|
||||
`;
|
||||
const element = document.querySelector("#username-id") as FillableFormFieldElement;
|
||||
|
||||
const labelTag = collectAutofillContentService["createAutofillFieldLeftLabel"](element);
|
||||
|
||||
expect(labelTag).toEqual("");
|
||||
});
|
||||
|
||||
it("does not collect text from a parent sibling that contains a form field", () => {
|
||||
// Exercises the parent-walk code path: the input has no direct previous
|
||||
// siblings, so the traversal walks up to the parent and checks its previous
|
||||
// sibling — which should be blocked because it contains a form field.
|
||||
document.body.innerHTML = `
|
||||
<div>
|
||||
<div>Enter Country Code <select><option>US</option></select></div>
|
||||
<div>
|
||||
<input type="text" name="username" id="username-id">
|
||||
</div>
|
||||
</div>
|
||||
`;
|
||||
const element = document.querySelector("#username-id") as FillableFormFieldElement;
|
||||
|
||||
const labelTag = collectAutofillContentService["createAutofillFieldLeftLabel"](element);
|
||||
|
||||
expect(labelTag).toEqual("");
|
||||
});
|
||||
|
||||
it("does not stop traversal at a sibling div that has no form field descendant", () => {
|
||||
document.body.innerHTML = `
|
||||
<div>
|
||||
<div>Helpful label</div>
|
||||
<input type="text" name="username" id="username-id">
|
||||
</div>
|
||||
`;
|
||||
const element = document.querySelector("#username-id") as FillableFormFieldElement;
|
||||
|
||||
const labelTag = collectAutofillContentService["createAutofillFieldLeftLabel"](element);
|
||||
|
||||
expect(labelTag).toEqual("Helpful label");
|
||||
});
|
||||
});
|
||||
|
||||
describe("createAutofillFieldTopLabel", () => {
|
||||
@ -1653,6 +1728,42 @@ describe("CollectAutofillContentService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("containsChildField", () => {
|
||||
it("returns true when the element contains an input descendant", () => {
|
||||
const div = document.createElement("div");
|
||||
div.innerHTML = `<span>Enter Country Code</span><input type="text" />`;
|
||||
|
||||
expect(collectAutofillContentService["containsChildField"](div)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true when the element contains a select descendant", () => {
|
||||
const div = document.createElement("div");
|
||||
div.innerHTML = `<select><option>US</option></select>`;
|
||||
|
||||
expect(collectAutofillContentService["containsChildField"](div)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true when the element contains a textarea descendant", () => {
|
||||
const div = document.createElement("div");
|
||||
div.innerHTML = `<textarea></textarea>`;
|
||||
|
||||
expect(collectAutofillContentService["containsChildField"](div)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when the element contains no form field descendants", () => {
|
||||
const div = document.createElement("div");
|
||||
div.innerHTML = `<span>Helper text</span>`;
|
||||
|
||||
expect(collectAutofillContentService["containsChildField"](div)).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when the node is a text node", () => {
|
||||
const textNode = document.createTextNode("Enter Country Code");
|
||||
|
||||
expect(collectAutofillContentService["containsChildField"](textNode)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isNewSectionElement", () => {
|
||||
const validElementTags = [
|
||||
"html",
|
||||
|
||||
@ -36,6 +36,7 @@ import {
|
||||
} from "./abstractions/collect-autofill-content.service";
|
||||
import { DomElementVisibilityService } from "./abstractions/dom-element-visibility.service";
|
||||
import { DomQueryService } from "./abstractions/dom-query.service";
|
||||
import { AutoFillConstants } from "./autofill-constants";
|
||||
|
||||
export class CollectAutofillContentService implements CollectAutofillContentServiceInterface {
|
||||
private readonly sendExtensionMessage = sendExtensionMessage;
|
||||
@ -662,6 +663,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
|
||||
break;
|
||||
}
|
||||
|
||||
if (this.containsChildField(currentElement)) {
|
||||
break;
|
||||
}
|
||||
|
||||
const textContent = this.getTextContentFromElement(currentElement);
|
||||
if (textContent) {
|
||||
labelTextContent.push(textContent);
|
||||
@ -712,6 +717,18 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
|
||||
: null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether any of an element's descendants are form fields.
|
||||
*/
|
||||
private containsChildField(element: Node): boolean {
|
||||
if (nodeIsElement(element)) {
|
||||
const fields = AutoFillConstants.FieldElements.join(", ");
|
||||
return !!element.querySelector(fields);
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the element's tag indicates that a transition to a new section of the
|
||||
* page is occurring. If so, we should not use the element or its children in order
|
||||
@ -794,6 +811,10 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
|
||||
return textContentItems;
|
||||
}
|
||||
|
||||
if (this.containsChildField(currentElement)) {
|
||||
return textContentItems;
|
||||
}
|
||||
|
||||
const textContent = this.getTextContentFromElement(currentElement);
|
||||
if (textContent) {
|
||||
textContentItems.push(textContent);
|
||||
@ -813,11 +834,15 @@ export class CollectAutofillContentService implements CollectAutofillContentServ
|
||||
let siblingElement = nodeIsElement(currentElement)
|
||||
? currentElement.previousElementSibling
|
||||
: currentElement.previousSibling;
|
||||
while (siblingElement?.lastChild && !this.isNewSectionElement(siblingElement)) {
|
||||
while (
|
||||
siblingElement?.lastChild &&
|
||||
!this.isNewSectionElement(siblingElement) &&
|
||||
!this.containsChildField(siblingElement)
|
||||
) {
|
||||
siblingElement = siblingElement.lastChild;
|
||||
}
|
||||
|
||||
if (this.isNewSectionElement(siblingElement)) {
|
||||
if (this.isNewSectionElement(siblingElement) || this.containsChildField(siblingElement)) {
|
||||
return textContentItems;
|
||||
}
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user