-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] - Fix for Safari partially autofilling form #43058
Changes from 2 commits
d19ae02
d05fe78
f62dc33
4826afc
dfcc554
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}) { | ||
|
htoor3 marked this conversation as resolved.
Outdated
|
||
| 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'; | ||
|
mdebbar marked this conversation as resolved.
|
||
| } | ||
|
|
||
| if (browserHasAutofillOverlay()) { | ||
| domElement.classList.add(transparentTextEditingClass); | ||
| } | ||
|
|
@@ -191,6 +201,7 @@ class EngineAutofillForm { | |
| final Map<String, DomHTMLElement> elements = <String, DomHTMLElement>{}; | ||
| final Map<String, AutofillInfo> items = <String, AutofillInfo>{}; | ||
| 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear to me why we are not passing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the autofill to work, we only need the actual As for why we need to disable pointer events here on an "invisible" form, Safari has this weird behavior where the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh nvm. I didn't notice that this is for |
||
|
|
||
| // 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; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.