Add support for software text editing controls #15560

edman · 2020-01-13T21:29:23Z

Includes selection, copy, cut, paste, as well as partial support for up
and down movement.

Text editing controls can be accessed in GBoard by:
top-left arrow > three dots menu > text editing

Partial fix for flutter/flutter#9419 and flutter/flutter#37371.

Before

After

edman · 2020-01-13T21:39:39Z

Some comments about the limitations of this PR:

Home/End are not supported. I'm not sure where events for those keys are sent in the engine.

Up/Down work except when moving around long lines that wrap around the view (see the video above). Basically the InputConnectionAdaptor does not know the width of the text view and thinks text always fits.

engine/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java

Lines 47 to 49 in 72420a3

    
           // We create a dummy Layout with max width so that the selection 
        
           // shifting acts as if all text were in one line. 
        
           mLayout = new DynamicLayout(mEditable, new TextPaint(), Integer.MAX_VALUE, Layout.Alignment.ALIGN_NORMAL, 1.0f, 0.0f, false);

justinmc

A few nitpicks but otherwise LGTM 👍

Right now this will not support the home/end buttons, so we should add a note to flutter/flutter#9419 about that when this is merged.

@GaryQian Any last minute ideas for getting the width here? Otherwise I think the current up/down arrow behavior is ok for now. Also, what do you think about tests for this?

shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java

GaryQian · 2020-01-15T22:00:43Z

Obtaining the width at the embedder level is non-trivial. Unless it is urgently needed, I would hold off on developing a system to pass width down to there.

The sendKeyEvent method should be unit-testable?

justinmc · 2020-01-16T01:08:48Z

Alright let's leave the up/down behavior as-is for now and revisit if needed.

Unit testing sounds like a good idea. @edman You'll have to add a new unit test file for this since one doesn't already exist for InputConnectionAdaptor unfortunately. It should be somewhat similar to the unit test file added in this PR: https://github.com/flutter/engine/pull/12432/files#diff-af19d10c7a69b9cab3ee96c4f64a9a9cR1

edman · 2020-01-16T23:26:34Z

shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java

+        // WIP
+        // The asserts below don't work. KEYCODE_DPAD_UP moves the caret to
+        // index 0, as if the text were in one single line.
+        // assertEquals(expectedSelection, Selection.getSelectionStart(editable));
+        // assertEquals(expectedSelection, Selection.getSelectionEnd(editable));


Couldn't get the up/down tests to work as expected yet.

Up moves selection to beginning of the text and down moves to end. There's a line break in the text so it should move to the next/previous line instead.

Not sure why it behaves differently in tests.. Will look more tomorrow.

So I have noticed through working on other bugs that depending on the input method, some keystrokes/events do not pass through this method at all. I am not yet clear on where those events go but that behavior may have something to do with this.

justinmc · 2020-02-18T21:13:49Z

~~We should check whether this fixes flutter/flutter#50875.~~ Nevermind, that issue is iOS specific.

gschier · 2020-02-19T21:33:51Z

RenderEditable contains its own logic for arrow navigation of hardware keyboards, including up/down. Could leave it up to the UI layer for this too maybe.

https://github.com/flutter/flutter/blob/11549e45a386220cbacd683b8a94cbac85d62a41/packages/flutter/lib/src/rendering/editable.dart#L480

edman · 2020-02-26T18:03:51Z

@justinmc I changed the test logic and added a note.

justinmc

LGTM

I think this way of doing the tests is fine with the note. There is one failure, I'll try to see if it's an infrastructure thing.

justinmc · 2020-02-28T00:40:23Z

@gschier Thanks for pointing that out, I hadn't realized it was being handled in both places. I talked to @gspencergoog and it sounds like it's due for a refactor to consolidate. This should be fine for now.

fluttergithubbot · 2020-02-28T00:41:01Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Mac Web Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

justinmc · 2020-02-28T02:03:43Z

Hmm I reran that failure and it succeeded, but it's still red. @edman Can you try merging with master and pushing?

Includes selection, copy, cut, paste, as well as partial support for up and down movement. Text editing controls can be accessed in GBoard by: top-left arrow > three dots menu > text editing Partial fix for flutter/flutter#9419 and flutter/flutter#37371.

Run with: testing/run_tests.py --type=java --java-filter=io.flutter.plugin.editing.InputConnectionAdaptorTest

edman · 2020-02-28T18:18:37Z

@GaryQian FYI we'll be landing this once the tree is green.

…e#15560)

* 9beac71 Add support for software text editing controls (flutter/engine#15560) * ab9f83f Roll fuchsia/sdk/core/linux-amd64 from RYDur... to bgFop... (flutter/engine#16855) * 62d0c08 Roll src/third_party/dart dda5bcee00d3..4dad6d77ba50 (6 commits) (flutter/engine#16856) * 76d12d2 Roll src/third_party/skia 03d9e8af0d25..262796edeba6 (11 commits) (flutter/engine#16857) * 01a52b9 Try rasterizing images and layers only once, even when their rasterization fails. Further enforce the same access threshold on layers as on Pictures. Previously layers would always be cached. The latter is a semantic change. (flutter/engine#16545) * 3b0d1a8 script for fetching correct flutter version (flutter/engine#16818) * 9746ddb Roll src/third_party/dart 4dad6d77ba50..6708f6d4c7df (15 commits) (flutter/engine#16860) * 90736bb Roll src/third_party/skia 262796edeba6..54cb21430ccb (23 commits) (flutter/engine#16861) * 142882e Roll src/third_party/skia 54cb21430ccb..e1ae9c4bcf2e (4 commits) (flutter/engine#16863) * 7192356 Manual roll of Dart 09bbd3cca5...6708f6d4c7 (flutter/engine#16864) * fae5ff6 Roll src/third_party/skia e1ae9c4bcf2e..5d1c3e2ead61 (2 commits) (flutter/engine#16865) * c6525a4 Roll src/third_party/skia 5d1c3e2ead61..59b160f99106 (2 commits) (flutter/engine#16866) * 755e2b5 Roll src/third_party/skia 59b160f99106..71a20b2685c6 (1 commits) (flutter/engine#16867) * 93d0eff Roll src/third_party/skia 71a20b2685c6..ecbb0fb2d5bc (1 commits) (flutter/engine#16869) * bfee5ae Roll fuchsia/sdk/core/linux-amd64 from bgFop... to F_Ihm... (flutter/engine#16871) * ca29b47 Roll fuchsia/sdk/core/mac-amd64 from K26F5... to 79I0C... (flutter/engine#16872) * 708ca2c Roll src/third_party/skia ecbb0fb2d5bc..666707336e07 (1 commits) (flutter/engine#16873) * 78ea291 Roll fuchsia/sdk/core/mac-amd64 from 79I0C... to NoQzJ... (flutter/engine#16874) * 7b43bb8 Roll fuchsia/sdk/core/linux-amd64 from F_Ihm... to 22R78... (flutter/engine#16875) * 75c9c62 Roll src/third_party/skia 666707336e07..231f1bf56556 (1 commits) (flutter/engine#16876) * 7a86b83 Roll src/third_party/dart 09bbd3cca5d4..860dca93ea42 (1 commits) (flutter/engine#16877) * d0c87e2 Roll src/third_party/skia 231f1bf56556..d6205322cdc5 (1 commits) (flutter/engine#16878) * b4c5854 Roll src/third_party/skia d6205322cdc5..6729496a037f (1 commits) (flutter/engine#16879) * 03f0ee5 Roll src/third_party/skia 6729496a037f..367dbff98555 (1 commits) (flutter/engine#16880) * d84b968 Roll fuchsia/sdk/core/mac-amd64 from NoQzJ... to q2DAy... (flutter/engine#16881) * 4490e7c Roll fuchsia/sdk/core/linux-amd64 from 22R78... to 9NHsJ... (flutter/engine#16882) * a39e7ac Roll src/third_party/skia 367dbff98555..986680240f81 (1 commits) (flutter/engine#16884) * 62b1e50 Roll src/third_party/dart 860dca93ea42..fbe9f6115d2f (9 commits) (flutter/engine#16885) * 5e474ee Roll src/third_party/skia 986680240f81..9dd0bd78b2d7 (2 commits) (flutter/engine#16886)

Fixes flutter/flutter#101569 This block of code was originally introduced in #15560 , but removing it does not have any affect on the software text editing controls in GBoard. Before this change * shift + arrow right/left selection would collapse after releasing the shift key. * shift + mouse click to expand selection would collapse after releasing the shift key. After this change * shift key up no longer collapses the selection.

googlebot added the cla: yes label Jan 13, 2020

auto-assign bot requested a review from GaryQian January 13, 2020 21:29

edman requested a review from justinmc January 13, 2020 21:30

edman force-pushed the textediting branch 3 times, most recently from 93d68c7 to 04685c5 Compare January 13, 2020 22:00

justinmc approved these changes Jan 14, 2020

View reviewed changes

shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java Outdated Show resolved Hide resolved

shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java Outdated Show resolved Hide resolved

edman force-pushed the textediting branch from 04685c5 to 7eb5bc9 Compare January 15, 2020 21:05

edman force-pushed the textediting branch from 7eb5bc9 to 10fe8e4 Compare January 16, 2020 23:08

edman commented Jan 16, 2020

View reviewed changes

This was referenced Jan 23, 2020

[Android] Tracking - Text edition issues memspace/zefyr#237

Open

[Web] Tracking - Text edition issues memspace/zefyr#238

Open

justinmc mentioned this pull request Feb 18, 2020

Multiline text up/down arrow navigation doesn't work on iOS hardware keyboards flutter/flutter#50875

Closed

edman force-pushed the textediting branch from 10fe8e4 to 5e830c3 Compare February 26, 2020 18:02

edman force-pushed the textediting branch from 5e830c3 to 357929c Compare February 26, 2020 18:29

justinmc approved these changes Feb 28, 2020

View reviewed changes

justinmc added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 28, 2020

fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 28, 2020

edman added 3 commits February 28, 2020 18:30

Introduce InputConnectionAdaptor tests

6802529

Run with: testing/run_tests.py --type=java --java-filter=io.flutter.plugin.editing.InputConnectionAdaptorTest

Fix BUILD.gn comment on run_tests.py --java-filter flag

a501e49

edman force-pushed the textediting branch from 357929c to a501e49 Compare February 28, 2020 17:35

edman merged commit 9beac71 into flutter:master Feb 28, 2020

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 29, 2020

9beac71 Add support for software text editing controls (flutter/engin…

c297660

…e#15560)

engine-flutter-autoroll mentioned this pull request Feb 29, 2020

Roll engine 96061d62281d..142882e1baea (9 commits) flutter/flutter#51689

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 2, 2020

9beac71 Add support for software text editing controls (flutter/engin…

7547eac

…e#15560)

engine-flutter-autoroll mentioned this pull request Mar 2, 2020

Roll engine 96061d62281d..5e474ee860a3 (29 commits) flutter/flutter#51792

Merged

justinmc mentioned this pull request Mar 18, 2020

Several questions about CupertinoWidget flutter/flutter#37371

Closed

justinmc mentioned this pull request Mar 31, 2020

Make DPAD movement consider grapheme clusters #17420

Merged

This was referenced May 16, 2023

[Android]Text selection using the physical keyboard is lost on releasing shift key flutter/flutter#101569

Closed

[Android] Do not collapse text selection on shift key up #42075

Merged

Add support for software text editing controls #15560

Add support for software text editing controls #15560

Uh oh!

Conversation

edman commented Jan 13, 2020

Before

After

Uh oh!

edman commented Jan 13, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GaryQian commented Jan 15, 2020

Uh oh!

justinmc commented Jan 16, 2020

Uh oh!

edman Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaryQian Jan 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc commented Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschier commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edman commented Feb 26, 2020

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc commented Feb 28, 2020

Uh oh!

fluttergithubbot commented Feb 28, 2020

Uh oh!

justinmc commented Feb 28, 2020

Uh oh!

edman commented Feb 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

edman Jan 16, 2020 •

edited

Loading

GaryQian Jan 31, 2020 •

edited

Loading

justinmc commented Feb 18, 2020 •

edited

Loading

gschier commented Feb 19, 2020 •

edited

Loading