Repository navigation
Conversation
On the web a browser or extension password manager blurs the hidden autofill input while it fills the login form and then hands focus back. The connection was torn down on that blur and rebuilt on the return focus, which dropped the filled values and churned the autofill DOM, so strict managers disabled themselves. Keep one stable connection across the transient blur: EditableText skips the close for an autofill field on web, the engine defers or keeps its half alive in handleBlur, and the filled values are forwarded for every field (including the focused one) through input, change and animationstart listeners plus a rescan when focus returns. Non-focused proxies are floored to a detectable size, reused across focus for a stable identity, and clustered next to the focused field so proximity-based managers fill the whole form. A value that arrives while the connection is momentarily null is routed through the last connection. Fixes flutter#174773
github-actions
Bot
added
a: text input
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances web autofill support by preventing zero-sized inputs, reusing dormant autofill elements, and keeping the text input connection open during transient blurs. Review feedback points out a logic issue where noteFrameworkEditingState uses an outdated focusedElementId when called during placeForm, suggesting that the field ID be passed explicitly. The feedback also warns of a potential connection leak in EditableTextState due to an early return on blur, and recommends changing the type of tabIndex from double to int.
Addresses review feedback: record the framework value under the field's own uniqueIdentifier rather than relying on focusedElementId at the call site, which is clearer at the placeForm call point.
|
Thanks for the review. I pushed the quick fix (explicit field id for |
There was a problem hiding this comment.
Code Review
This pull request improves web autofill behavior by keeping non-focused autofill fields discoverable but non-interactive, preventing text input connections from closing on transient blurs, and reusing dormant autofill elements to avoid DOM churn. Feedback on these changes suggests clearing min-width and min-height when hiding elements to ensure they shrink properly, correcting a type mismatch where tabIndex is declared as a double instead of an int, and using /// for private member documentation to align with the style guide.
A hidden autofill proxy is shrunk to 1px, but the min-width/min-height floor set for discoverable fields (and on the focused element) overrode that, so the element never actually shrank. Clear the floor in the hide branch before applying the 1px width/height. Also promote two private-member comments to /// per the repo style guide and drop a stray blank line so the formatter is happy.
Routing autofill through updateEditingStateWithTag moved it ahead of the _currentConnection null check, which also moved the shared `methodCall.arguments as List<dynamic>` cast ahead of requestExistingInputState. That request carries no arguments, so the cast threw on it. Compute the args inside the updateEditingStateWithTag block, and again just before the client id lookup, leaving the null check and requestExistingInputState exactly as upstream.
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
Two fixes from review of the web autofill form: - setEditingState now records the framework's value through noteFrameworkEditingState, so a programmatic controller.text change to a focused field is not mistaken for a browser autofill on the next rescan and echoed back, which collapsed the cursor to the end. - Non-focused autofill proxies get tabindex=-1 and aria-hidden=true, both at creation and across focus switches, so keyboard Tab and screen readers no longer land on the invisible fields. They stay DOM-discoverable so password managers still fill them.
The updateEditingStateWithTag fallback to the last connection is a web-only workaround for a browser password manager autofilling in the window after the connection is torn down. Gate it behind kIsWeb so native platforms keep using the current connection. The existing test is conditionalized so it asserts the web routing on web and the unchanged native behavior otherwise.
|
Tried closing on genuine unfocus and it regresses Proton Pass fully on web. Proton's filled value reads empty from the DOM until the user interacts with the field, so the connection has to stay open past the blur to pick it up (confirmed on an instrumented build: it fires the Only way I see to get both is a heuristic: hold briefly after an autofill |
Thanks for digging into the Proton behavior. On holding: after a normal unfocus the soft keyboard stays up, and on desktop keystrokes keep going to the old field. I reproduced both, so we can't land this as-is. On the timer: animationstart fires while the value is still empty, and Proton only reveals it when the user next touches the field, which may be seconds later or never. So a short hold expires before the value lands and Proton stays broken, while a long hold keeps the keyboard up. A fixed timer can't wait for an unbounded user action without bringing the retention back. Question: is the keep-open truly only needed for Proton? You mentioned Google and Bitwarden were unaffected when you removed the early return, so if that holds, we could split this: one PR for the general fix that closes on unfocus and works for the immediate managers, and a follow-up PR for the Proton-specific handling. On verifying: web autofill behavior is hard to see from the diff, so a deployed before/after demo makes it much easier for reviewers to confirm the fix. Firebase Hosting is free, so it costs nothing to stand one up. I built before/after demos while testing this and can share the URLs, and adding before/after links to the PR description would speed up review for everyone. |
…steners A password manager can write into a field after the framework has already closed the connection: the user confirms the fill after the widget lost focus. The form stays in the DOM for that, so its listeners forward what arrives. Cancel the previous instance's listeners when a dormant form is adopted, otherwise they accumulate and later typing is re-forwarded as an autofill, collapsing the cursor to the end of the field.
Removes the keep-open guard from EditableText, so unfocusing an autofill field closes the connection again, same as upstream and as every other field. This fixes the two regressions found in review: on mobile web the soft keyboard stayed up after tapping a non-text widget or unfocus(), and on desktop web the hidden input kept DOM focus so keystrokes kept updating the old controller. The engine close path (goDormant plus safeBlur) hides the input and hands DOM focus to the view root. The engine still keeps its half open across the transient blur a password manager causes while it fills the form; that part never depended on the framework guard. Fills that land after the close are forwarded through the dormant form's listeners (previous commit). Known limitation, split out on purpose: Proton Pass reveals its filled value only when the user next interacts with the field, so it needs the connection held open past the blur. That will be a follow-up; Google Password Manager and Bitwarden work with the close-on-unfocus behavior.
flutter-zl
added
the
waiting for response
|
Hi @sero583 , do you plan to continue working on this PR? If not, I plan to invest some time on this issue. |
|
Sorry for the delay, I'll be able to look into this this weekend. I'll get your feedback worked in, so we can have a final commit to get this done :) |
github-actions
Bot
removed
the
waiting for response
Resolves the conflicts with flutter#187108, flutter#190942 and flutter#193269, and reformats the change for the Dart 3.13 formatter from flutter#192649.
When a form goes dormant, the fields placed on screen next to the focused field are parked off screen, at a size above the floor password managers check, and every field leaves the tab order, so nothing of a dismissed form stays hit-testable over the app or reachable with Tab. A form that goes dormant while focus is away for a fill, because a manager's menu or popup took it, keeps its fields in place until the fill is over: the next tap on the page, or the window getting the focus back. A manager only fills a field that hit-tests back to itself. The view unfocus deferral applies only to a blur to nothing from a field of a multi-field autofill form, which the engine marks with a class. The iOS caret drag deferral in ViewFocusBinding is unchanged. The form's submit button is aria-hidden. The fields are hidden from assistive technology only in semantics mode. In semantics mode the non-focused fields and the synthetic placeholder stay hidden off WebKit; outside it every field keeps a real size. Pointer events on the fields follow _isWebKit. A pointer event on a non-focused field is positioned like an event on any other element, not through the focused field's geometry. The injected autofill style carries the CSP nonce, is scoped to flutter-view and uses one rule per pseudo-class. A dormant element is reused only while it is still in the document. Also fixes the analyzer infos in the change.
|
@flutter-zl sorry for the delay, time has been tight lately. Thanks for testing on the demos. Everything is in, and master is merged.
Also fixed: a tap on a non-focused proxy hit the assert in Demos are redeployed from this head: https://flutter-autofill-190126.web.app. Bitwarden (inline menu and toolbar popup), Proton Pass and Chrome's manager fill both fields from either field, in Chrome and Firefox. |
flutter-zl
left a comment
flutter-zl
left a comment
There was a problem hiding this comment.
One remaining blocking issue: beginFillWindow starts when the active field blurs, but the window only ends when that same field regains focus. If a manager focuses a sibling proxy instead, placement updates remain disabled and the proxies freeze at stale coordinates.
Click username.
Click Run fill-window probe.
Resize the browser horizontally.
The viewport moved 280 px while the proxy moved 0 px.
Please add a regression test and end the fill window when any form field receives focus. Other than this, the PR looks ready to merge.
A password manager's menu taking the focus from the active field opens the fill window, which holds the fields still while the manager fills. The window ended only when that same field, the window or the page got the focus back. A manager that focuses another field of the form instead left the window open, so placement updates stayed disabled and the fields stayed at stale coordinates, for example after a resize. Every field of the form now ends the fill window when it gets the focus. A placement that arrives while the window is open is now held by the form and applied when the window ends, instead of being dropped until the next layout change. It moves the fields but not the focus, since the manager may have just focused another field. A form that goes dormant drops the held placement, as its connection is closing.
|
@flutter-zl thanks, the probe made this easy to pin down. Fixed in 5c1670f. Every field of the form now ends the fill window when it gets the focus, not only the field the manager was invoked from. While testing it I found a second way to end up with stale coordinates. A placement that arrived while the window was open was only stored, and nothing applied it when the window ended, so the fields stayed put until the next layout change. The form now holds that placement and applies it when the window ends, whichever way it ends. It moves the fields but not the focus, since the manager may have just focused another field. A form that goes dormant drops it, as its connection is closing. Tests: One thing that looks like the same bug but is not: if the page itself loses the focus, for example when DevTools opens to resize the window, the connection closes and the form stays put until the fill is over, the exception from my last reply. The probe then reports a frozen proxy until the page has the focus again, and after that the fields are placed correctly. |
flutter-zl
left a comment
flutter-zl
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the fix.
flutter-zl
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/190126, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
|
We hit this in a production Flutter web app (CanvasKit, Flutter 3.50) and tested the demos linked above:
Today our users have to click into the password field first. Thanks for the work on this. We'd be glad to see it land, and we're happy to re-test once the conflict is resolved. |
|
Follow-up: after a Touch ID fill, keystrokes go to the password field (Safari, also on To be precise about my comment above: the fill itself is fixed in Safari, but where keystrokes go after the fill is not. Environment
Steps to reproducePrecondition: Apple Passwords has a saved credential for the demo (for example saved once via "Fill test credential + save").
Expected resultsThe typed text goes to the username field, and the password stays unchanged. Actual resultsThe typed text ends up in the password field instead; the username stays Code sampleThe demo app from this PR, unchanged: https://flutter-autofill-190126.web.app/repro/main.dart LogsFocus log of the demo, in order: Notes
|
What this changes
On the web a password manager could not fill a login form as a pair. Which field arrived depended on which field the manager was invoked from, and in several combinations nothing arrived at all. That is true of shipped builds too, not just of this branch's starting point.
Flutter paints text on canvas and synthesises hidden
<input>proxies so managers and the browser have something to work with. Those proxies did not satisfy what managers actually check before filling a field. Three causes, each found by reading what the managers check and measuring the live DOM against it:EngineAutofillFormstyled them withshouldHideElement: !_isWebKit, so outside WebKit every field of a form except the focused one waswidth: 0; height: 0. A manager will not treat an input that small as a real field (Bitwarden wants 10x10, Proton Pass 30x15), and it confirms the field by hit testing its own centre point and checking it gets that field back, which a zero sized box also fails. So only the focused field was ever a candidate and a login form could not be filled as a pair. The proxies now keep a real size, floored withmin-width/min-heightbecause the framework geometry can itself report a pixel, and the default action of their pointer input is suppressed, so a tap on an invisible field does not move the browser's focus to it. The tap still reaches the app.disable()parked that field's proxy off screen and zero sized, and a manager's menu taking focus is what closes the connection, so the field being parked was the field the user was in. Invoked from the email field it hid the email field and filled the password; invoked from the password field it hid the password and filled only the email. The shrink also outlives the moment: at least one manager memoises its fillability verdict per element, and these elements are reused for the life of the form, so a single measurement taken while the field was zero sized wrote it off for good. Position is not part of that verdict, so the proxy is moved but keeps its size.<body>with norelatedTarget, which was reported as the whole view losing focus, so the framework dropped its focus, closed the connection, and rebuilt it when focus returned a moment later. Managers that check their own state between fields lost the rest of the form in that gap. A blur to nothing from a field of a multi-field autofill form (the engine marks that form with a class) now waits before that is reported, and a focus arriving back in the view cancels it. Everything else, including a blur from any other form or to a known element outside the view, is reported at once, unchanged. The wait is on a pair of browser events, afocusoutand thefocusinafter it, about a millisecond apart. It is not a timer waiting on a manager. It shares its timer andkTransientBlurSettleDelaywith the iOS caret drag deferral from Reland "[web] Keep the keyboard up during an iOS caret drag" #190942.On the first of those, #187108 already established that WebKit refuses to autofill a zero sized input and kept a real size there for that reason. What this adds is that the same holds on every engine once a third party manager rather than the browser is doing the filling: Bitwarden and Proton Pass each apply their own minimum size before they will treat an input as a field. So
_isWebKitbeing false on Chrome and Firefox is not the same thing as those engines tolerating a zero sized field. Pointer events follow_isWebKitat every proxy site, as #187108 does. Semantics mode keeps the #187108 styling: there the non-focused fields stay hidden off WebKit.To present a login form a manager can pair, the non focused proxies are stacked under the focused field with
position: fixed. When the form goes dormant the fields the clustering pinned are parked off screen, above the managers' size floor, and every field leaves the tab order, so nothing of a dismissed form stays hit testable or reachable with Tab. A form that goes dormant while focus is away for a fill (a manager's menu or popup took it) keeps its fields in place until the fill is over, that is the next real tap on the page or the window getting the focus back, because a manager only fills a field that hit tests back to itself.A pointer event on a non focused proxy, which is hit testable now, is positioned like any other element rather than through the focused field's geometry.
The result is that Bitwarden, Proton Pass and Chrome's own manager each fill both fields, from either field, including after clearing a field by hand and filling again.
Two things follow from the same invariant, that what a manager reads from the DOM has to be what the framework holds. Clearing a field by hand now reaches the proxy, so a manager stops offering to save a password that has already been deleted; previously the proxy kept the last character and the prompt never went away. And a reused proxy no longer serves the previous connection's value to the next manager that reads it.
Accessibility
Giving the proxies a real size makes them reachable in ways a zero sized element was not, so this is handled explicitly.
Non focused proxies are now set to
tabindex="-1"(upstream left freshly built proxies in the tab order), and parked ones too, so Tab does not land on an invisible password box; only the focused proxy is in the tab order. With semantics on they are markedaria-hidden="true", which upstream did not do: the field then has a real semantics node and the proxy is a duplicate.With semantics off the attribute is deliberately not set on the fields. Proton Pass rejects an
aria-hiddeninput before it even measures it, so setting it unconditionally puts back the same class of bug this PR fixes, and with semantics off the proxy is the only representation of that field in the DOM, so hiding it hides nothing the user could reach another way.The form's submit button is
aria-hiddenunconditionally. It is plumbing that onlysaveFormsever clicks, and with semantics off it was what made the invisible proxies read as a complete login form in VoiceOver's form controls rotor.Google Password Manager's auto submit on mobile is deliberately not addressed here. Separate behaviour, separate discussion, follow up.
Tested
Demos, the same app built twice, plus a plain HTML login form as a reference for how a manager behaves when Flutter is not involved. The app prints each controller's value on screen, so it is visible whether what the manager wrote reached the Flutter side or only the hidden input.
Verified against the real extensions in Chrome and Firefox: Bitwarden, Proton Pass and Chrome's own password manager, invoked from each field, and after emptying a field by hand.
text_editing_test.dart,view_focus_binding_test.dart,event_position_helper_test.dartand the semantics text field tests pass on Chrome (dart2js and dart2wasm) and Firefox.engine/src/flutter/lib/web_ui/test/engine/text_editing_test.dart: 16 added, 1 removed. The removed one ishidden autofill elements should have a width and height of 0 on non-Safari browsers, which asserted the behaviour the first cause above changes. It is replaced bynon-focused autofill fields are discoverable and hit-testable on non-Safari browsers, asserting what the proxies do now: a non zero size, out of the tab order, taking part in hit testing. The Safari cases keep their existing expectations. Five existing tests are adjusted:focus and disconnection with delaying blur in iOSnow runs with autofill off and expects the deferred close,Moves the focus across input elementsuses a distinct config for its second field, andmultiTextField Autofill: setClient, setEditingState, show, setSizeAndTransform setEditingState, clearClient,form wakeUp and goDormantandValidate single element formgain assertions for the clustering, the parking, the marker class andaria-hidden.engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart: 5 added, existing tests unchanged. The existing ones passing unmodified is the check that ordinary view unfocus is untouched. The added ones cover a manager stepping between two fields, which is not reported, a focus that never comes back, which still is, and three cases reported at once: an input outside any form, a form that is not the engine's, and a blur to an element outside the view.engine/src/flutter/lib/web_ui/test/engine/pointer_binding/event_position_helper_test.dart: 1 added, for the pointer position on a non focused proxy.packages/flutter/test/services/autofill_test.dart: 1 added, for the last connection fallback.Fixes #174773
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.