Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form] Always scan shadow elements when categorizing the form inputs #703

Merged
merged 13 commits into from
Nov 22, 2024
Merged
51 changes: 28 additions & 23 deletions dist/autofill-debug.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 28 additions & 23 deletions dist/autofill.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 27 additions & 6 deletions src/Form/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
shouldLog,
safeRegexTest,
getActiveElement,
getFormElements,
queryFormElements,
findElementsInShadowTree,
} from '../autofill-utils.js';

import { getInputSubtype, getInputMainType, createMatching, getInputVariant } from './matching.js';
Expand Down Expand Up @@ -386,16 +387,36 @@ class Form {
}
}

/**
* The function looks for form's control elements, and returns them if they're iterable.
* @param {*} selector
*/
getFormControlElements(selector) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put this in the utils as well. This class is already gigantic.

// Some sites seem to be overriding `form.elements`, so we need to check if it's still iterable.
if (this.form instanceof HTMLFormElement && this.form.elements != null && Symbol.iterator in Object(this.form.elements)) {
// For form elements we use .elements to catch fields outside the form itself using the form attribute.
// It also catches all elements when the markup is broken.
// We use .filter to avoid specific types of elements.
const formControls = [...this.form.elements].filter((el) => el.matches(selector));
return [...formControls];
} else {
return null;
}
}

Copy link
Collaborator Author

@dbajpeyi dbajpeyi Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function now has a clear separate purpose, that's different from querying the elements which is what we rather do in most other cases.

categorizeInputs() {
const selector = this.matching.cssSelector('formInputsSelector');
// If there's no form container and it's just a lonely input field (this.form is an input field)
if (this.form.matches(selector)) {
this.addInput(this.form);
} else {
/** @type {Element[] | NodeList} */

// Also scan the form for shadow elements
const foundInputs = getFormElements(this.form, selector, true, true);
// Attempt to get form's control elements first as it can catch elements when markup is broke, or if the fields are outside the form.
// Other wise use queryFormElements, that can scan for shadow tree.
const formControlElements = this.getFormControlElements(selector);
const foundInputs =
formControlElements != null
? [...formControlElements, ...findElementsInShadowTree(this.form, selector)]
: queryFormElements(this.form, selector, true);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's clear here now that we need to try to get elements first using .elements for a specific reason. Otherwise we do regular querying (with shadow true)


if (foundInputs.length < MAX_INPUTS_PER_FORM) {
foundInputs.forEach((input) => this.addInput(input));
Expand Down Expand Up @@ -465,7 +486,7 @@ class Form {

get submitButtons() {
const selector = this.matching.cssSelector('submitButtonSelector');
const allButtons = /** @type {HTMLElement[]} */ (getFormElements(this.form, selector));
const allButtons = /** @type {HTMLElement[]} */ (queryFormElements(this.form, selector));
return allButtons.filter(
(btn) => isPotentiallyViewable(btn) && isLikelyASubmitButton(btn, this.matching) && buttonMatchesFormType(btn, this),
);
Expand Down
4 changes: 2 additions & 2 deletions src/Form/FormAnalyzer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { removeExcessWhitespace, Matching } from './matching.js';
import { constants } from '../constants.js';
import { matchingConfiguration } from './matching-config/__generated__/compiled-matching-config.js';
import { findElementsInShadowTree, getFormElements, getTextShallow, isLikelyASubmitButton, safeRegexTest } from '../autofill-utils.js';
import { findElementsInShadowTree, queryFormElements, getTextShallow, isLikelyASubmitButton, safeRegexTest } from '../autofill-utils.js';

class FormAnalyzer {
/** @type HTMLElement */
Expand Down Expand Up @@ -311,7 +311,7 @@ class FormAnalyzer {

// Check form contents (noisy elements are skipped with the safeUniversalSelector)
const selector = this.matching.cssSelector('safeUniversalSelector');
const formElements = getFormElements(this.form, selector);
const formElements = queryFormElements(this.form, selector);
for (let i = 0; i < formElements.length; i++) {
// Safety cutoff to avoid huge DOMs freezing the browser
if (i >= 200) break;
Expand Down
18 changes: 4 additions & 14 deletions src/autofill-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -589,30 +589,20 @@ function findElementsInShadowTree(root, selector) {

/**
* Default operation: finds elements using querySelectorAll.
* Optionally, if forced an form.elements is iterable, attempts to find elements using .elements instead,
* to catch field outside the form itsel using the form attribute. It also catches all elements
* when the markup is broken. We use .filter along with a selector to avoid any unwanted elements.
* Optionally, can be forced to scan the shadow tree.
* @param {string} selector
* @param {boolean} forceScanShadowTree
* @param {boolean} shouldCheckFormControls
* @returns {Element[]}
*/
function getFormElements(form, selector, forceScanShadowTree = false, shouldCheckFormControls = false) {
function queryFormElements(form, selector, forceScanShadowTree = false) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note this needs to be in the utils as it's also used outside of Form class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point this is function no longer has anything to do with forms, right? It's just a wrapper around querySelectorAll that also works with the shadow tree. Should we rename it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah for sure!

// Some sites seem to be overriding `form.elements`, so we need to check if it's still iterable.
/** @type {Element[]|NodeListOf<Element>} element */
let formElements = [];
if (shouldCheckFormControls && form instanceof HTMLFormElement && form.elements != null && Symbol.iterator in Object(form.elements)) {
formElements = [...form.elements].filter((el) => el.matches(selector));
} else {
formElements = form.querySelectorAll(selector);
}
const formElements = form.querySelectorAll(selector);

if (forceScanShadowTree || formElements.length === 0) {
return [...formElements, ...findElementsInShadowTree(form, selector)];
} else {
return [...formElements];
}
return [...formElements];
}

export {
Expand Down Expand Up @@ -648,5 +638,5 @@ export {
pierceShadowTree,
getActiveElement,
findElementsInShadowTree,
getFormElements,
queryFormElements,
};
Loading
Loading