From cf40f7305723650f3be3ffd48db478b7bafe795d Mon Sep 17 00:00:00 2001 From: Juanjo Tugores Date: Thu, 5 Sep 2024 11:23:55 -0700 Subject: [PATCH] Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In [some cases][1], text editing utilities re-focus the `` element during a blur event. This causes an unusual sequence of `focusin` and `focusout` events, leading to the engine sending unintended events. Consider the following HTML code: ```html
``` Clicking input1, then input2, then input3 produces the following console logs: ``` // Input1 is clicked focusin: focus was gained by ​ // Input2 is clicked focusout: focus is leaving ​ and it will go to ​ focusin: focus was gained by ​ // Input3 is clicked focusout: focus is leaving ​ and it will go to ​ focusin: focus was gained by ​ ``` Now, let's add a blur handler that changes focus: ```html
``` The log sequence changes and gives the wrong impression that no dom element has focus: ``` // Input1 is clicked focusin: focus was gained by ​ // Input2 is clicked focusout: focus is leaving ​ and it will go to ​ focusin: focus was gained by ​ // Input3 is clicked, but the handler kicks in and instead of the following line being a focusout, it results in a focusin call first. focusin: focus was gained by ​ focusout: focus is leaving ​ and it will go to null ``` In addition to that, during `focusout` processing, `activeElement` typically points to ``. However, if an element is focused during a `blur` event, `activeElement` points to that focused element. Although, undocumented it can be verified with: ```html
``` We leverage these behaviors to ignore `focusout` events when the document has focus but `activeElement` is not ``. https://github.com/flutter/flutter/issues/153022 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../lib/web_ui/lib/src/engine/dom.dart | 4 ++++ .../view_focus_binding.dart | 10 ++++++++ .../view_focus_binding_test.dart | 24 +++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart index d3f42272788..5fb335f11b7 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart @@ -339,6 +339,10 @@ extension DomHTMLDocumentExtension on DomHTMLDocument { @JS('visibilityState') external JSString get _visibilityState; String get visibilityState => _visibilityState.toDart; + + @JS('hasFocus') + external JSBoolean _hasFocus(); + bool hasFocus() => _hasFocus().toDart; } @JS('document') diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index 54b71a9d076..97aa20e4082 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -64,6 +64,16 @@ final class ViewFocusBinding { }); late final DomEventListener _handleFocusout = createDomEventListener((DomEvent event) { + // During focusout processing, activeElement typically points to . + // However, if an element is focused during a blur event, activeElement points to that focused element. + // We leverage this behavior to ignore focusout events where the document has focus but activeElement is not . + // + // Refer to https://github.com/flutter/engine/pull/54965 for more info. + final bool wasFocusInvoked = domDocument.hasFocus() && domDocument.activeElement != domDocument.body; + if (wasFocusInvoked) { + return; + } + event as DomFocusEvent; _handleFocusChange(event.relatedTarget as DomElement?); }); diff --git a/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart b/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart index 883176cef92..fe0f9c5479e 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart @@ -270,6 +270,30 @@ void testMain() { expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); }); + + test('works even if focus is changed in the middle of a blur call', () { + final DomElement input1 = createDomElement('input'); + final DomElement input2 = createDomElement('input'); + final EngineFlutterView view = createAndRegisterView(dispatcher); + final DomEventListener focusInput1Listener = createDomEventListener((DomEvent event) { + input1.focusWithoutScroll(); + }); + + view.dom.rootElement.append(input1); + view.dom.rootElement.append(input2); + + input1.addEventListener('blur', focusInput1Listener); + input1.focusWithoutScroll(); + // The event handler above should move the focus back to input1. + input2.focusWithoutScroll(); + input1.removeEventListener('blur', focusInput1Listener); + + expect(dispatchedViewFocusEvents, hasLength(1)); + + expect(dispatchedViewFocusEvents[0].viewId, view.viewId); + expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); + }); }); }