From f0e2596c5c30ab3ba6cc7744d83d4f7bcaba47bb Mon Sep 17 00:00:00 2001 From: Hassan <110993981+htoor3@users.noreply.github.com> Date: Fri, 26 May 2023 11:17:18 -0500 Subject: [PATCH] [web] - Fix autofill group input ordering (flutter/engine#42268) Ordering of input elements inside of the DOM tree for autofill groups does not reflect the order of the form rendered on screen. This is causing some issues with password managers and autofill, specifically Bitwarden. We are currently always appending the currently focused input element to the end of the form. This leads to a tree that appears out of order: Screenshot 2023-05-23 at 2 57 37 PM This fix is tracking the position of where the focused input node should be inserted and inserting it there, rather than always at the end of the form. Once the tree is ordered correctly, Bitwarden's autofill logic works in Flutter forms. Tree order after fix: Screenshot 2023-05-23 at 6 01 05 PM Fixes https://github.com/flutter/flutter/issues/61301 --- .../src/engine/text_editing/text_editing.dart | 22 +++++++++- .../web_ui/test/engine/text_editing_test.dart | 41 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index d0ad3a28183..49746c9245e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -145,6 +145,7 @@ class EngineAutofillForm { this.elements, this.items, this.formIdentifier = '', + this.insertionReferenceNode, }); final DomHTMLFormElement formElement; @@ -153,6 +154,7 @@ class EngineAutofillForm { final Map? items; + final DomHTMLElement? insertionReferenceNode; /// Identifier for the form. /// /// It is constructed by concatenating unique ids of input elements on the @@ -189,6 +191,7 @@ class EngineAutofillForm { final Map elements = {}; final Map items = {}; final DomHTMLFormElement formElement = createDomHTMLFormElement(); + DomHTMLElement? insertionReferenceNode; // Validation is in the framework side. formElement.noValidate = true; @@ -209,6 +212,7 @@ class EngineAutofillForm { AutofillInfo.fromFrameworkMessage(focusedElementAutofill); if (fields != null) { + bool fieldIsFocusedElement = false; for (final Map field in fields.cast>()) { final Map autofillInfo = field.readJson('autofill'); @@ -234,6 +238,17 @@ class EngineAutofillForm { items[autofill.uniqueIdentifier] = autofill; elements[autofill.uniqueIdentifier] = htmlElement; formElement.append(htmlElement); + + // We want to track the node in the position directly after our focused + // element, so we can later insert that element in the correct position + // right before this node. + if(fieldIsFocusedElement){ + insertionReferenceNode = htmlElement; + fieldIsFocusedElement = false; + } + } else { + // current field is the focused element that we create elsewhere + fieldIsFocusedElement = true; } } } else { @@ -268,16 +283,21 @@ class EngineAutofillForm { formElement.append(submitButton); + // If the focused node is at the end of the form, we'll default to inserting + // it before the submit field. + insertionReferenceNode ??= submitButton; + return EngineAutofillForm( formElement: formElement, elements: elements, items: items, formIdentifier: formIdentifier, + insertionReferenceNode: insertionReferenceNode ); } void placeForm(DomHTMLElement mainTextEditingElement) { - formElement.append(mainTextEditingElement); + formElement.insertBefore(mainTextEditingElement, insertionReferenceNode); defaultTextEditingRoot.append(formElement); } diff --git a/engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart b/engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart index c3c00a07c1c..0b651e6c677 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart @@ -2176,6 +2176,47 @@ Future testMain() async { expect(autofillForm, isNull); }); + test('placeForm() should place element in correct position', () { + final List fields = createFieldValues([ + 'email', + 'username', + 'password', + ], [ + 'field1', + 'field2', + 'field3' + ]); + final EngineAutofillForm autofillForm = + EngineAutofillForm.fromFrameworkMessage( + createAutofillInfo('email', 'field1'), fields)!; + + expect(autofillForm.elements, hasLength(2)); + + List formChildNodes = + autofillForm.formElement.childNodes.toList() as List; + + // Only username, password, submit nodes are created + expect(formChildNodes, hasLength(3)); + expect(formChildNodes[0].name, 'username'); + expect(formChildNodes[1].name, 'current-password'); + expect(formChildNodes[2].type, 'submit'); + // insertion point for email should be before username + expect(autofillForm.insertionReferenceNode, formChildNodes[0]); + + final DomHTMLInputElement testInputElement = createDomHTMLInputElement(); + testInputElement.name = 'email'; + autofillForm.placeForm(testInputElement); + + formChildNodes = autofillForm.formElement.childNodes.toList() + as List; + // email node should be placed before username + expect(formChildNodes, hasLength(4)); + expect(formChildNodes[0].name, 'email'); + expect(formChildNodes[1].name, 'username'); + expect(formChildNodes[2].name, 'current-password'); + expect(formChildNodes[3].type, 'submit'); + }); + tearDown(() { clearForms(); });