diff --git a/browser/extensions/formautofill/test/browser/heuristics/browser.toml b/browser/extensions/formautofill/test/browser/heuristics/browser.toml index b79ee229b7f9..37f0b55bc3a5 100644 --- a/browser/extensions/formautofill/test/browser/heuristics/browser.toml +++ b/browser/extensions/formautofill/test/browser/heuristics/browser.toml @@ -22,6 +22,8 @@ skip-if = ["apple_silicon && !debug"] ["browser_ignore_invisible_fields.js"] +["browser_ignore_unfocusable_fields.js"] + ["browser_label_rules.js"] ["browser_multiple_section.js"] diff --git a/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_invisible_fields.js b/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_invisible_fields.js index d22b1d5031f0..5e55fa6e6912 100644 --- a/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_invisible_fields.js +++ b/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_invisible_fields.js @@ -7,7 +7,14 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ add_heuristic_tests([ { - description: "all fields are visible", + description: + "All fields are visible (interactivityCheckMode is set to visibility).", + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "visibility", + ], + ], fixtureData: ` @@ -44,7 +51,14 @@ add_heuristic_tests([ ], }, { - description: "some fields are invisible because of css style", + description: + "Some fields are invisible due to css styling (interactivityCheckMode is set to visibility).", + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "visibility", + ], + ], fixtureData: ` @@ -52,9 +66,9 @@ add_heuristic_tests([ - - - + + +
@@ -71,15 +85,22 @@ add_heuristic_tests([ { fieldName: "name" }, { fieldName: "tel" }, { fieldName: "email" }, + { fieldName: "country" }, ], }, ], }, { - // hidden and style="display:none" are always considered regardless what visibility check we use + // hidden, style="display:none" are always considered (when mode visibility) description: - "invisible fields are identified because number of elemenent in the form exceed the threshold", - prefs: [["extensions.formautofill.heuristics.visibilityCheckThreshold", 1]], + "Number of form elements exceeds the threshold (interactivityCheckMode is set to visibility).", + prefs: [ + ["extensions.formautofill.heuristics.visibilityCheckThreshold", 1], + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "visibility", + ], + ], fixtureData: ` @@ -87,9 +108,9 @@ add_heuristic_tests([ - - - + + +
@@ -106,7 +127,7 @@ add_heuristic_tests([ { fieldName: "name" }, { fieldName: "tel" }, { fieldName: "email" }, - { fieldName: "address-line1" }, + { fieldName: "country" }, { fieldName: "address-line2" }, ], }, diff --git a/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_unfocusable_fields.js b/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_unfocusable_fields.js new file mode 100644 index 000000000000..6f762b2ceec3 --- /dev/null +++ b/browser/extensions/formautofill/test/browser/heuristics/browser_ignore_unfocusable_fields.js @@ -0,0 +1,179 @@ +/* Any copyright is dedicated to the Public Domain. +http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* global add_heuristic_tests */ + +"use strict"; + +add_heuristic_tests([ + { + description: "All visual fields are considered focusable.", + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "focusability", + ], + ], + fixtureData: ` + + +
+ + + + + + +
+ +
+
+ + + + `, + expectedResult: [ + { + default: { + reason: "autocomplete", + }, + fields: [ + { fieldName: "name" }, + { fieldName: "tel" }, + { fieldName: "email" }, + { fieldName: "country" }, + { fieldName: "postal-code" }, + { fieldName: "address-line1" }, + { fieldName: "address-line2" }, + ], + }, + ], + }, + { + // ignore opacity (see Bug 1835852), + description: + "Invisible fields with style.opacity=0 set are considered focusable.", + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "focusability", + ], + ], + fixtureData: ` + + +
+ + + + + + +
+ +
+
+ + + + `, + expectedResult: [ + { + default: { + reason: "autocomplete", + }, + fields: [ + { fieldName: "name" }, + { fieldName: "tel" }, + { fieldName: "email" }, + { fieldName: "country" }, + { fieldName: "postal-code" }, + { fieldName: "address-line1" }, + { fieldName: "address-line2" }, + ], + }, + ], + }, + { + description: + "Some fields are considered unfocusable due to their invisibility.", + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "focusability", + ], + ], + fixtureData: ` + + +
+ + + + + + +
+ +
+
+ + + `, + expectedResult: [ + { + default: { + reason: "autocomplete", + }, + fields: [ + { fieldName: "name" }, + { fieldName: "tel" }, + { fieldName: "email" }, + { fieldName: "country" }, + ], + }, + ], + }, + { + description: `Disabled field and field with tabindex="-1" is considered unfocusable`, + prefs: [ + [ + "extensions.formautofill.heuristics.interactivityCheckMode", + "focusability", + ], + ], + fixtureData: ` + + +
+ + + + + + + +
+ + + `, + expectedResult: [ + { + default: { + reason: "autocomplete", + }, + fields: [ + { fieldName: "name" }, + { fieldName: "tel" }, + { fieldName: "email" }, + { fieldName: "address-line1" }, + { fieldName: "address-line2" }, + ], + }, + ], + }, +]); diff --git a/browser/extensions/formautofill/test/unit/head.js b/browser/extensions/formautofill/test/unit/head.js index 4b37592d9ae1..7b01a84da01a 100644 --- a/browser/extensions/formautofill/test/unit/head.js +++ b/browser/extensions/formautofill/test/unit/head.js @@ -71,6 +71,24 @@ region-name-tw = Taiwan L10nRegistry.getInstance().registerSources([mockSource]); } +/** + * Mock the return value of Services.focus.elementIsFocusable + * since a field's focusability can't be tested in a unit test. + */ +(function ignoreAFieldsFocusability() { + let stub = sinon.stub(Services, "focus").get(() => { + return { + elementIsFocusable() { + return true; + }, + }; + }); + + registerCleanupFunction(() => { + stub.restore(); + }); +})(); + do_get_profile(); const EXTENSION_ID = "formautofill@mozilla.org"; diff --git a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js index fd5d6e6d4c83..49ffb0083a00 100644 --- a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js +++ b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js @@ -421,39 +421,6 @@ const TESTCASES = [ "cc-exp-year": "25", }, }, - { - description: - "Form with hidden input and visible input that share the same autocomplete attribute", - document: `
- - - - - - - - -
`, - focusedInputId: "visible-cc", - profileData: { - guid: "123", - "cc-number": "4111111111111111", - "cc-name": "test name", - "cc-exp-month": 6, - "cc-exp-year": 25, - }, - expectedResult: { - guid: "123", - "visible-cc": "4111111111111111", - "visible-name": "test name", - "cc-exp-month": "06", - "cc-exp-year": "25", - "hidden-cc": undefined, - "hidden-cc-2": undefined, - "hidden-name": undefined, - "hidden-name-2": undefined, - }, - }, { description: "Fill credit card fields in a form where the value property is being used as a placeholder for cardholder name", diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index ca937f8e6d24..0a36ca2e2a3e 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -3986,6 +3986,9 @@ pref("extensions.formautofill.creditCards.heuristics.fathom.testConfidence", "0" pref("extensions.formautofill.firstTimeUse", true); pref("extensions.formautofill.loglevel", "Warn"); +// The interactivityCheckMode pref is only temporary. +// It will be removed when we decide to only support the `focusability` mode +pref("extensions.formautofill.heuristics.interactivityCheckMode", "focusability"); pref("toolkit.osKeyStore.loglevel", "Warn"); diff --git a/toolkit/components/formautofill/Constants.ios.mjs b/toolkit/components/formautofill/Constants.ios.mjs index 619c1b9aadb9..be92dade608d 100644 --- a/toolkit/components/formautofill/Constants.ios.mjs +++ b/toolkit/components/formautofill/Constants.ios.mjs @@ -27,9 +27,10 @@ const IOS_DEFAULT_PREFERENCES = { "extensions.formautofill.addresses.ignoreAutocompleteOff": true, "extensions.formautofill.heuristics.enabled": true, "extensions.formautofill.section.enabled": true, - // WebKit doesn't support the checkVisibility API, setting the threshold value to 0 to esnure + // WebKit doesn't support the checkVisibility API, setting the threshold value to 0 to ensure // `IsFieldVisible` function doesn't use it "extensions.formautofill.heuristics.visibilityCheckThreshold": 0, + "extensions.formautofill.heuristics.interactivityCheckMode": "focusability", "extensions.formautofill.focusOnAutofill": false, }; diff --git a/toolkit/components/formautofill/Helpers.ios.mjs b/toolkit/components/formautofill/Helpers.ios.mjs index b8992deb1028..9214a2767a3b 100644 --- a/toolkit/components/formautofill/Helpers.ios.mjs +++ b/toolkit/components/formautofill/Helpers.ios.mjs @@ -123,10 +123,23 @@ export const OSKeyStore = withNotImplementedError({ ensureLoggedIn: () => true, }); +// Checks an element's focusability and accessibility via keyboard navigation +const checkFocusability = element => { + return ( + !element.disabled && + !element.hidden && + element.style.display != "none" && + element.tabIndex != "-1" + ); +}; + // Define mock for Services // NOTE: Services is a global so we need to attach it to the window // eslint-disable-next-line no-shadow export const Services = withNotImplementedError({ + focus: withNotImplementedError({ + elementIsFocusable: checkFocusability, + }), intl: withNotImplementedError({ getAvailableLocaleDisplayNames: () => [], getRegionDisplayNames: () => [], diff --git a/toolkit/components/formautofill/shared/FormAutofillHeuristics.sys.mjs b/toolkit/components/formautofill/shared/FormAutofillHeuristics.sys.mjs index 5012a3da7252..da6671efc6e3 100644 --- a/toolkit/components/formautofill/shared/FormAutofillHeuristics.sys.mjs +++ b/toolkit/components/formautofill/shared/FormAutofillHeuristics.sys.mjs @@ -550,25 +550,7 @@ export const FormAutofillHeuristics = { * all sections within its field details in the form. */ getFormInfo(form) { - let elements = Array.from(form.elements).filter(element => - lazy.FormAutofillUtils.isCreditCardOrAddressFieldType(element) - ); - - // Due to potential performance impact while running visibility check on - // a large amount of elements, a comprehensive visibility check - // (considering opacity and CSS visibility) is only applied when the number - // of eligible elements is below a certain threshold. - const runVisiblityCheck = - elements.length < lazy.FormAutofillUtils.visibilityCheckThreshold; - if (!runVisiblityCheck) { - lazy.log.debug( - `Skip running visibility check, because of too many elements (${elements.length})` - ); - } - - elements = elements.filter(element => - lazy.FormAutofillUtils.isFieldVisible(element, runVisiblityCheck) - ); + let elements = this.getFormElements(form); const scanner = new lazy.FieldScanner(elements, element => this.inferFieldInfo(element, elements) @@ -617,6 +599,44 @@ export const FormAutofillHeuristics = { ); }, + /** + * Get form elements that are of credit card or address type and filtered by either + * visibility or focusability - depending on the interactivity mode (default = focusability) + * This distinction is only temporary as we want to test switching from visibility mode + * to focusability mode. The visibility mode is then removed. + * + * @param {HTMLElement} form + * @returns {Array} elements filtered by interactivity mode (visibility or focusability) + */ + getFormElements(form) { + let elements = Array.from(form.elements).filter(element => + lazy.FormAutofillUtils.isCreditCardOrAddressFieldType(element) + ); + const interactivityMode = lazy.FormAutofillUtils.interactivityCheckMode; + + if (interactivityMode == "focusability") { + elements = elements.filter(element => + lazy.FormAutofillUtils.isFieldFocusable(element) + ); + } else if (interactivityMode == "visibility") { + // Due to potential performance impact while running visibility check on + // a large amount of elements, a comprehensive visibility check + // (considering opacity and CSS visibility) is only applied when the number + // of eligible elements is below a certain threshold. + const runVisiblityCheck = + elements.length < lazy.FormAutofillUtils.visibilityCheckThreshold; + if (!runVisiblityCheck) { + lazy.log.debug( + `Skip running visibility check, because of too many elements (${elements.length})` + ); + } + elements = elements.filter(element => + lazy.FormAutofillUtils.isFieldVisible(element, runVisiblityCheck) + ); + } + return elements; + }, + /** * The result is an array contains the sections with its belonging field details. * diff --git a/toolkit/components/formautofill/shared/FormAutofillUtils.sys.mjs b/toolkit/components/formautofill/shared/FormAutofillUtils.sys.mjs index 795f0a5d35d3..54756221c351 100644 --- a/toolkit/components/formautofill/shared/FormAutofillUtils.sys.mjs +++ b/toolkit/components/formautofill/shared/FormAutofillUtils.sys.mjs @@ -438,7 +438,7 @@ FormAutofillUtils = { * @returns {boolean} true if the element is visible */ isFieldVisible(element, visibilityCheck = true) { - if (visibilityCheck) { + if (visibilityCheck && element.checkVisibility) { return element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true, @@ -448,6 +448,23 @@ FormAutofillUtils = { return !element.hidden && element.style.display != "none"; }, + /** + * Determines if an element is focusable + * and accessible via keyboard navigation or not. + * + * @param {HTMLElement} element + * + * @returns {bool} true if the element is focusable and accessible + */ + isFieldFocusable(element) { + return ( + // The Services.focus.elementIsFocusable API considers elements with + // tabIndex="-1" set as focusable. But since they are not accessible + // via keyboard navigation we treat them as non-interactive + Services.focus.elementIsFocusable(element, 0) && element.tabIndex != "-1" + ); + }, + /** * Determines if an element is eligible to be used by credit card or address autofill. * @@ -1226,6 +1243,13 @@ XPCOMUtils.defineLazyPreferenceGetter( 200 ); +XPCOMUtils.defineLazyPreferenceGetter( + FormAutofillUtils, + "interactivityCheckMode", + "extensions.formautofill.heuristics.interactivityCheckMode", + "focusability" +); + // This is only used in iOS XPCOMUtils.defineLazyPreferenceGetter( FormAutofillUtils,