Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. #54965

tugorez · 2024-09-05T02:31:57Z

In some cases, text editing utilities re-focus the <input /> 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:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: focus was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
  </script>
</body>
</html>

Clicking input1, then input2, then input3 produces the following console logs:

// Input1 is clicked
focusin: focus was gained by <input type value="1" id="input1">

// Input2 is clicked
focusout: focus is leaving <input type value="1" id="input1"> and it will go to <input type value="2" id="input2">
focusin: focus was gained by <input type value="2" id="input2">

// Input3 is clicked
focusout: focus is leaving <input type value="2" id="input2"> and it will go to <input type value="3" id="input3">
focusin: focus was gained by <input type value="3" id="input3">

Now, let's add a blur handler that changes focus:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: focus was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
    input2.addEventListener('blur', (ev) => {
      input2.focus();
    });
  </script>
</body>
</html>

The log sequence changes and gives the wrong impression that no dom element has focus:

// Input1 is clicked
focusin: focus was gained by <input type value="1" id="input1">

// Input2 is clicked
focusout: focus is leaving <input type value="1" id="input1"> and it will go to <input type value="2" id="input2">
focusin: focus was gained by <input type value="2" id="input2">

// 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 <input type value="2" id="input2">
focusout: focus is leaving <input type value="2" id="input2"> and it will go to null

In addition to that, during focusout processing, activeElement typically points to <body />. However, if an element is focused during a blur event, activeElement points to that focused element. Although, undocumented it can be verified with:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('document.hasFocus()', document.hasFocus());     
      console.log('document.activeElement', document.activeElement);
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
    input2.addEventListener('blur', (ev) => {
      input2.focus();
    });
  </script>
</body>
</html>

We leverage these behaviors to ignore focusout events when the document has focus but activeElement is not <body />.

flutter/flutter#153022

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

mdebbar

In addition to that, during focusout processing, activeElement typically points to <body />. However, if an element is focused during a blur event, activeElement points to that focused element. Although, undocumented it can be verified with:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('document.hasFocus()', document.hasFocus());     
      console.log('document.activeElement', document.activeElement);
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
    input2.addEventListener('blur', (ev) => {
      input2.focus();
    });
  </script>
</body>
</html>

I tried this on multiple browsers to make sure they have the same behavior. Chrome and Safari match the behavior you described. Firefox didn't. In fact, Firefox seems to be ignoring input2.focus() completely.

Anyhow, I think your change is okay. On Firefox, it has no effect because that condition is never true.

lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart

tugorez · 2024-09-05T17:27:10Z

In addition to that, during focusout processing, activeElement typically points to <body />. However, if an element is focused during a blur event, activeElement points to that focused element. Although, undocumented it can be verified with:
<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('document.hasFocus()', document.hasFocus());     
      console.log('document.activeElement', document.activeElement);
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
    input2.addEventListener('blur', (ev) => {
      input2.focus();
    });
  </script>
</body>
</html>
I tried this on multiple browsers to make sure they have the same behavior. Chrome and Safari match the behavior you described. Firefox didn't. In fact, Firefox seems to be ignoring input2.focus() completely.

Anyhow, I think your change is okay. On Firefox, it has no effect because that condition is never true.

That is actually true. Super interesting. This is a different, special, behavior tho. If the same node tries to get refocused firefox "prevents it". So in a sense, firefox doesn't allow a node to keep focus against the user will. If you change the code to something like this, you'll see the originally described behavior.

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body>
  <div id="container">
    <input type="" value="1" id="input1">
    <input type="" value="2" id="input2">
    <input type="" value="3" id="input3">
  </div>

  <script>
    container.addEventListener('focusin', (ev) => {
      console.log('focusin: was gained by', ev.target);
    });
    container.addEventListener('focusout', (ev) => {
      console.log('document.hasFocus()', document.hasFocus());     
      console.log('document.activeElement', document.activeElement);
      console.log('focusout: focus is leaving', ev.target, 'and it will go to', ev.relatedTarget);
    });
    input2.addEventListener('blur', (ev) => {
      input1.focus();
    });
  </script>
</body>
</html>

…ng.dart Co-authored-by: Mouad Debbar <[email protected]>

… change focus in the middle of a blur call. (flutter/engine#54965)

…154691) flutter/engine@c50eb8a...015f3b1 2024-09-05 [email protected] [engine] always force platform channel responses to schedule a task. (flutter/engine#54975) 2024-09-05 [email protected] Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…isions) (#154691)" (#154726) Reverts: #154691 Initiated by: jtmcdole Reason for reverting: breaking flutter-dashboard Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@c50eb8a...015f3b1 2024-09-05 [email protected] [engine] always force platform channel responses to schedule a task. (flutter/engine#54975) 2024-09-05 [email protected] Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

… change focus in the middle of a blur call. (flutter/engine#54965)

flutter/engine@c50eb8a...419fb8c 2024-09-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[engine] always force platform channel responses to schedule a task. (#54975)" (flutter/engine#55000) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from b6bab0fde426 to 6ad117bd2efe (2 revisions) (flutter/engine#54999) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Test Scripts from D9INMR2u4wcyiZ750... to 5dqcFlKzRjJb6V95W... (flutter/engine#54998) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from a09312b70d37 to b6bab0fde426 (3 revisions) (flutter/engine#54997) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from 368f209ccca5 to a09312b70d37 (1 revision) (flutter/engine#54995) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from aec11ae18bb6 to 368f209ccca5 (3 revisions) (flutter/engine#54992) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Linux SDK from xNv47d1TZmK9XgTxu... to PBeI0gGvgFdXV6hCg... (flutter/engine#54990) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from 809f868ded1c to aec11ae18bb6 (22 revisions) (flutter/engine#54988) 2024-09-06 [[email protected]](mailto:[email protected]) Removes the int storage from Color (flutter/engine#54714) 2024-09-06 [[email protected]](mailto:[email protected]) iOS,macOS: Add logging of duplicate codesign binaries (flutter/engine#54987) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Test Scripts from k4lKsecg0pdIp-U7c... to D9INMR2u4wcyiZ750... (flutter/engine#54984) 2024-09-05 [[email protected]](mailto:[email protected]) Manual roll of Dart. (flutter/engine#54983) 2024-09-05 [[email protected]](mailto:[email protected]) iOS,macOS: add unsigned_binaries.txt (flutter/engine#54977) 2024-09-05 [[email protected]](mailto:[email protected]) Manual Skia roll to 809f868ded1c (flutter/engine#54972) 2024-09-05 [[email protected]](mailto:[email protected]) [canvaskit] Fix incorrect calculation of ImageFilter paint bounds (flutter/engine#54980) 2024-09-05 [[email protected]](mailto:[email protected]) [engine] always force platform channel responses to schedule a task. (flutter/engine#54975) 2024-09-05 [[email protected]](mailto:[email protected]) Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from xNv47d1TZmK9 to PBeI0gGvgFdX --------- Co-authored-by: Christopher Fujino <[email protected]> Co-authored-by: Zachary Anderson <[email protected]>

github-actions bot added the platform-web Code specifically for the web engine label Sep 5, 2024

tugorez changed the title ~~focusout works when focus is changed in the middle of a blur call.~~ Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. Sep 5, 2024

tugorez requested review from ditman and mdebbar September 5, 2024 03:00

tugorez marked this pull request as ready for review September 5, 2024 03:00

focusout works when focus changes in a blur call.

7afe406

tugorez mentioned this pull request Sep 5, 2024

Enable view focus #54966

Merged

8 tasks

tugorez force-pushed the text-fields branch from e5c61e9 to 7afe406 Compare September 5, 2024 03:24

mdebbar approved these changes Sep 5, 2024

View reviewed changes

lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart Outdated Show resolved Hide resolved

Update lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_bindi…

192bdb4

…ng.dart Co-authored-by: Mouad Debbar <[email protected]>

tugorez added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 5, 2024

auto-submit bot merged commit 0462d19 into flutter:main Sep 5, 2024
30 checks passed

engine-flutter-autoroll mentioned this pull request Sep 5, 2024

Roll Flutter Engine from c50eb8a65097 to 0462d1937908 (1 revision) flutter/flutter#154683

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 5, 2024

0462d1937 Fix unexpected ViewFocus events when Text Editing utilities…

253bd1f

… change focus in the middle of a blur call. (flutter/engine#54965)

engine-flutter-autoroll mentioned this pull request Sep 5, 2024

Roll Flutter Engine from c50eb8a65097 to 015f3b1dec53 (2 revisions) flutter/flutter#154691

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 5, 2024

0462d1937 Fix unexpected ViewFocus events when Text Editing utilities…

cc1c737

… change focus in the middle of a blur call. (flutter/engine#54965)

auto-submit bot mentioned this pull request Sep 6, 2024

Reverts "Roll Flutter Engine from c50eb8a65097 to 015f3b1dec53 (2 revisions) (#154691)" flutter/flutter#154726

Merged

engine-flutter-autoroll mentioned this pull request Sep 6, 2024

Roll Flutter Engine from c50eb8a65097 to 419fb8c0ab3e (17 revisions) flutter/flutter#154731

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2024

0462d1937 Fix unexpected ViewFocus events when Text Editing utilities…

ffdd721

… change focus in the middle of a blur call. (flutter/engine#54965)

a-siva mentioned this pull request Sep 6, 2024

Roll Flutter Engine from c50eb8a65097 to 419fb8c0ab3e flutter/flutter#154734

Merged

engine-flutter-autoroll mentioned this pull request Sep 6, 2024

Roll Flutter Engine from c50eb8a65097 to c40bc44b52aa (18 revisions) flutter/flutter#154735

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2024

0462d1937 Fix unexpected ViewFocus events when Text Editing utilities…

1c50b9d

… change focus in the middle of a blur call. (flutter/engine#54965)

engine-flutter-autoroll mentioned this pull request Sep 6, 2024

Roll Flutter Engine from c50eb8a65097 to 7f2718dec797 (19 revisions) flutter/flutter#154736

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2024

0462d1937 Fix unexpected ViewFocus events when Text Editing utilities…

74e1663

… change focus in the middle of a blur call. (flutter/engine#54965)

tugorez deleted the text-fields branch September 6, 2024 21:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. #54965

Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. #54965

Uh oh!

tugorez commented Sep 5, 2024 •

edited

Loading

Uh oh!

mdebbar left a comment

Uh oh!

Uh oh!

tugorez commented Sep 5, 2024

Uh oh!

Uh oh!

Uh oh!

Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. #54965

Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. #54965

Uh oh!

Conversation

tugorez commented Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tugorez commented Sep 5, 2024

Uh oh!

Uh oh!

Uh oh!

tugorez commented Sep 5, 2024 •

edited

Loading