Flutter views can gain focus #54985

tugorez · 2024-09-05T23:27:05Z

I am unsure why the tabindex was removed when semantics were enabled. It seems this change was made without a clear explanation (by me). This PR shouldn't cause any issues as Flutter Views already have a tabindex, we're not adding a new one. This change is necessary because the semantics text strategy refocuses the view on deactivation, requiring the Flutter view to be focusable.

ThIs PR is a requirement to enable #54966.

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.

ditman

This PR seems correct. What it does is to make the current flutter view focusable (so it can retain the focus if needed), but inaccessible by keyboard. See tabindex docs.

ditman · 2024-11-05T23:08:00Z

The only change I want to make is to rename the method a little bit, because all it seems to be doing is to modify the keyboard reachability of the view, but the name is more generic than what it actually does.

ditman · 2024-11-06T18:48:52Z

I'm taking a quick stab at attaching the focus handlers to the flutter view root, rather than the body.

ditman · 2024-11-12T00:56:52Z

Looks like I broke something in Safari with my latest changes, reviewing:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8731518283441607553/+/u/test:_run_suite_chrome-dart2js-html-engine/stdout

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

yjbanov · 2024-11-12T20:34:36Z

lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart

@@ -68,28 +68,6 @@ void testMain() {
      expect(view2.dom.rootElement.getAttribute('tabindex'), '0');
    });

-    test('never marks the views as focusable with semantincs enabled', () async {


Should we also add some tests? :)

Well, there's tests already (this file is just showing the removed test, but there's others to verify the rest of the functionality!)

yjbanov

ditman · 2024-11-14T00:15:14Z

One last update and applying autosubmit.

auto-submit · 2024-11-14T00:51:11Z

auto label is removed for flutter/engine/54985, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

ditman · 2024-11-18T19:25:49Z

Come on, autosubmit, throw me a bone here!

…159111) flutter/engine@878f593...7643ef1 2024-11-18 [email protected] iOS: Eliminate unguarded-availability opt-out (flutter/engine#56689) 2024-11-18 [email protected] [skwasm] Use `displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight` (flutter/engine#56686) 2024-11-18 [email protected] Extract TestGLContext to separate translation unit (flutter/engine#56647) 2024-11-18 [email protected] Roll Skia from b79e71223284 to 492e8347d7a4 (2 revisions) (flutter/engine#56687) 2024-11-18 [email protected] Update the Skia build scripts for a refactoring of the Fontconfig font manager (flutter/engine#56684) 2024-11-18 [email protected] iOS,macOS: Enable ARC in flutter_cflags_objc[c] (flutter/engine#56685) 2024-11-18 [email protected] Re-land "Remove `android_jit_release_x86`." (flutter/engine#56681) 2024-11-18 [email protected] Roll Skia from 0d24bd3268ef to b79e71223284 (1 revision) (flutter/engine#56683) 2024-11-18 [email protected] Flutter views can gain focus (flutter/engine#54985) 2024-11-18 [email protected] Roll Skia from 0b74d5c3eb4f to 0d24bd3268ef (1 revision) (flutter/engine#56680) 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] 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

…159113) flutter/engine@878f593...c9a965c 2024-11-18 [email protected] Roll Dart SDK from 625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688) 2024-11-18 [email protected] iOS: Eliminate unguarded-availability opt-out (flutter/engine#56689) 2024-11-18 [email protected] [skwasm] Use `displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight` (flutter/engine#56686) 2024-11-18 [email protected] Extract TestGLContext to separate translation unit (flutter/engine#56647) 2024-11-18 [email protected] Roll Skia from b79e71223284 to 492e8347d7a4 (2 revisions) (flutter/engine#56687) 2024-11-18 [email protected] Update the Skia build scripts for a refactoring of the Fontconfig font manager (flutter/engine#56684) 2024-11-18 [email protected] iOS,macOS: Enable ARC in flutter_cflags_objc[c] (flutter/engine#56685) 2024-11-18 [email protected] Re-land "Remove `android_jit_release_x86`." (flutter/engine#56681) 2024-11-18 [email protected] Roll Skia from 0d24bd3268ef to b79e71223284 (1 revision) (flutter/engine#56683) 2024-11-18 [email protected] Flutter views can gain focus (flutter/engine#54985) 2024-11-18 [email protected] Roll Skia from 0b74d5c3eb4f to 0d24bd3268ef (1 revision) (flutter/engine#56680) 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] 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

…159116) flutter/engine@878f593...10dc07b 2024-11-18 [email protected] Update emulator definitions version to latest available from chrome infra (flutter/engine#56313) 2024-11-18 [email protected] Roll Dart SDK from 625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688) 2024-11-18 [email protected] iOS: Eliminate unguarded-availability opt-out (flutter/engine#56689) 2024-11-18 [email protected] [skwasm] Use `displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight` (flutter/engine#56686) 2024-11-18 [email protected] Extract TestGLContext to separate translation unit (flutter/engine#56647) 2024-11-18 [email protected] Roll Skia from b79e71223284 to 492e8347d7a4 (2 revisions) (flutter/engine#56687) 2024-11-18 [email protected] Update the Skia build scripts for a refactoring of the Fontconfig font manager (flutter/engine#56684) 2024-11-18 [email protected] iOS,macOS: Enable ARC in flutter_cflags_objc[c] (flutter/engine#56685) 2024-11-18 [email protected] Re-land "Remove `android_jit_release_x86`." (flutter/engine#56681) 2024-11-18 [email protected] Roll Skia from 0d24bd3268ef to b79e71223284 (1 revision) (flutter/engine#56683) 2024-11-18 [email protected] Flutter views can gain focus (flutter/engine#54985) 2024-11-18 [email protected] Roll Skia from 0b74d5c3eb4f to 0d24bd3268ef (1 revision) (flutter/engine#56680) 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] 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

…159118) flutter/engine@878f593...c1b0e18 2024-11-19 [email protected] [Impeller] use sync fence for image uploads. (flutter/engine#56609) 2024-11-18 [email protected] Update emulator definitions version to latest available from chrome infra (flutter/engine#56313) 2024-11-18 [email protected] Roll Dart SDK from 625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688) 2024-11-18 [email protected] iOS: Eliminate unguarded-availability opt-out (flutter/engine#56689) 2024-11-18 [email protected] [skwasm] Use `displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight` (flutter/engine#56686) 2024-11-18 [email protected] Extract TestGLContext to separate translation unit (flutter/engine#56647) 2024-11-18 [email protected] Roll Skia from b79e71223284 to 492e8347d7a4 (2 revisions) (flutter/engine#56687) 2024-11-18 [email protected] Update the Skia build scripts for a refactoring of the Fontconfig font manager (flutter/engine#56684) 2024-11-18 [email protected] iOS,macOS: Enable ARC in flutter_cflags_objc[c] (flutter/engine#56685) 2024-11-18 [email protected] Re-land "Remove `android_jit_release_x86`." (flutter/engine#56681) 2024-11-18 [email protected] Roll Skia from 0d24bd3268ef to b79e71223284 (1 revision) (flutter/engine#56683) 2024-11-18 [email protected] Flutter views can gain focus (flutter/engine#54985) 2024-11-18 [email protected] Roll Skia from 0b74d5c3eb4f to 0d24bd3268ef (1 revision) (flutter/engine#56680) 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] 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

I am unsure why the `tabindex` was removed when semantics were enabled. It seems this change was made without a clear explanation (by me). This PR shouldn't cause any issues as Flutter Views already have a tabindex, we're not adding a new one. This change is necessary because the semantics text strategy refocuses the view on deactivation, requiring the Flutter view to be focusable. ThIs PR is a requirement to enable flutter/engine#54966. flutter#153022 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Flutter view can gain focus.

eea5707

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

tugorez marked this pull request as ready for review September 6, 2024 04:20

tugorez requested review from yjbanov, mdebbar and ditman September 6, 2024 04:20

Merge branch 'main' into view-focus-semantics

bb10b63

ditman approved these changes Nov 5, 2024

View reviewed changes

Minor PR changes.

b1c044d

ditman added 2 commits November 11, 2024 16:24

Attach focusin/out to the rootElement of the view.

8f843a7

Merge branch 'main' into view-focus-semantics

9a28cfb

Use state properly in changeViewFocus.

c096407

yjbanov reviewed Nov 12, 2024

View reviewed changes

Address PR comments.

d735a65

ditman requested a review from yjbanov November 13, 2024 21:59

yjbanov approved these changes Nov 13, 2024

View reviewed changes

Merge branch 'main' into view-focus-semantics

8ec21bb

ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2024

ditman added 2 commits November 14, 2024 18:37

Merge branch 'main' into view-focus-semantics

5ef3d70

Merge branch 'main' into view-focus-semantics

ce79e30

ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 18, 2024

auto-submit bot merged commit dec7eb9 into flutter:main Nov 18, 2024
31 checks passed

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 18, 2024

engine-flutter-autoroll mentioned this pull request Nov 18, 2024

Roll Flutter Engine from 878f593802e1 to 03af645aaee0 (3 revisions) flutter/flutter#159102

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

f83bdd5

engine-flutter-autoroll mentioned this pull request Nov 18, 2024

Roll Flutter Engine from 878f593802e1 to 68ef504afea4 (4 revisions) flutter/flutter#159103

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

db08ea6

engine-flutter-autoroll mentioned this pull request Nov 18, 2024

Roll Flutter Engine from 878f593802e1 to 635dee808701 (7 revisions) flutter/flutter#159109

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

125f057

engine-flutter-autoroll mentioned this pull request Nov 18, 2024

Roll Flutter Engine from 878f593802e1 to 7643ef10c25f (10 revisions) flutter/flutter#159111

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

e720016

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter Engine from 878f593802e1 to c9a965c4e5e6 (11 revisions) flutter/flutter#159113

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

0ca951d

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter Engine from 878f593802e1 to 10dc07bf548f (12 revisions) flutter/flutter#159116

Closed

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

456722c

engine-flutter-autoroll mentioned this pull request Nov 19, 2024

Roll Flutter Engine from 878f593802e1 to c1b0e18a70b3 (13 revisions) flutter/flutter#159118

Merged

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

dec7eb9b2 Flutter views can gain focus (flutter/engine#54985)

1f6f41b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Flutter views can gain focus #54985

Flutter views can gain focus #54985

Uh oh!

tugorez commented Sep 5, 2024 •

edited

Loading

Uh oh!

ditman left a comment

Uh oh!

ditman commented Nov 5, 2024

Uh oh!

ditman commented Nov 6, 2024

Uh oh!

ditman commented Nov 12, 2024

Uh oh!

Uh oh!

Uh oh!

yjbanov Nov 12, 2024

Uh oh!

ditman Nov 13, 2024

Uh oh!

yjbanov left a comment

Uh oh!

ditman commented Nov 14, 2024

Uh oh!

auto-submit bot commented Nov 14, 2024

Uh oh!

ditman commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Flutter views can gain focus #54985

Flutter views can gain focus #54985

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!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

ditman commented Nov 5, 2024

Uh oh!

ditman commented Nov 6, 2024

Uh oh!

ditman commented Nov 12, 2024

Uh oh!

Uh oh!

Uh oh!

yjbanov Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

ditman Nov 13, 2024

Choose a reason for hiding this comment

Uh oh!

yjbanov left a comment

Choose a reason for hiding this comment

Uh oh!

ditman commented Nov 14, 2024

Uh oh!

auto-submit bot commented Nov 14, 2024

Uh oh!

ditman commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

tugorez commented Sep 5, 2024 •

edited

Loading