iPhone floating cursor selection

[moffatman]

Floating cursor selection hasn't worked as we haven't been returning a real value for caretRectForPosition:. If we have the selection rectangles, we can do so.

Fixes #30476

Requires selection rects to be turned on for iPhone (https://github.com/flutter/flutter/pull/113048)

Pre-launch Checklist

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

[chinmaygarde]

cc @cbracken @jmagman

[jmagman]

@LongCatIsLooong might know the most about this #26486

[LongCatIsLooong]

Is this implementing this feature?

A second force press leaves a selection anchor and then selects full words between the anchor and the current floating cursor position.

I think we should do that in the framework if possible. Is there a way that allows us to detect the second force press? Sending the bounding boxes of visible glyphs is going to be very expensive.

[moffatman]

I haven't found any way to detect the second press, the keyboard just changes the selection based on the position of the floating cursor.

[jmagman]

@LongCatIsLooong do you have any more review requests or can this land?

[moffatman]

@LongCatIsLooong We are already sending the rects since https://github.com/flutter/flutter/pull/113048. I don't know how to look up benchmark results, maybe you can check the impact?

[LongCatIsLooong]

Sorry for the delayed response. I'm not aware of any existing benchmarks that cover this, the framework only sends new rects when the underlying text layout changes, or when the text field stops scrolling (and if the text is too long when the floating cursor moves close enough to an edge the text field may scroll, that could break the interaction here too), and I don't think the performance of entering text is currently measured/monitored (~it's probably worth doing, I'll create an issue for that~ on a second thought, may not be a good idea since we have to interact with the OS's input system https://github.com/flutter/flutter/issues/115595).

Is the 2nd force press event detectable via {begin/update/end}floatingCursor? If so I think we can replicate UIKit's behavior in the framework, like we do for regular gestures?

[moffatman]

We can't find out about the second force press, the system just sends selection updates if used once we have the correct rectangles.

[LongCatIsLooong]

Ah I see it's using _UIKeyboardTextSelectionInteraction. What happens when you use the spacebar selection gesture to extend the selection outside of the text field's bounds? I think the framework currently stops sending rects if the text field starts to scroll.

[moffatman]

Right now only the rects of the visible characters are sent so you can't scroll outside of the current viewport. I just tried always sending all the rects, it works surprisingly well, as since the rects aren't updated during scroll, they remain stable as UIKit changes the selection, which causes the framework to scroll. But it sometimes seems kind of glitchy, there may be some update leaking through from somewhere.

In general you wouldn't need to send all the rects to get scrolling to work, since the keyboard is only a fixed width. Probably it would be optimal to send the current visible characters as well as the same width of the view off of each edge. No idea about vertical scrolling though...

[chinmaygarde]

Are we making progress on this? Can we land this perhaps and file a followup?

[jmagman]

Almost ready to land, waiting on answer to last questions from @LongCatIsLooong

[moffatman]

Upon revisiting this to check out linebreak / RTL text I think we will need some other framework changes first. Mainly to avoid existing floating cursor implementation from conflicting. Also working on engine improvements to handle various edge cases in caret rect positioning.

[jmagman]

@moffatman let us know when this is ready to re-review.

[moffatman]

@jmagman It's ready for re-review

[moffatman]

Here is the Skia change: https://skia-review.googlesource.com/c/skia/+/619838

Will also need another framework PR to send the text directionality of each selection rect

[jmagman]

Adding @hellohuanlin for review.

[hellohuanlin]

@moffatman Hi, I just want to double check - the solution is not completed yet right?

I checked out your branch and try it on a dummy project, and it does not seem to support selections yet.

Flutter with your PR:

https://user-images.githubusercontent.com/41930132/209243560-c50933d3-5fa5-44cc-a630-ea59ea576051.MOV

Apple: https://user-images.githubusercontent.com/41930132/209243544-d66fc74e-e89a-4c82-8198-0a72c11ed5ee.MOV

[hellohuanlin]

Will also need another framework PR to send the text directionality of each selection rect

Gotcha, should I checkout both this PR and https://github.com/flutter/flutter/pull/117436 to verify?

[moffatman]

@hellohuanlin Yes, you'll need the framework PR there as well.

Also, there is a problem going on at the same time, there was a change of a method channel name in both framework and engine from TextInput.setSelectionRects to Scribble.setSelectionRects. And every time I rebase my work here, the framework PR seems to keep getting either rebased or relanded.

I think the current status is that the framework name change is currently not included, it would be merged here: https://github.com/flutter/flutter/pull/115296. So to get my solution to work, you will have to change the name of the setSelectionRects method in either the framework or engine so that both are the same.

[hellohuanlin]

I pulled your framework change. But got this issue. Not sure what this is (probably not related to your PR). I will take a closer look after Christmas.

[hellohuanlin]

@moffatman looks like NullThrownError is deprecated: https://github.com/flutter/flutter/commit/f5249bcb0a26118cffad7413325ab42cbe6c7c09

You probably wanna rebase. For now I will manually make the change.

[hellohuanlin]

@hellohuanlin Yes, you'll need the framework PR there as well.

Also, there is a problem going on at the same time, there was a change of a method channel name in both framework and engine from TextInput.setSelectionRects to Scribble.setSelectionRects. And every time I rebase my work here, the framework PR seems to keep getting either rebased or relanded.

I think the current status is that the framework name change is currently not included, it would be merged here: flutter/flutter#115296. So to get my solution to work, you will have to change the name of the setSelectionRects method in either the framework or engine so that both are the same.

I tried this and it didn't work - same result as my previous video. Is there anything else I need to do to test this out?

[moffatman]

@hellohuanlin Any errors in console? And did you change the method name in _PlatformTextInputControl.setSelectionRects?

[hellohuanlin]

@moffatman I renamed to TextInput.setSelectionRects. No errors in console.

[moffatman]

@hellohuanlin What was kSetSelectionRectsMethod in shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm?

[chinmaygarde]

Are we making progress on this?

[moffatman]

Need the following to be merged first

https://skia-review.googlesource.com/c/skia/+/619838/1 @jason-simmons (?)

https://github.com/flutter/flutter/pull/115296 @justinmc

Will do a rebase at some point soon and see why it didn't work for @hellohuanlin

[chinmaygarde]

Pinging @jason-simmons on the Skia PR.

[chinmaygarde]

We have a plan to land the Skia patch but that involves updating a test assertion which will require manual intervention. Jason is going to attempt to do that after the Clang roll.