[Android] Add support for text processing actions #44579

bleroux · 2023-08-10T11:53:55Z

Description

This Android related PR adds a channel buffer and a plugin to interact with Android 'process text' feature. It makes it possible to query text processing actions and to run those actions (for instance 'calling' Google translate).
Text actions that outputs a processed text are supported.

The implementation is based on the great sample provided by @gualse , see flutter/flutter#107603 (comment).

In order to return a non empty list of text actions, the implementation will require adding a section to the Android manifest file (see flutter/flutter#107603 (comment)).
Adding this section automatically to new or existing Flutter apps is not part of this PR but will be tackled in a future PR.

Related Issue

Android engine side for flutter/flutter#139361

Tests

Adds 3 tests.

justinmc

This approach LGTM. I think it's good to keep this separate in its own method channel.
CC @camsim99 in case you have any quick thoughts from the Android side of things.

justinmc · 2023-08-14T20:42:33Z

shell/platform/android/io/flutter/plugin/text/ProcessTextPlugin.java

+      infos = packageManager.queryIntentActivities(intent, PackageManager.ResolveInfoFlags.of(0));
+    } else {
+      infos = packageManager.queryIntentActivities(intent, 0);


What's the difference between the calls in these two versions? I'm just wondering if it's anything we should be documenting or anything like that.

The previous version is deprecated, the new one allow using 'long' values instead of 'int' for the previous one.
I will probabl add a comment above the call to the old one to mention it is deprecated.

camsim99

The approach looks good to me! Just a question on some constructor logic.

camsim99 · 2023-08-14T22:45:01Z

shell/platform/android/io/flutter/plugin/text/ProcessTextPlugin.java

+    this.processTextChannel = processTextChannel;
+    this.packageManager = processTextChannel.packageManager;
+
+    this.cacheResolveInfos();


I'm wondering if we can do this elsewhere because this will happen immediately when a ProcessTextPlugin is created, and that will happen regardless of whether or not this functionality is used. Would it make sense to be part of queryTextActions() or processTextAction(...)?

Very good point, I will try lazy loading instead.

camsim99

Looks good, just some nits!

camsim99 · 2023-10-10T22:01:18Z

shell/platform/android/io/flutter/plugin/text/ProcessTextPlugin.java

+  @Override
+  public Map<String, String> queryTextActions() {
+    if (resolveInfosById == null) {
+      resolveInfosById = new HashMap<String, ResolveInfo>();


Nit: can this happen in cacheResolveInfos versus here?

Yes, great suggestion, and doing so we can remove the resolveInfosById.clear()call your mention below.

camsim99 · 2023-10-10T22:02:27Z

shell/platform/android/io/flutter/plugin/text/ProcessTextPlugin.java

+      infos = packageManager.queryIntentActivities(intent, 0);
+    }
+
+    resolveInfosById.clear();


Is this ever called where resolveInfosById wouldn't be an empty array? Wondering if we need this call.

…engine#44579)

…136346) flutter/engine@cb0dabd...d93fe23 2023-10-11 leroux_bruno@yahoo.fr [Android] Add support for text processing actions (flutter/engine#44579) 2023-10-11 skia-flutter-autoroll@skia.org Roll Skia from 4935bed4260d to 11e41e0e2f9f (3 revisions) (flutter/engine#46759) 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 jsimmons@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://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

)" This reverts commit d93fe23.

@bleroux

…46788) This reverts commit d93fe23. This change is causing integration tests to fail on attempts to roll the latest master version of flutter into the packages repo. See a sample failure here: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8767521956894285553/+/u/Run_package_tests/native_integration_tests/stdout The root cause seems to be that we are getting request codes in `onActivityResult` that we never added to the `requestsByCode` map, so the call to remove returns null (and we then call success getting a NPE). More info: 1. There is a [discussion in discord here](https://discord.com/channels/608014603317936148/1161718667566919761/1161721935927980052) 2. And the failure can be reproduced by running `dart run script/tool/bin/flutter_plugin_tools.dart native-test --android --packages file_selector` from the root of the packages repo (with a flutter checkout that contains these changes). cc @bleroux [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

## Description This is a reland of #44579 which was reverted in #46788. This reland adds a check into `onActivityResult` in order to return early if the result is related to an unknown request code (aka the result is related to a request sent by another plugin). It also adds one test that simulates receiving such an unknown request code. ## Related Issue Android engine side for flutter/flutter#107603 ## Tests Adds 4 tests.

@gualse

## Description This Android related PR adds a channel buffer and a plugin to interact with Android 'process text' feature. It makes it possible to query text processing actions and to run those actions (for instance 'calling' Google translate). Text actions that outputs a processed text are supported. The implementation is based on the great sample provided by @gualse , see flutter/flutter#107603 (comment). In order to return a non empty list of text actions, the implementation will require adding a section to the Android manifest file (see flutter/flutter#107603 (comment)). Adding this section automatically to new or existing Flutter apps is not part of this PR but will be tackled in a future PR. ## Related Issue Android engine side for flutter/flutter#107603 ## Tests Adds 3 tests.

@bleroux

…46788) This reverts commit d93fe23. This change is causing integration tests to fail on attempts to roll the latest master version of flutter into the packages repo. See a sample failure here: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8767521956894285553/+/u/Run_package_tests/native_integration_tests/stdout The root cause seems to be that we are getting request codes in `onActivityResult` that we never added to the `requestsByCode` map, so the call to remove returns null (and we then call success getting a NPE). More info: 1. There is a [discussion in discord here](https://discord.com/channels/608014603317936148/1161718667566919761/1161721935927980052) 2. And the failure can be reproduced by running `dart run script/tool/bin/flutter_plugin_tools.dart native-test --android --packages file_selector` from the root of the packages repo (with a flutter checkout that contains these changes). cc @bleroux [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

## Description This is a reland of #44579 which was reverted in #46788. This reland adds a check into `onActivityResult` in order to return early if the result is related to an unknown request code (aka the result is related to a request sent by another plugin). It also adds one test that simulates receiving such an unknown request code. ## Related Issue Android engine side for flutter/flutter#107603 ## Tests Adds 4 tests.

## Description This PR adds `ProcessTextService` on the framework side to communicate with the engine to query and run text processing actions (on the engine side, only Android is supported currently, see flutter/engine#44579). ## Related Issue Non-UI framework side for #107603 ## Tests Adds 3 tests.

…7207) ## Description This PR adds a new section to the Android manifest file. This section is required for flutter/engine#44579 (because it uses `PackageManager.queryIntentActivities` API ), see: 1. https://developer.android.com/training/package-visibility?hl=en 2. https://developer.android.com/reference/android/content/Intent#ACTION_PROCESS_TEXT 3. https://developer.android.com/reference/android/content/pm/PackageManager?hl=en#queryIntentActivities(android.content.Intent,%20android.content.pm.PackageManager.ResolveInfoFlags) ## Related Issue Android manifest update for #107603 ## Tests This PR updates the integration tests and examples Android manifest files, this will help catch error or warning related to this change.

…tter#137207) ## Description This PR adds a new section to the Android manifest file. This section is required for flutter/engine#44579 (because it uses `PackageManager.queryIntentActivities` API ), see: 1. https://developer.android.com/training/package-visibility?hl=en 2. https://developer.android.com/reference/android/content/Intent#ACTION_PROCESS_TEXT 3. https://developer.android.com/reference/android/content/pm/PackageManager?hl=en#queryIntentActivities(android.content.Intent,%20android.content.pm.PackageManager.ResolveInfoFlags) ## Related Issue Android manifest update for flutter#107603 ## Tests This PR updates the integration tests and examples Android manifest files, this will help catch error or warning related to this change.

Используется, например, для обработки текста при выделении. Пока никакого эффекта, но заработает как только релизнут flutter со следующим PR: flutter/engine#44579

github-actions bot added the platform-android label Aug 10, 2023

bleroux force-pushed the android_process_text branch from 0bab9c3 to d9e1b29 Compare August 10, 2023 12:37

bleroux requested a review from justinmc August 11, 2023 14:14

bleroux mentioned this pull request Aug 11, 2023

Flutter should respect custom system-wide text selection toolbar buttons flutter/flutter#107603

Open

justinmc reviewed Aug 14, 2023

View reviewed changes

camsim99 reviewed Aug 14, 2023

View reviewed changes

bleroux force-pushed the android_process_text branch from d9e1b29 to baf8e4c Compare October 3, 2023 07:36

bleroux changed the title ~~WIP - [Android] Add support for text processing actions~~ [Android] Add support for text processing actions Oct 3, 2023

bleroux marked this pull request as ready for review October 3, 2023 07:40

bleroux force-pushed the android_process_text branch 7 times, most recently from c8bb275 to 6a4274b Compare October 4, 2023 06:06

bleroux requested a review from camsim99 October 4, 2023 13:12

bleroux force-pushed the android_process_text branch 3 times, most recently from 08753a2 to 641c524 Compare October 10, 2023 12:42

camsim99 approved these changes Oct 10, 2023

View reviewed changes

bleroux added 7 commits October 11, 2023 08:51

[Android] Add support for text processing actions

826ff12

Add documentation and lazy loading for the queryIntentActivities call

139e2bb

Add tests and more documentation

f80ef6f

Add queries section to the engine Android manifest file

fdc3deb

Update comments

77196a0

Use activity name as text action ID

55ee3eb

Move resolveInfosById instantiations to cacheResolveInfos

554c18f

bleroux force-pushed the android_process_text branch from 641c524 to 554c18f Compare October 11, 2023 07:58

bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 11, 2023

auto-submit bot merged commit d93fe23 into flutter:main Oct 11, 2023
25 checks passed

engine-flutter-autoroll mentioned this pull request Oct 11, 2023

Roll Flutter Engine from cb0dabd3b33c to d93fe2367d21 (2 revisions) flutter/flutter#136346

Merged

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

d93fe2367 [Android] Add support for text processing actions (flutter/…

ca8ebbb

…engine#44579)

gmackall pushed a commit to gmackall/engine that referenced this pull request Oct 11, 2023

Revert "[Android] Add support for text processing actions (flutter#44579

df7e128

)" This reverts commit d93fe23.

bleroux deleted the android_process_text branch October 12, 2023 06:51

bleroux mentioned this pull request Oct 12, 2023

Reland - [Android] Add support for text processing actions #46817

Merged

This was referenced Oct 24, 2023

Add ProcessTextService flutter/flutter#137145

Merged

[flutter_tools] - Add queries section to Android manifest file flutter/flutter#137207

Merged

HansMuller mentioned this pull request Nov 17, 2023

Flutter Framework Team: Notable Commits flutter/flutter#121415

Open

bleroux mentioned this pull request Dec 1, 2023

[Android] Respect custom system-wide text selection toolbar buttons flutter/flutter#139361

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Android] Add support for text processing actions #44579

[Android] Add support for text processing actions #44579

bleroux commented Aug 10, 2023 •

edited

justinmc left a comment

justinmc Aug 14, 2023

bleroux Aug 16, 2023

camsim99 left a comment

camsim99 Aug 14, 2023

bleroux Aug 16, 2023

bleroux Oct 4, 2023

camsim99 left a comment

camsim99 Oct 10, 2023

bleroux Oct 11, 2023

camsim99 Oct 10, 2023

[Android] Add support for text processing actions #44579

[Android] Add support for text processing actions #44579

Conversation

bleroux commented Aug 10, 2023 • edited

Description

Related Issue

Tests

justinmc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camsim99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camsim99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bleroux commented Aug 10, 2023 •

edited