tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit c5c90264f9edc7971d2ac8e9d0b54a5a374e99b6
parent eee6488f783ab72b1afc95c0a824629b25e1458b
Author: agoloman <agoloman@mozilla.com>
Date:   Thu, 30 Oct 2025 16:36:58 +0200

Revert "Bug 1968086, trigger the formautofill field identification from the form fill controller instead of from a separate focus event, and wait for the field identification to complete before showing a popup, r=dimi,credential-management-reviewers" for causing mochitest failures @test_autocomplete_change_after_focus.html.

This reverts commit 530d18a10cf5f9abd1b5448490acb245de4e7440.

Diffstat:
Mbrowser/extensions/formautofill/api.js | 2+-
Mbrowser/extensions/formautofill/test/browser/browser_email_dropdown.js | 64----------------------------------------------------------------
Mtoolkit/components/formautofill/FormAutofillChild.sys.mjs | 218+++++++++++++++++++++++++++++++++----------------------------------------------
Mtoolkit/components/passwordmgr/LoginManagerChild.sys.mjs | 8++++----
Mtoolkit/components/satchel/nsFormFillController.cpp | 80+++++++++++++------------------------------------------------------------------
Mtoolkit/components/satchel/nsFormFillController.h | 9---------
Mtoolkit/components/satchel/nsIFormFillController.idl | 18------------------
7 files changed, 110 insertions(+), 289 deletions(-)

diff --git a/browser/extensions/formautofill/api.js b/browser/extensions/formautofill/api.js @@ -147,7 +147,7 @@ this.formautofill = class extends ExtensionAPI { child: { esModuleURI: "resource://autofill/FormAutofillChild.sys.mjs", events: { - focusin: { capture: true }, + focusin: {}, "form-changed": { createActor: false }, "form-submission-detected": { createActor: false }, }, diff --git a/browser/extensions/formautofill/test/browser/browser_email_dropdown.js b/browser/extensions/formautofill/test/browser/browser_email_dropdown.js @@ -7,9 +7,6 @@ const PAGE_URL = // login manager and formautofill providers, that if an address is saved, // that the formautofill popup gets priority over the login manager. -// The first two tests check what happens when the field is focused and the -// popup is manually opened with the keyboard. - add_task(async function test_email_field_is_address_dropdown() { await SpecialPowers.pushPrefEnv({ set: [["signon.rememberSignons", true]], @@ -60,64 +57,3 @@ add_task( ); } ); - -// The next two tests check what happens when the field is focused but the -// popup is not manually opened. - -add_task(async function test_email_field_is_address_dropdown_onfocus() { - // However, if no addresses are saved, show the login manager. - await removeAllRecords(); - - await SpecialPowers.pushPrefEnv({ - set: [["signon.rememberSignons", true]], - }); - // If an address is saved, show the formautofill dropdown. - await setStorage(TEST_ADDRESS_1); - - await BrowserTestUtils.withNewTab( - { gBrowser, url: PAGE_URL }, - async function (browser) { - // Note that at present the popup will appear on focus because - // it could be a login form, even through the address items appear - // in the popup menu. - await SpecialPowers.spawn(browser, [], () => { - content.document.getElementById("email").focus(); - }); - await runAndWaitForAutocompletePopupOpen(browser, () => {}); - const item = getDisplayedPopupItems(browser)[2]; - - is( - item.getAttribute("ac-value"), - "Manage addresses", - "Address popup should show a valid email suggestion" - ); - - await closePopup(browser); - } - ); -}); - -add_task( - async function test_email_field_shows_login_dropdown_when_no_saved_address_onfocus() { - // However, if no addresses are saved, show the login manager. - await removeAllRecords(); - await BrowserTestUtils.withNewTab( - { gBrowser, url: PAGE_URL }, - async function (browser) { - await SpecialPowers.spawn(browser, [], () => { - content.document.getElementById("email").focus(); - }); - await runAndWaitForAutocompletePopupOpen(browser, () => {}); - const item = getDisplayedPopupItems(browser)[0]; - - is( - item.getAttribute("ac-value"), - "Manage Passwords", - "Login Manager should be shown" - ); - - await closePopup(browser); - } - ); - } -); diff --git a/toolkit/components/formautofill/FormAutofillChild.sys.mjs b/toolkit/components/formautofill/FormAutofillChild.sys.mjs @@ -31,18 +31,6 @@ ChromeUtils.defineESModuleGetters(lazy, { setTimeout: "resource://gre/modules/Timer.sys.mjs", }); -class FormFillFocusListener { - handleFocus(element) { - let actor = - element.ownerGlobal?.windowGlobalChild?.getActor("FormAutofill"); - return actor?.handleFocus(element); - } - - QueryInterface = ChromeUtils.generateQI(["nsIFormFillFocusListener"]); -} - -let gFormFillFocusListener; - /** * Handles content's interactions for the frame. */ @@ -51,7 +39,7 @@ export class FormAutofillChild extends JSWindowActorChild { * Keep track of autofill handlers that are waiting for the parent process * to send back the identified result. */ - #handlerWaitingForDetectedComplete = new Map(); + #handlerWaitingForDetectedComplete = new Set(); /** * Keep track of handler that are waiting for the @@ -102,97 +90,92 @@ export class FormAutofillChild extends JSWindowActorChild { * is run due to a form change */ onFieldsDetectedComplete(fieldDetails, isUpdate = false) { - let fieldsDetectedResolver; - - try { - const handler = this._fieldDetailsManager.getFormHandlerByRootElementId( - fieldDetails[0].rootElementId - ); + if (!fieldDetails.length) { + return; + } - fieldsDetectedResolver = - this.#handlerWaitingForDetectedComplete.get(handler); - this.#handlerWaitingForDetectedComplete.delete(handler); + const handler = this._fieldDetailsManager.getFormHandlerByRootElementId( + fieldDetails[0].rootElementId + ); + this.#handlerWaitingForDetectedComplete.delete(handler); - if (isUpdate) { - if (this.#handlerWaitingForFormSubmissionComplete.has(handler)) { - // The form change was detected before the form submission, but was probably initiated - // by it, so don't touch the fieldDetails in this case. - return; - } - handler.updateFormByElement(fieldDetails[0].element); - this._fieldDetailsManager.addFormHandlerByElementEntries(handler); + if (isUpdate) { + if (this.#handlerWaitingForFormSubmissionComplete.has(handler)) { + // The form change was detected before the form submission, but was probably initiated + // by it, so don't touch the fieldDetails in this case. + return; } + handler.updateFormByElement(fieldDetails[0].element); + this._fieldDetailsManager.addFormHandlerByElementEntries(handler); + } - handler.setIdentifiedFieldDetails(fieldDetails); - handler.setUpDynamicFormChangeObserver(); + handler.setIdentifiedFieldDetails(fieldDetails); + handler.setUpDynamicFormChangeObserver(); - let addressFields = []; - let creditcardFields = []; + let addressFields = []; + let creditcardFields = []; - handler.fieldDetails.forEach(fd => { - if (lazy.FormAutofillUtils.isAddressField(fd.fieldName)) { - addressFields.push(fd); - } else if (lazy.FormAutofillUtils.isCreditCardField(fd.fieldName)) { - creditcardFields.push(fd); - } - }); - - // Bug 1905040. This is only a temporarily workaround for now to skip marking address fields - // autocompletable whenever we detect an address field. We only mark address field when - // it is a valid address section (This is done in the parent) - const addressFieldSet = new Set(addressFields.map(fd => fd.fieldName)); - if ( - addressFieldSet.size < lazy.FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD - ) { - addressFields = []; + handler.fieldDetails.forEach(fd => { + if (lazy.FormAutofillUtils.isAddressField(fd.fieldName)) { + addressFields.push(fd); + } else if (lazy.FormAutofillUtils.isCreditCardField(fd.fieldName)) { + creditcardFields.push(fd); } + }); - // Inform the autocomplete controller these fields are autofillable - [...addressFields, ...creditcardFields].forEach(fieldDetail => { - this.#markAsAutofillField(fieldDetail); - - if ( - fieldDetail.element == lazy.FormAutofillContent.controlledElement && - !isUpdate - ) { - this.showPopupIfEmpty(fieldDetail.element, fieldDetail.fieldName); - } - }); + // Bug 1905040. This is only a temporarily workaround for now to skip marking address fields + // autocompletable whenever we detect an address field. We only mark address field when + // it is a valid address section (This is done in the parent) + const addressFieldSet = new Set(addressFields.map(fd => fd.fieldName)); + if ( + addressFieldSet.size < lazy.FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD + ) { + addressFields = []; + } - if (isUpdate) { - // The fields detection was re-run because of a form change, this means - // FormAutofillChild already registered its interest in form submissions - // in the initial field detection process - return; - } + // Inform the autocomplete controller these fields are autofillable + [...addressFields, ...creditcardFields].forEach(fieldDetail => { + this.#markAsAutofillField(fieldDetail); - // Do not need to listen to form submission event because if the address fields do not contain - // 'street-address' or `address-linx`, we will not save the address. if ( - creditcardFields.length || - (addressFields.length && - [ - "street-address", - "address-line1", - "address-line2", - "address-line3", - ].some(fieldName => addressFieldSet.has(fieldName))) + fieldDetail.element == lazy.FormAutofillContent.controlledElement && + !isUpdate ) { - this.manager - .getActor("FormHandler") - .registerFormSubmissionInterest(this, { - includesFormRemoval: lazy.FormAutofill.captureOnFormRemoval, - includesPageNavigation: lazy.FormAutofill.captureOnPageNavigation, - }); + this.showPopupIfEmpty(fieldDetail.element, fieldDetail.fieldName); + } + }); - // TODO (Bug 1901486): Integrate pagehide to FormHandler. - if (!this._hasRegisteredPageHide.has(handler)) { - this.registerPageHide(handler); - this._hasRegisteredPageHide.add(true); - } + if (isUpdate) { + // The fields detection was re-run because of a form change, this means + // FormAutofillChild already registered its interest in form submissions + // in the initial field detection process + return; + } + + // Do not need to listen to form submission event because if the address fields do not contain + // 'street-address' or `address-linx`, we will not save the address. + if ( + creditcardFields.length || + (addressFields.length && + [ + "street-address", + "address-line1", + "address-line2", + "address-line3", + ].some(fieldName => addressFieldSet.has(fieldName))) + ) { + this.manager + .getActor("FormHandler") + .registerFormSubmissionInterest(this, { + includesFormRemoval: lazy.FormAutofill.captureOnFormRemoval, + includesPageNavigation: lazy.FormAutofill.captureOnPageNavigation, + }); + + // TODO (Bug 1901486): Integrate pagehide to FormHandler. + if (!this._hasRegisteredPageHide.has(handler)) { + this.registerPageHide(handler); + this._hasRegisteredPageHide.add(true); } - } finally { - fieldsDetectedResolver?.(); } } @@ -255,7 +238,7 @@ export class FormAutofillChild extends JSWindowActorChild { // Bail out if the child process is still waiting for the parent to send a // `onFieldsDetectedComplete` or `onFieldsUpdatedComplete` message, // or a form submission is currently still getting processed. - return null; + return; } if (handler.fillOnFormChangeData.isWithinDynamicFormChangeThreshold) { @@ -263,7 +246,7 @@ export class FormAutofillChild extends JSWindowActorChild { // initiated by a user but by the site due to the form change. Bail out here, // because we will receive the form-changed-event anyway and should not process the // field detection here, since this would block the second autofill process. - return null; + return; } // Bail out if there is nothing changed since last time we identified this element @@ -295,22 +278,18 @@ export class FormAutofillChild extends JSWindowActorChild { ) ) { handler.setIdentifiedFieldDetails(detectedFields); - return null; + return; } - return new Promise(resolve => { - this.sendAsyncMessage( - "FormAutofill:OnFieldsDetected", - detectedFields.map(field => field.toVanillaObject()) - ); + this.sendAsyncMessage( + "FormAutofill:OnFieldsDetected", + detectedFields.map(field => field.toVanillaObject()) + ); - // Notify the parent about the newly identified fields because - // the autofill section information is maintained on the parent side. - this.#handlerWaitingForDetectedComplete.set(handler, resolve); - }); + // Notify the parent about the newly identified fields because + // the autofill section information is maintained on the parent side. + this.#handlerWaitingForDetectedComplete.add(handler); } - - return null; } /** @@ -360,7 +339,7 @@ export class FormAutofillChild extends JSWindowActorChild { if (detectedFields.length) { // This actor should receive `onFieldsDetectedComplete`message after // `idenitfyFields` is called - this.#handlerWaitingForDetectedComplete.set(handler, null); + this.#handlerWaitingForDetectedComplete.add(handler); } return detectedFields; } @@ -475,22 +454,9 @@ export class FormAutofillChild extends JSWindowActorChild { switch (evt.type) { case "focusin": { - if (AppConstants.MOZ_GECKOVIEW) { - this.handleFocus(evt.target); - break; - } - - if (!gFormFillFocusListener) { - gFormFillFocusListener = new FormFillFocusListener(); - - const formFillController = Cc[ - "@mozilla.org/satchel/form-fill-controller;1" - ].getService(Ci.nsIFormFillController); - formFillController.addFocusListener(gFormFillFocusListener); - } + this.onFocusIn(evt.target); break; } - case "form-changed": { const { form, changes } = evt.detail; this.onFormChange(form, changes); @@ -508,7 +474,7 @@ export class FormAutofillChild extends JSWindowActorChild { } } - handleFocus(element) { + onFocusIn(element) { const handler = this._fieldDetailsManager.getFormHandler(element); // When autofilling and clearing a field, we focus on the element before modifying the value. // (See FormAutofillHandler.fillFieldValue and FormAutofillHandler.clearFilledFields). @@ -517,7 +483,7 @@ export class FormAutofillChild extends JSWindowActorChild { !lazy.FormAutofillUtils.isCreditCardOrAddressFieldType(element) || handler?.isAutofillInProgress ) { - return null; + return; } const doc = element.ownerDocument; @@ -528,25 +494,25 @@ export class FormAutofillChild extends JSWindowActorChild { this._hasDOMContentLoadedHandler = true; doc.addEventListener( "DOMContentLoaded", - () => this.handleFocus(lazy.FormAutofillContent.controlledElement), + () => this.onFocusIn(lazy.FormAutofillContent.controlledElement), { once: true } ); } - return null; + return; } if ( AppConstants.MOZ_GECKOVIEW || !lazy.FormAutofillContent.savedFieldNames ) { - this.debug("handleFocus: savedFieldNames are not known yet"); + this.debug("onFocusIn: savedFieldNames are not known yet"); // Init can be asynchronous because we don't need anything from the parent // at this point. this.sendAsyncMessage("FormAutofill:InitStorage"); } - return this.identifyFieldsWhenFocused(element); + this.identifyFieldsWhenFocused(element); } /** @@ -682,7 +648,7 @@ export class FormAutofillChild extends JSWindowActorChild { mergedFields.map(field => field.toVanillaObject()) ); - this.#handlerWaitingForDetectedComplete.set(handler, null); + this.#handlerWaitingForDetectedComplete.add(handler); if ( lazy.FormAutofill.fillOnDynamicFormChanges && diff --git a/toolkit/components/passwordmgr/LoginManagerChild.sys.mjs b/toolkit/components/passwordmgr/LoginManagerChild.sys.mjs @@ -150,7 +150,7 @@ const observer = { this.handleKeydown(aEvent, field, loginManagerChild, ownerDocument); break; - case "focusin": + case "focus": this.handleFocus(field, docState, aEvent.target); break; @@ -616,7 +616,7 @@ export class LoginFormState { this.generatedPasswordFields.add(passwordField); // blur/focus: listen for focus changes to we can mask/unmask generated passwords - for (let eventType of ["blur", "focusin"]) { + for (let eventType of ["blur", "focus"]) { passwordField.addEventListener(eventType, observer, { capture: true, mozSystemGroup: true, @@ -668,7 +668,7 @@ export class LoginFormState { this.generatedPasswordFields.delete(passwordField); // Remove all the event listeners added in _passwordEditedOrGenerated - for (let eventType of ["blur", "focusin"]) { + for (let eventType of ["blur", "focus"]) { passwordField.removeEventListener(eventType, observer, { capture: true, mozSystemGroup: true, @@ -3108,7 +3108,7 @@ export class LoginManagerChild extends JSWindowActorChild { if (usernameField) { lazy.log("Attaching event listeners to usernameField."); - usernameField.addEventListener("focusin", observer); + usernameField.addEventListener("focus", observer); usernameField.addEventListener("mousedown", observer); } diff --git a/toolkit/components/satchel/nsFormFillController.cpp b/toolkit/components/satchel/nsFormFillController.cpp @@ -21,7 +21,6 @@ #include "mozilla/dom/KeyboardEventBinding.h" #include "mozilla/dom/MouseEvent.h" #include "mozilla/dom/PageTransitionEvent.h" -#include "mozilla/dom/Promise-inl.h" #include "mozilla/Logging.h" #include "mozilla/PresShell.h" #include "mozilla/Services.h" @@ -752,7 +751,7 @@ nsFormFillController::HandleEvent(Event* aEvent) { NS_ENSURE_STATE(internalEvent); switch (internalEvent->mMessage) { - case eFocusIn: + case eFocus: return Focus(aEvent); case eMouseDown: return MouseDown(aEvent); @@ -825,7 +824,7 @@ void nsFormFillController::AttachListeners(EventTarget* aEventTarget) { EventListenerManager* elm = aEventTarget->GetOrCreateListenerManager(); NS_ENSURE_TRUE_VOID(elm); - elm->AddEventListenerByType(this, u"focusin"_ns, TrustedEventsAtCapture()); + elm->AddEventListenerByType(this, u"focus"_ns, TrustedEventsAtCapture()); elm->AddEventListenerByType(this, u"blur"_ns, TrustedEventsAtCapture()); elm->AddEventListenerByType(this, u"pagehide"_ns, TrustedEventsAtCapture()); elm->AddEventListenerByType(this, u"mousedown"_ns, TrustedEventsAtCapture()); @@ -896,66 +895,27 @@ nsresult nsFormFillController::HandleFocus(Element* aElement) { // multiple input forms and the fact that a mousedown into an already focused // field does not trigger another focus. - bool shouldShowPopup = false; - - // If we have not seen a right click yet, just show the popup. - if (mControlledElement) { - if (HasBeenTypePassword(mControlledElement)) { - if (mLastRightClickTimeStamp.IsNull()) { - mPasswordPopupAutomaticallyOpened = true; - shouldShowPopup = true; - } else { - uint64_t timeDiff = - (TimeStamp::Now() - mLastRightClickTimeStamp).ToMilliseconds(); - if (timeDiff > mFocusAfterRightClickThreshold) { - shouldShowPopup = true; - } - } - } + if (!HasBeenTypePassword(mControlledElement)) { + return NS_OK; } - // Some handlers, such as the form fill component need time to identify - // which fields match which field types, so ask them to provide a promise - // which will resolve when this task is complete. This allows the - // autocomplete popup to be delayed until the field type is known. Note - // that this only handles popups that open when a field is focused; popups - // opened via, for example, a user keyboard action do not wait for this - // promise. - for (uint32_t idx = 0; idx < mFocusListeners.Length(); idx++) { - RefPtr<Promise> promise; - nsCOMPtr<nsIFormFillFocusListener> formFillFocus = mFocusListeners[idx]; - formFillFocus->HandleFocus(aElement, getter_AddRefs(promise)); - if (promise && promise->State() == Promise::PromiseState::Pending) { - // Cache the promise. If some other handler calls ShowPopup() - // it will also need to wait on this promise. - mFocusPendingPromise = promise; - WaitForPromise(shouldShowPopup); - return NS_OK; - } + // If we have not seen a right click yet, just show the popup. + if (mLastRightClickTimeStamp.IsNull()) { + mPasswordPopupAutomaticallyOpened = true; + ShowPopup(); + return NS_OK; } - if (shouldShowPopup) { + uint64_t timeDiff = + (TimeStamp::Now() - mLastRightClickTimeStamp).ToMilliseconds(); + if (timeDiff > mFocusAfterRightClickThreshold) { + mPasswordPopupAutomaticallyOpened = true; ShowPopup(); } return NS_OK; } -void nsFormFillController::WaitForPromise(bool showPopup) { - mFocusPendingPromise->AddCallbacksWithCycleCollectedArgs( - [showPopup](JSContext* aCx, JS::Handle<JS::Value> aValue, - ErrorResult& aRv, nsFormFillController* self) - MOZ_CAN_RUN_SCRIPT_BOUNDARY_LAMBDA { - self->mFocusPendingPromise = nullptr; - if (showPopup) { - self->ShowPopup(); - } - }, - [](JSContext* aCx, JS::Handle<JS::Value> aValue, ErrorResult& aRv, - nsFormFillController* self) { self->mFocusPendingPromise = nullptr; }, - this); -} - nsresult nsFormFillController::Focus(Event* aEvent) { nsCOMPtr<nsIContent> input = do_QueryInterface(aEvent->GetComposedTarget()); return HandleFocus(MOZ_KnownLive(Element::FromNodeOrNull(input))); @@ -1111,11 +1071,6 @@ nsresult nsFormFillController::MouseDown(Event* aEvent) { NS_IMETHODIMP nsFormFillController::ShowPopup() { - if (mFocusPendingPromise) { - WaitForPromise(true); - return NS_OK; - } - bool isOpen = false; GetPopupOpen(&isOpen); if (isOpen) { @@ -1154,15 +1109,6 @@ NS_IMETHODIMP nsFormFillController::GetPasswordPopupAutomaticallyOpened( return NS_OK; } -NS_IMETHODIMP -nsFormFillController::AddFocusListener(nsIFormFillFocusListener* aListener) { - if (!mFocusListeners.Contains(aListener)) { - mFocusListeners.AppendElement(aListener); - } - - return NS_OK; -} - void nsFormFillController::StartControllingInput(Element* aElement) { MOZ_LOG(sLogger, LogLevel::Verbose, ("StartControllingInput for %p", aElement)); diff --git a/toolkit/components/satchel/nsFormFillController.h b/toolkit/components/satchel/nsFormFillController.h @@ -7,7 +7,6 @@ #define __nsFormFillController__ #include "mozilla/TimeStamp.h" -#include "mozilla/dom/Promise.h" #include "nsIFormFillController.h" #include "nsIAutoCompleteInput.h" #include "nsIAutoCompleteSearch.h" @@ -15,7 +14,6 @@ #include "nsIAutoCompletePopup.h" #include "nsIDOMEventListener.h" #include "nsCOMPtr.h" -#include "nsCOMArray.h" #include "nsStubMutationObserver.h" #include "nsTHashMap.h" #include "nsInterfaceHashtable.h" @@ -109,9 +107,6 @@ class nsFormFillController final : public nsIFormFillController, bool IsTextControl(nsINode* aNode); - MOZ_CAN_RUN_SCRIPT - void WaitForPromise(bool showPopup); - // members ////////////////////////////////////////// nsCOMPtr<nsIAutoCompleteController> mController; @@ -132,10 +127,6 @@ class nsFormFillController final : public nsIFormFillController, nsTHashMap<nsPtrHashKey<const nsINode>, bool> mAutoCompleteInputs; - nsCOMArray<nsIFormFillFocusListener> mFocusListeners; - - RefPtr<mozilla::dom::Promise> mFocusPendingPromise; - uint16_t mFocusAfterRightClickThreshold; uint32_t mTimeout; uint32_t mMinResultsForPopup; diff --git a/toolkit/components/satchel/nsIFormFillController.idl b/toolkit/components/satchel/nsIFormFillController.idl @@ -11,17 +11,6 @@ webidl Document; webidl Element; webidl Event; -[scriptable, uuid(644ac1f4-2122-498e-87bf-2b01d2e24c1e)] -interface nsIFormFillFocusListener : nsISupports { - /* - * Called when a form field is focused and may be used to delay showing - * autocomplete UI until the operation is complete. - * - * @param element the element that is focused. - */ - Promise handleFocus(in Element element); -}; - /* * nsIFormFillController is an interface for controlling form fill behavior * on HTML documents. Any number of docShells can be controller concurrently. @@ -57,13 +46,6 @@ interface nsIFormFillController : nsISupports * Open the autocomplete popup, if possible. */ [can_run_script] void showPopup(); - - /* - * Add a listener which will be notified when a field is focused. - * - * @param listener the listener to be notified. - */ - void addFocusListener(in nsIFormFillFocusListener listener); }; [scriptable, function, uuid(604419ab-55a0-4831-9eca-1b9e67cc4751)]