From 5ff90cfb0b96cbaa9504f91f55ab3a8121f8ccdc Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Tue, 23 May 2023 17:58:09 -0500 Subject: [PATCH 1/4] Keep track of position to insert our focused element instead of always appending --- .../src/engine/text_editing/text_editing.dart | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index f8c9127d27b63..72d1aa725a3ca 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -145,6 +145,7 @@ class EngineAutofillForm { this.elements, this.items, this.formIdentifier = '', + this.nodeToInsertBefore, }); final DomHTMLFormElement formElement; @@ -153,6 +154,7 @@ class EngineAutofillForm { final Map? items; + final DomHTMLElement? nodeToInsertBefore; /// 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? nodeToInsertBefore; // 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){ + nodeToInsertBefore = 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. + nodeToInsertBefore ??= submitButton; + return EngineAutofillForm( formElement: formElement, elements: elements, items: items, formIdentifier: formIdentifier, + nodeToInsertBefore: nodeToInsertBefore ); } void placeForm(DomHTMLElement mainTextEditingElement) { - formElement.append(mainTextEditingElement); + formElement.insertBefore(mainTextEditingElement, nodeToInsertBefore); defaultTextEditingRoot.append(formElement); } From 3c334baa61821346c5e6ba3716d99cb6431bc86a Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Tue, 23 May 2023 17:58:47 -0500 Subject: [PATCH 2/4] Whitespace --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 72d1aa725a3ca..66b654d73dd17 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -284,7 +284,7 @@ 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. + // it before the submit field. nodeToInsertBefore ??= submitButton; return EngineAutofillForm( From 2c0ffce18ca84e7315ed5dddbebf76923c79ff01 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Wed, 24 May 2023 15:37:34 -0500 Subject: [PATCH 3/4] Change naming, add tests --- .../src/engine/text_editing/text_editing.dart | 14 ++++---- lib/web_ui/test/engine/text_editing_test.dart | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 66b654d73dd17..bb52dee47698b 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -145,7 +145,7 @@ class EngineAutofillForm { this.elements, this.items, this.formIdentifier = '', - this.nodeToInsertBefore, + this.insertionReferenceNode, }); final DomHTMLFormElement formElement; @@ -154,7 +154,7 @@ class EngineAutofillForm { final Map? items; - final DomHTMLElement? nodeToInsertBefore; + final DomHTMLElement? insertionReferenceNode; /// Identifier for the form. /// /// It is constructed by concatenating unique ids of input elements on the @@ -191,7 +191,7 @@ class EngineAutofillForm { final Map elements = {}; final Map items = {}; final DomHTMLFormElement formElement = createDomHTMLFormElement(); - DomHTMLElement? nodeToInsertBefore; + DomHTMLElement? insertionReferenceNode; // Validation is in the framework side. formElement.noValidate = true; @@ -243,7 +243,7 @@ class EngineAutofillForm { // element, so we can later insert that element in the correct position // right before this node. if(fieldIsFocusedElement){ - nodeToInsertBefore = htmlElement; + insertionReferenceNode = htmlElement; fieldIsFocusedElement = false; } } else { @@ -285,19 +285,19 @@ class EngineAutofillForm { // If the focused node is at the end of the form, we'll default to inserting // it before the submit field. - nodeToInsertBefore ??= submitButton; + insertionReferenceNode ??= submitButton; return EngineAutofillForm( formElement: formElement, elements: elements, items: items, formIdentifier: formIdentifier, - nodeToInsertBefore: nodeToInsertBefore + insertionReferenceNode: insertionReferenceNode ); } void placeForm(DomHTMLElement mainTextEditingElement) { - formElement.insertBefore(mainTextEditingElement, nodeToInsertBefore); + formElement.insertBefore(mainTextEditingElement, insertionReferenceNode); defaultTextEditingRoot.append(formElement); } diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index c3c00a07c1c43..236465eca7ab1 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -2176,6 +2176,42 @@ 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'); + // 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[0].name, 'email'); + expect(formChildNodes[1].name, 'username'); + }); + tearDown(() { clearForms(); }); From 1eb2a697e28c2222307963053b737a4186dba786 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Thu, 25 May 2023 11:05:09 -0500 Subject: [PATCH 4/4] Modify test to check for all children in form --- lib/web_ui/test/engine/text_editing_test.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index 236465eca7ab1..0b651e6c677a9 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -2198,6 +2198,8 @@ Future testMain() async { // 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]); @@ -2208,8 +2210,11 @@ Future testMain() async { 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(() {