From d19ae0223a2f07ac6e1628a9ed4d8dee97791ad5 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Wed, 21 Jun 2023 14:48:53 -0500 Subject: [PATCH 1/4] Make sure hidden autofill elements are still visible to Safari --- .../src/engine/text_editing/text_editing.dart | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 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 f599c56b88b0a..4c25edf0e1ad9 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 @@ -102,7 +102,9 @@ void _setStaticStyleAttributes(DomHTMLElement domElement) { /// /// They are assigned once during the creation of the DOM element. void _hideAutofillElements(DomHTMLElement domElement, - {bool isOffScreen = false}) { + {bool isOffScreen = false, + bool shouldHideElement = true, + bool shouldDisablePointerEvents = false}) { final DomCSSStyleDeclaration elementStyle = domElement.style; elementStyle ..whiteSpace = 'pre-wrap' @@ -115,8 +117,6 @@ void _hideAutofillElements(DomHTMLElement domElement, ..outline = 'none' ..border = 'none' ..resize = 'none' - ..width = '0' - ..height = '0' ..textShadow = 'transparent' ..transformOrigin = '0 0 0'; @@ -126,6 +126,16 @@ void _hideAutofillElements(DomHTMLElement domElement, ..left = '${offScreenOffset}px'; } + if (shouldHideElement) { + elementStyle + ..width = '0' + ..height = '0'; + } + + if (shouldDisablePointerEvents) { + elementStyle.pointerEvents = 'none'; + } + if (browserHasAutofillOverlay()) { domElement.classList.add(transparentTextEditingClass); } @@ -191,6 +201,7 @@ class EngineAutofillForm { final Map elements = {}; final Map items = {}; final DomHTMLFormElement formElement = createDomHTMLFormElement(); + final bool isSafariDesktopStrategy = textEditing.strategy is SafariDesktopTextEditingStrategy; DomHTMLElement? insertionReferenceNode; // Validation is in the framework side. @@ -201,7 +212,10 @@ class EngineAutofillForm { e.preventDefault(); })); - _hideAutofillElements(formElement); + // We need to explicitly disable pointer events on the form in Safari Desktop, + // so that we don't have pointer event collisions if users hover over or click + // into the invisible autofill elements within the form. + _hideAutofillElements(formElement, shouldDisablePointerEvents: isSafariDesktopStrategy); // We keep the ids in a list then sort them later, in case the text fields' // locations are re-ordered on the framework side. @@ -233,7 +247,16 @@ class EngineAutofillForm { final DomHTMLElement htmlElement = engineInputType.createDomElement(); autofill.editingState.applyToDomElement(htmlElement); autofill.applyToDomElement(htmlElement); - _hideAutofillElements(htmlElement); + + // Safari Desktop does not respect elements that are invisible (or + // have no size) and that leads to issues with autofill only partially + // working (ref: https://github.com/flutter/flutter/issues/71275). + // Thus, we have to make sure that the elements remain invisible to users, + // but not to Safari for autofill to work. Since these elements are + // sized and placed on the DOM, we also have to disable pointer events. + _hideAutofillElements(htmlElement, + shouldHideElement: !isSafariDesktopStrategy, + shouldDisablePointerEvents: isSafariDesktopStrategy); items[autofill.uniqueIdentifier] = autofill; elements[autofill.uniqueIdentifier] = htmlElement; From d05fe78d146920d0e9633af419ea6d632e455c84 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Wed, 21 Jun 2023 14:49:49 -0500 Subject: [PATCH 2/4] Whitespace --- lib/web_ui/lib/src/engine/text_editing/text_editing.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 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 4c25edf0e1ad9..76b18be167676 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 @@ -214,7 +214,7 @@ class EngineAutofillForm { // We need to explicitly disable pointer events on the form in Safari Desktop, // so that we don't have pointer event collisions if users hover over or click - // into the invisible autofill elements within the form. + // into the invisible autofill elements within the form. _hideAutofillElements(formElement, shouldDisablePointerEvents: isSafariDesktopStrategy); // We keep the ids in a list then sort them later, in case the text fields' @@ -248,9 +248,9 @@ class EngineAutofillForm { autofill.editingState.applyToDomElement(htmlElement); autofill.applyToDomElement(htmlElement); - // Safari Desktop does not respect elements that are invisible (or + // Safari Desktop does not respect elements that are invisible (or // have no size) and that leads to issues with autofill only partially - // working (ref: https://github.com/flutter/flutter/issues/71275). + // working (ref: https://github.com/flutter/flutter/issues/71275). // Thus, we have to make sure that the elements remain invisible to users, // but not to Safari for autofill to work. Since these elements are // sized and placed on the DOM, we also have to disable pointer events. From 4826afca7aa59c3ebf53e9614c9436112fedcb94 Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Thu, 22 Jun 2023 17:18:24 -0500 Subject: [PATCH 3/4] Add tests --- lib/web_ui/test/engine/text_editing_test.dart | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index 07e68c3e746ee..a0da159ffb5c5 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -2290,6 +2290,63 @@ Future testMain() async { expect(formChildNodes[3].type, 'submit'); }); + test( + 'hidden autofill elements should have a width and height of 0 on non-Safari browsers', + () { + final List fields = createFieldValues([ + 'email', + 'username', + 'password', + ], [ + 'field1', + 'field2', + 'field3' + ]); + final EngineAutofillForm autofillForm = + EngineAutofillForm.fromFrameworkMessage( + createAutofillInfo('email', 'field1'), fields)!; + final List formChildNodes = + autofillForm.formElement.childNodes.toList() + as List; + final DomHTMLInputElement username = formChildNodes[0]; + final DomHTMLInputElement password = formChildNodes[1]; + + expect(username.name, 'username'); + expect(password.name, 'current-password'); + expect(username.style.width, '0px'); + expect(username.style.height, '0px'); + expect(password.style.width, '0px'); + expect(password.style.height, '0px'); + }, skip: isSafari); + + test( + 'hidden autofill elements should not have a width and height of 0 on Safari', + () { + final List fields = createFieldValues([ + 'email', + 'username', + 'password', + ], [ + 'field1', + 'field2', + 'field3' + ]); + final EngineAutofillForm autofillForm = + EngineAutofillForm.fromFrameworkMessage( + createAutofillInfo('email', 'field1'), fields)!; + final List formChildNodes = + autofillForm.formElement.childNodes.toList() + as List; + final DomHTMLInputElement username = formChildNodes[0]; + final DomHTMLInputElement password = formChildNodes[1]; + expect(username.name, 'username'); + expect(password.name, 'current-password'); + expect(username.style.width, isNot(equals('0px'))); + expect(username.style.height, isNot(equals('0px'))); + expect(password.style.width, isNot(equals('0px'))); + expect(password.style.height, isNot(equals('0px'))); + }, skip: !isSafari); + tearDown(() { clearForms(); }); From dfcc55417bd737002487112f483a1d4984a3278e Mon Sep 17 00:00:00 2001 From: Hassan Toor Date: Fri, 23 Jun 2023 12:44:24 -0500 Subject: [PATCH 4/4] Refactor --- .../src/engine/text_editing/text_editing.dart | 20 ++++++++++--------- lib/web_ui/test/engine/text_editing_test.dart | 14 +++++++++---- 2 files changed, 21 insertions(+), 13 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 76b18be167676..c32819677200b 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 @@ -101,10 +101,12 @@ void _setStaticStyleAttributes(DomHTMLElement domElement) { /// element. /// /// They are assigned once during the creation of the DOM element. -void _hideAutofillElements(DomHTMLElement domElement, - {bool isOffScreen = false, - bool shouldHideElement = true, - bool shouldDisablePointerEvents = false}) { +void _styleAutofillElements( + DomHTMLElement domElement, { + bool isOffScreen = false, + bool shouldHideElement = true, + bool shouldDisablePointerEvents = false, +}) { final DomCSSStyleDeclaration elementStyle = domElement.style; elementStyle ..whiteSpace = 'pre-wrap' @@ -215,7 +217,7 @@ class EngineAutofillForm { // We need to explicitly disable pointer events on the form in Safari Desktop, // so that we don't have pointer event collisions if users hover over or click // into the invisible autofill elements within the form. - _hideAutofillElements(formElement, shouldDisablePointerEvents: isSafariDesktopStrategy); + _styleAutofillElements(formElement, shouldDisablePointerEvents: isSafariDesktopStrategy); // We keep the ids in a list then sort them later, in case the text fields' // locations are re-ordered on the framework side. @@ -254,7 +256,7 @@ class EngineAutofillForm { // Thus, we have to make sure that the elements remain invisible to users, // but not to Safari for autofill to work. Since these elements are // sized and placed on the DOM, we also have to disable pointer events. - _hideAutofillElements(htmlElement, + _styleAutofillElements(htmlElement, shouldHideElement: !isSafariDesktopStrategy, shouldDisablePointerEvents: isSafariDesktopStrategy); @@ -300,7 +302,7 @@ class EngineAutofillForm { // In order to submit the form when Framework sends a `TextInput.commit` // message, we add a submit button to the form. final DomHTMLInputElement submitButton = createDomHTMLInputElement(); - _hideAutofillElements(submitButton, isOffScreen: true); + _styleAutofillElements(submitButton, isOffScreen: true); submitButton.className = 'submitBtn'; submitButton.type = 'submit'; @@ -326,7 +328,7 @@ class EngineAutofillForm { void storeForm() { formsOnTheDom[formIdentifier] = formElement; - _hideAutofillElements(formElement, isOffScreen: true); + _styleAutofillElements(formElement, isOffScreen: true); } /// Listens to `onInput` event on the form fields. @@ -1348,7 +1350,7 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements inputConfiguration.autofillGroup?.formElement != null) { // Subscriptions are removed, listeners won't be triggered. activeDomElement.blur(); - _hideAutofillElements(activeDomElement, isOffScreen: true); + _styleAutofillElements(activeDomElement, isOffScreen: true); inputConfiguration.autofillGroup?.storeForm(); } else { activeDomElement.remove(); diff --git a/lib/web_ui/test/engine/text_editing_test.dart b/lib/web_ui/test/engine/text_editing_test.dart index a0da159ffb5c5..0451c01feb993 100644 --- a/lib/web_ui/test/engine/text_editing_test.dart +++ b/lib/web_ui/test/engine/text_editing_test.dart @@ -2315,8 +2315,11 @@ Future testMain() async { expect(password.name, 'current-password'); expect(username.style.width, '0px'); expect(username.style.height, '0px'); + expect(username.style.pointerEvents, isNot('none')); expect(password.style.width, '0px'); expect(password.style.height, '0px'); + expect(password.style.pointerEvents, isNot('none')); + expect(autofillForm.formElement.style.pointerEvents, isNot('none')); }, skip: isSafari); test( @@ -2341,10 +2344,13 @@ Future testMain() async { final DomHTMLInputElement password = formChildNodes[1]; expect(username.name, 'username'); expect(password.name, 'current-password'); - expect(username.style.width, isNot(equals('0px'))); - expect(username.style.height, isNot(equals('0px'))); - expect(password.style.width, isNot(equals('0px'))); - expect(password.style.height, isNot(equals('0px'))); + expect(username.style.width, isNot('0px')); + expect(username.style.height, isNot('0px')); + expect(username.style.pointerEvents, 'none'); + expect(password.style.width, isNot('0px')); + expect(password.style.height, isNot('0px')); + expect(password.style.pointerEvents, 'none'); + expect(autofillForm.formElement.style.pointerEvents, 'none'); }, skip: !isSafari); tearDown(() {