Upstream changes necessary for text editing in flutter web #39344

mdebbar · 2019-08-27T18:40:20Z

In flutter web, we place a transparent DOM input field right on top of flutter's text field. This enables us to leverage things like context menu, cut/copy/paste, spellcheck and autocomplete from the browser.

To make that possible, we need the framework to send more information about the text field such as text styling, location, size, transform matrix, etc. This PR provides that information in the form of platform messages.

This PR also disables flutter's text field toolbar when running inside the browser because we want to show the browser's native toolbar and context menu.

Fixes #39343

jonahwilliams · 2019-08-27T18:54:41Z

packages/flutter/lib/src/services/text_input.dart

I think this information could also be used for autocomplete on android, see https://developer.android.com/reference/android/view/autofill/AutofillManager.html#notifyViewEntered(android.view.View)

Since the info is sent as a platform message, all platforms will be able to take advantage of it.

What do you think of creating a class that contains all information pertaining to an editable? We could extend this class to include other data in the future without changing the signature of this method?

Having said that, we probably want to keep things that are updated on different schedules separate. For example, positioning and size are updated on every frame, but autocomplete might be scheduled based on user input. SemanticsNode is one example that contains everything in the world and I'm not sure I'm a fan of that design. We end up pushing way more data than we need on every frame.

jonahwilliams · 2019-08-27T18:56:14Z

packages/flutter/lib/src/services/text_input.dart

It is a layering violation for services to know about painting. Instead, the system channels here should probably only expose the primitive values, and leave interpreting text style to the views themselves.

jonahwilliams · 2019-08-27T18:56:26Z

packages/flutter/lib/src/widgets/editable_text.dart

Suggested change

if(kIsWeb) {

if (kIsWeb) {

ferhatb · 2019-08-28T23:05:18Z

packages/flutter/lib/src/widgets/editable_text.dart

Suggestion: Change comment to : Optimize by skipping the update if nothing has changed since last paint.

yjbanov · 2019-08-29T14:44:09Z

packages/flutter/lib/src/rendering/editable.dart

onPaint is not a good name for this. RenderEditable does not paint when scrolled inside a parent scrollable. We may stop depending on painting altogether and find another method for updating the TextConnection, but the name will stick. I'm now thinking if the proper fix does involve having this callback here at all, and so adding it now will lead to a future breaking change 🤔

yjbanov · 2019-08-29T14:50:53Z

packages/flutter/lib/src/services/text_input.dart

What do you think of creating a class that contains all information pertaining to an editable? We could extend this class to include other data in the future without changing the signature of this method?

Having said that, we probably want to keep things that are updated on different schedules separate. For example, positioning and size are updated on every frame, but autocomplete might be scheduled based on user input. SemanticsNode is one example that contains everything in the world and I'm not sure I'm a fan of that design. We end up pushing way more data than we need on every frame.

yjbanov · 2019-08-29T14:52:09Z

packages/flutter/lib/src/services/text_input.dart

Is renderBoxSize a good name? How about editableBoxSize?

yjbanov · 2019-08-29T14:53:46Z

packages/flutter/lib/src/services/text_input.dart

How can this method guarantee that the values are taken from the render box? This method is public and developers can use it however they like. I think instead we should say that the size is used to size the editable box.

yjbanov · 2019-08-29T14:55:52Z

packages/flutter/lib/src/services/text_input.dart

Can we use transform.storage?

yjbanov · 2019-08-29T15:06:05Z

packages/flutter/lib/src/widgets/editable_text.dart

I wonder if we should have a more descriptive and less platform-dependent kUsePlatformContextMenu or something. I imagine this being used in other situations, such as links and images, and on other platforms, such as Flutter for desktop.

tvolkert · 2019-08-30T17:11:19Z

It looks like this was merged while the tree was red. Please do not merge while the tree is red unless you are specifically merging a change to turn the tree green (https://github.com/flutter/flutter/wiki/Tree-hygiene#overview), as it complicates the job trying to get the tree green again.

mdebbar · 2019-08-30T17:14:30Z

Sorry, that was my bad. Should I revert or keep?

tvolkert · 2019-08-30T17:17:01Z

No need to revert - just FYI for the future. Thanks!

Hixie · 2019-09-03T19:40:25Z

@mdebbar It looks like the gallery transition perf test started failing around when you landed this. Can you see if you can figure out if it was this commit, or if not, which commit it was?

Nipodemos · 2019-09-05T00:29:11Z

hello
just a reminder that this PR is still on ToDo tab on "Unfork flutter from Web" Project.
Since it's already merged, shoudn't this be on Done tab?

aleor · 2019-09-12T20:15:24Z

Hello @mdebbar, @tvolkert, @xster, @Hixie
I've started getting exceptions on iOS when using flutter dev or master channel. They happen when CupertinoTextField widget gets a focus or on any change (i.e. on each new symbol typed). It doesn't happen on release 1.9, though.

Error message: "No implementation found for method TextInput.setEditableSizeAndTransform on channel flutter/textinput", stack:

MethodChannel.invokeMethod (/Users/ra/dev/flutter/packages/flutter/lib/src/services/platform_channel.dart:319) <asynchronous gap> (Unknown Source:0) OptionalMethodChannel.invokeMethod (/Users/ra/dev/flutter/packages/flutter/lib/src/services/platform_channel.dart:429) <asynchronous gap> (Unknown Source:0) TextInputConnection.setEditableSizeAndTransform (/Users/ra/dev/flutter/packages/flutter/lib/src/services/text_input.dart:681) EditableTextState._updateSizeAndTransform (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/editable_text.dart:1661) EditableTextState._openInputConnection (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/editable_text.dart:1350) EditableTextState._openOrCloseInputConnectionIfNeeded (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/editable_text.dart:1375) EditableTextState._handleFocusChanged (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/editable_text.dart:1631) EditableTextState._handleFocusChanged (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/editable_text.dart:0) ChangeNotifier.notifyListeners (/Users/ra/dev/flutter/packages/flutter/lib/src/foundation/change_notifier.dart:206) FocusNode._notify (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/focus_manager.dart:765) FocusManager._applyFocusChange (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/focus_manager.dart:1331) FocusManager._applyFocusChange (/Users/ra/dev/flutter/packages/flutter/lib/src/widgets/focus_manager.dart:0) _rootRun (dart:async/zone.dart:1120) _rootRun (dart:async/zone.dart:0) _CustomZone.run (dart:async/zone.dart:1021) _CustomZone.runGuarded (dart:async/zone.dart:923) _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:963) _rootRun (dart:async/zone.dart:1124) _rootRun (dart:async/zone.dart:0) _CustomZone.run (dart:async/zone.dart:1021) _CustomZone.runGuarded (dart:async/zone.dart:923) _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:963) _microtaskLoop (dart:async/schedule_microtask.dart:41) _startMicrotaskLoop (dart:async/schedule_microtask.dart:50) _startMicrotaskLoop (dart:async/schedule_microtask.dart:0)

Another exception is about TextInput.setStyle implementation being not found.

nturgut · 2019-09-13T16:22:31Z

Thanks @aleor, I am actively working on it now.

gdomingues · 2019-10-22T19:49:19Z

Hello @nturgut , I have experienced the situation above on Android, though the exceptions are swallowed and can only be observed if you run the app in debug mode on VS Code and mark the "All Exceptions" checkbox on debug panel.

Besides that, this bug was introduced by this pull request. I'm circumventing it commenting out the assert on this line of text_input.dart.

mdebbar added a: text input Entering text in a text field or keyboard related problems platform-web Web applications specifically labels Aug 27, 2019

mdebbar added this to the August 2019 milestone Aug 27, 2019

mdebbar requested review from Hixie, ferhatb, jonahwilliams and xster August 27, 2019 18:40

mdebbar self-assigned this Aug 27, 2019

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 27, 2019

googlebot added the cla: yes label Aug 27, 2019

jonahwilliams reviewed Aug 27, 2019

View reviewed changes

lesnitsky mentioned this pull request Aug 28, 2019

[web][upstream] Upstream new platform messages for text editing #39343

Closed

mdebbar requested review from jonahwilliams and yjbanov August 28, 2019 17:36

ferhatb reviewed Aug 28, 2019

View reviewed changes

yjbanov reviewed Aug 29, 2019

View reviewed changes

yjbanov approved these changes Aug 29, 2019

View reviewed changes

mdebbar added 5 commits August 29, 2019 15:21

Upstream changes necessary for text editing in flutter web

020e77a

Remove the dependency from services to painting

887907e

Fix some analyzer errors

34e3e70

More reliable way to update editable's transform

bf908d4

Address Yegor's comments

d0ee01d

mdebbar force-pushed the upstream_text_editing branch from f58e0c8 to d0ee01d Compare August 29, 2019 22:21

mdebbar merged commit a2957c5 into flutter:master Aug 30, 2019

mdebbar mentioned this pull request Sep 4, 2019

Handle new navigation platform messages flutter/engine#11880

Merged

nturgut mentioned this pull request Sep 13, 2019

TextInput.setStyle and TextInput.setEditableSizeAndTransform calls not handled in IOS/Android #40438

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021

Upstream changes necessary for text editing in flutter web #39344

Upstream changes necessary for text editing in flutter web #39344

Uh oh!

Conversation

mdebbar commented Aug 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvolkert commented Aug 30, 2019

Uh oh!

mdebbar commented Aug 30, 2019

Uh oh!

tvolkert commented Aug 30, 2019

Uh oh!

Hixie commented Sep 3, 2019

Uh oh!

Nipodemos commented Sep 5, 2019

Uh oh!

aleor commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nturgut commented Sep 13, 2019

Uh oh!

gdomingues commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

mdebbar commented Aug 27, 2019 •

edited

Loading

aleor commented Sep 12, 2019 •

edited

Loading

gdomingues commented Oct 22, 2019 •

edited

Loading