[web] Move text editing nodes outside of shadowDOM - reopened #40904

htoor3 · 2023-04-04T00:07:26Z

Password input autofill + suggest password functionality is broken for password inputs inside of a shadowDOM on chrome. This experiment is testing to see if we can lift text editing nodes outside of the shadowDOM while maintaining functionality.

Create a new host node outside of the shadowDOM for text-editing + accessibility
Attach all text editing nodes, accessibility placeholders, and semantics tree into that node
Ensure text editing + accessibility global styles are applied to new host node

Tree goes from:

                flt-glass-pane
                      |
                 #shadow-root
                      |
                 text nodes

to:

                            flutter-view
           -----------------------------------------------
           /                        |                  \
  flt-glass-pane             text-editing-host   semantics-host
          /  \                      |                  |
#shadow-root platform views     text nodes       semantics nodes

…iment

…nder host

…iment

…ove platform views under glass pane, fix tests and naming

ditman · 2023-04-04T19:48:59Z

lib/web_ui/lib/src/engine/embedder.dart

@@ -62,7 +63,10 @@ class FlutterViewEmbedder {
  /// Abstracts all the DOM manipulations required to embed a Flutter app in an user-supplied `hostElement`.
  final EmbeddingStrategy _embeddingStrategy;

-  // The tag name for the root view of the flutter app (glass-pane)
+  // The tag name for the Flutter View, which hosts the app.
+  static const String flutterViewTagName = 'flutter-view';


@yjbanov can this be flt-view? Custom elements work off the tag name, and I wouldn't want to have collisions if we ever implement custom element stuff.

flutter-dashboard · 2023-04-04T19:55:23Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #40904 at sha 0ab230a

ditman

I've tested this with my latest experiment app, and it seems to be working well.

Have you checked the tests in other repos so this can land @htoor3?

htoor3 · 2023-04-04T21:06:34Z

I've tested this with my latest experiment app, and it seems to be working well.

Have you checked the tests in other repos so this can land @htoor3?

Yes, the /packages integration tests are passing.

There is a framework test that will need a fix: flutter/flutter#124160

And it looks like there are some internal g3 tests that will likely be affected that I'll need to fix.

…ed (flutter/engine#40904)

…#40904)" This reverts commit 74d7933.

#40960) Reverts #40904

jonahwilliams · 2023-04-06T03:16:04Z

I'm still seeing failing tests on an engine roll that contains this change: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8784608634967978705/+/u/run_test.dart_for_web_long_running_tests_shard_and_subshard_5_5/test_stdout

Is this expected?

zanderso · 2023-04-06T03:19:06Z

Revert landed here: #40960

…ed (flutter/engine#40904)

…ions) (#124300) Manual roll requested by zra@google.com flutter/engine@610cdf0...1ec74c6 2023-04-06 110993981+htoor3@users.noreply.github.com Revert "[web] Move text editing nodes outside of shadowDOM - reopened" (flutter/engine#40960) 2023-04-05 110993981+htoor3@users.noreply.github.com [web] Move text editing nodes outside of shadowDOM - reopened (flutter/engine#40904) 2023-04-05 30870216+gaaclarke@users.noreply.github.com [Impeller] adjusted the rotated text test to have an opaque background and account for screen scale (flutter/engine#40954) 2023-04-05 jacksongardner@google.com Copy canvaskit files directly into `flutter_web_sdk` (flutter/engine#40951) 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 jonahwilliams@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

ditman · 2023-04-06T17:28:40Z

We hadn't seen the failures on test_driver/text_editing_integration.dart in an earlier attempt to land this. @htoor3 has a fix for that test so this can be relanded.

htoor3 · 2023-04-06T19:57:59Z

We hadn't seen the failures on test_driver/text_editing_integration.dart in an earlier attempt to land this. @htoor3 has a fix for that test so this can be relanded.

Thanks @ditman.
Fix PR is flutter/flutter#124299
Reland PR is #40968

…ions) (flutter#124300) Manual roll requested by zra@google.com flutter/engine@610cdf0...1ec74c6 2023-04-06 110993981+htoor3@users.noreply.github.com Revert "[web] Move text editing nodes outside of shadowDOM - reopened" (flutter/engine#40960) 2023-04-05 110993981+htoor3@users.noreply.github.com [web] Move text editing nodes outside of shadowDOM - reopened (flutter/engine#40904) 2023-04-05 30870216+gaaclarke@users.noreply.github.com [Impeller] adjusted the rotated text test to have an opaque background and account for screen scale (flutter/engine#40954) 2023-04-05 jacksongardner@google.com Copy canvaskit files directly into `flutter_web_sdk` (flutter/engine#40951) 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 jonahwilliams@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

htoor3 added 25 commits February 16, 2023 13:31

Add a text editing host node, attach all text editing nodes to it

b7f7f22

whitespace

1aa89a2

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

f055c0b

…iment

Move semantics tree outside of text editing host, change naming

d9f1dd1

Refactor

dfc1bab

fix text editing tests

23b5b6c

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

546b92c

…iment

Fix issues on semantic mode, fix tests, move platform views to new re…

9586c58

…nder host

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

f74442e

…iment

Fix text field tests

ae00e40

Remove unused imports

3b6f9eb

Fix host node tests

4f75b6b

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

b600dc6

…iment

Refactor, fix safari text field tests

df91ed5

Refactor, fix indentation, remove comments

d8f96cd

Trailing whitespace

cc4ed23

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

de2e916

…iment

Move conditional to compute talkback events when semantics enabled

97555e5

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

3ed55b8

…iment

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

36a0822

…iment

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

bf7f94e

…iment

Change function naming and comments for offset computations

401bb1c

Whitespace

3815cea

Merge remote-tracking branch 'upstream/main' into pass-autofill-exper…

251fa13

…iment

Add appRoot, remove renderHost, move glassPane to host shadow root, m…

1226d36

…ove platform views under glass pane, fix tests and naming

flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 4, 2023

htoor3 changed the title ~~Pass autofill reopen~~ [web] Move text editing nodes outside of shadowDOM - reopened Apr 4, 2023

htoor3 added 3 commits April 3, 2023 20:36

Merge remote-tracking branch 'upstream/main' into pass-autofill-reopen

64c3d1c

Merge remote-tracking branch 'upstream/main' into pass-autofill-reopen

ec0ae34

Change naming

d76662e

htoor3 marked this pull request as ready for review April 4, 2023 16:56

htoor3 added 3 commits April 4, 2023 12:07

Merge remote-tracking branch 'upstream/main' into pass-autofill-reopen

061fba8

Change naming of host element to flutter-view

bb00f8a

Fix comments

0ab230a

htoor3 requested review from yjbanov, ditman and mdebbar April 4, 2023 19:47

ditman reviewed Apr 4, 2023

View reviewed changes

flutter-dashboard bot added the will affect goldens label Apr 4, 2023

ditman approved these changes Apr 4, 2023

View reviewed changes

htoor3 mentioned this pull request Apr 4, 2023

[web] Skip temporarily broken test flutter/flutter#124160

Merged

8 tasks

Merge remote-tracking branch 'upstream/main' into pass-autofill-reopen

742960a

htoor3 added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 5, 2023

auto-submit bot merged commit 74d7933 into flutter:main Apr 5, 2023
34 checks passed

engine-flutter-autoroll mentioned this pull request Apr 6, 2023

Roll Flutter Engine from 610cdf0ae117 to 74d7933553c2 (3 revisions) flutter/flutter#124294

Closed

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

74d793355 [web] Move text editing nodes outside of shadowDOM - reopen…

980898b

…ed (flutter/engine#40904)

htoor3 added a commit that referenced this pull request Apr 6, 2023

Revert "[web] Move text editing nodes outside of shadowDOM - reopened (…

7a0f1f6

…#40904)" This reverts commit 74d7933.

htoor3 mentioned this pull request Apr 6, 2023

Revert "[web] Move text editing nodes outside of shadowDOM - reopened" #40960

Merged

zanderso pushed a commit that referenced this pull request Apr 6, 2023

Revert "[web] Move text editing nodes outside of shadowDOM - reopened" (

1ec74c6

#40960) Reverts #40904

engine-flutter-autoroll mentioned this pull request Apr 6, 2023

Manual roll Flutter Engine from 610cdf0ae117 to 1ec74c62fd3f (4 revisions) flutter/flutter#124300

Merged

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

74d793355 [web] Move text editing nodes outside of shadowDOM - reopen…

f64ad6a

…ed (flutter/engine#40904)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Move text editing nodes outside of shadowDOM - reopened #40904

[web] Move text editing nodes outside of shadowDOM - reopened #40904

htoor3 commented Apr 4, 2023 •

edited by yjbanov

ditman Apr 4, 2023

flutter-dashboard bot commented Apr 4, 2023

ditman left a comment

htoor3 commented Apr 4, 2023 •

edited

jonahwilliams commented Apr 6, 2023

zanderso commented Apr 6, 2023

ditman commented Apr 6, 2023

htoor3 commented Apr 6, 2023

[web] Move text editing nodes outside of shadowDOM - reopened #40904

[web] Move text editing nodes outside of shadowDOM - reopened #40904

Conversation

htoor3 commented Apr 4, 2023 • edited by yjbanov

ditman Apr 4, 2023

Choose a reason for hiding this comment

flutter-dashboard bot commented Apr 4, 2023

ditman left a comment

Choose a reason for hiding this comment

htoor3 commented Apr 4, 2023 • edited

jonahwilliams commented Apr 6, 2023

zanderso commented Apr 6, 2023

ditman commented Apr 6, 2023

htoor3 commented Apr 6, 2023

htoor3 commented Apr 4, 2023 •

edited by yjbanov

htoor3 commented Apr 4, 2023 •

edited