Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add image keyboard support on Android #27763

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
54a9657
Android GIF/Image Insertion
zlshames Jul 11, 2021
5f0582e
Remove debug logs
tneotia Jul 28, 2021
d2ab746
Add test for commitContent
tneotia Jul 29, 2021
983c07c
[flutter_releases] Flutter beta 2.5.0-5.3.pre Engine Cherrypicks (#28…
christopherfujino Sep 2, 2021
89ca066
[flutter_releases] Flutter Stable 2.2.3 Engine Cherrypicks Pt 2 (#27087)
christopherfujino Jun 30, 2021
b4c40dd
Full GIF/Image Insertion
zlshames Jul 11, 2021
17e900e
Update commitContent slightly
tneotia Nov 6, 2021
e414b03
Fix build
tneotia Nov 10, 2021
805f6f8
Fix build pt2
tneotia Nov 10, 2021
9123435
Fix build pt3
tneotia Nov 10, 2021
26d13e0
remove undefined function to test
ryanawhelan Jan 9, 2022
58c5299
update format to match dart standard for Java
ryanawhelan Jan 9, 2022
9d0eb17
Merge pull request #2 from ryanawhelan/fix_build
zlshames Jan 10, 2022
bbb3286
Adds contentCommitMimeTypes support
zlshames Jan 15, 2022
fa50825
Formatting fixes
zlshames Jan 15, 2022
0c1dbbd
Merge branch 'master' of github.com:BlueBubblesApp/engine into zach/c…
zlshames Jan 15, 2022
a933522
Merge pull request #3 from BlueBubblesApp/zach/commit-mimes
zlshames Jan 15, 2022
3cead60
2.8.1 content commit merges & comment fixes
zlshames Jan 28, 2022
4f735ff
Java refactor
zlshames Jan 28, 2022
76f5faf
Merge pull request #5 from BlueBubblesApp/zach/gif-insertion-pr
zlshames Jan 28, 2022
5d6d517
Adds additional parameter to TextInputChannel.Configuration
zlshames Jan 28, 2022
6004596
Merge pull request #6 from BlueBubblesApp/zach/gif-insertion-pr
zlshames Jan 28, 2022
0224531
Fix build failures
tneotia Apr 13, 2022
f1d7ab0
Merge remote-tracking branch 'flutter/master' into tanay/gif-insertio…
tneotia Apr 13, 2022
41fe8f0
Remove duplicate function
tneotia Apr 13, 2022
4105ebc
Fix missing imports
tneotia Apr 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Android GIF/Image Insertion
  • Loading branch information
zlshames authored and tneotia committed Jul 28, 2021
commit 54a9657e3d68e50f8390cdd92c74497afd99ea06
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,13 @@ public void previous(int inputClientId) {
"TextInputClient.performAction", Arrays.asList(inputClientId, "TextInputAction.previous"));
}

/** Instructs flutter to commit content back to the text channel */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period at the end of the sentence here.

public void commitContent(int inputClientId, Map<String, Object> content) {
Log.v(TAG, "Sending 'commitContent' message.");
channel.invokeMethod(
"TextInputClient.performAction", Arrays.asList(inputClientId, "TextInputAction.commitContent", content));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone have an opinion about the use of performAction for this type of thing? Makes sense to me but it would be nice to have a second opinion.

}

/** Instructs Flutter to execute an "unspecified" action. */
public void unspecifiedAction(int inputClientId) {
Log.v(TAG, "Sending 'unspecified' message.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

package io.flutter.plugin.editing;

import java.io.FileNotFoundException;
import java.io.ByteArrayOutputStream;
import java.io.InputStream;
import java.util.Map;
import java.util.HashMap;

import android.content.ClipData;
import android.content.ClipboardManager;
import android.content.Context;
Expand All @@ -23,6 +29,12 @@
import android.view.inputmethod.ExtractedText;
import android.view.inputmethod.ExtractedTextRequest;
import android.view.inputmethod.InputMethodManager;
import android.view.inputmethod.InputContentInfo;
import android.net.Uri;

import androidx.core.view.inputmethod.InputConnectionCompat;
import androidx.core.os.BuildCompat;

import io.flutter.Log;
import io.flutter.embedding.android.KeyboardManager;
import io.flutter.embedding.engine.FlutterJNI;
Expand Down Expand Up @@ -473,6 +485,79 @@ public boolean performEditorAction(int actionCode) {
return true;
}

@Override
public boolean commitContent(InputContentInfo inputContentInfo, int flags, Bundle opts) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good point was recently brought up by @matthew-carroll about how APIs like this should follow Flutter's declarative pattern instead of being imperative. So instead of telling the client to commit the content, informing the client that the user committed content (contentCommitted maybe?). Though, it seems that adjacent API methods already have an imperative pattern, so maybe now is not the time to change that.

Log.d("HackFlutterEngine", "Content Commit Invoked");

// Ensure permission is granted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period here.

if (BuildCompat.isAtLeastNMR1() && (flags & InputConnectionCompat.INPUT_CONTENT_GRANT_READ_URI_PERMISSION) != 0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildCompat.isAtLeastNMR1() is deprecated. You can use the equivalent: https://developer.android.com/reference/androidx/core/os/BuildCompat#isAtLeastNMR1()

try {
inputContentInfo.requestPermission();
Log.d("HackFlutterEngine", "Content Commit request permissions: PASS");
} catch (Exception e) {
Log.d("HackFlutterEngine", "Content Commit reqest permissions: FAIL");
return false;
}
}

if (inputContentInfo.getDescription().getMimeTypeCount() > 0) {
inputContentInfo.requestPermission();

final Uri uri = inputContentInfo.getContentUri();
final String mimeType = inputContentInfo.getDescription().getMimeType(0);
Log.d("HackFlutterEngine", "Content Commit received URI: " + uri + " (MIME: " + mimeType + ")");
Context context = mFlutterView.getContext();
Boolean retval = false;

try {
final InputStream is = context.getContentResolver().openInputStream(uri);
final byte[] data = this.readStreamFully(is, 64 * 1024);
Log.d("HackFlutterEngine", "Content Commit data length: " + data.length);

final Map<String, Object> obj = new HashMap<>();
obj.put("mimeType", mimeType);
obj.put("data", data);
obj.put("uri", uri != null ? uri.toString() : null);

// Commit the content to the text input channel
textInputChannel.commitContent(mClient, obj);
retval = true;
} catch (FileNotFoundException ex) {
Log.d("HackFlutterEngine", "Content Commit load file: FAIL (Not Found)");
} catch (Exception ex) {
Log.d("HackFlutterEngine", "Content Commit load data: FAIL");
} finally {
inputContentInfo.releasePermission();
}

if (retval) {
Log.d("HackFlutterEngine", "Content Commit Result: PASS");
}

return retval;
}

// If it gets to this point, it failed
Log.d("HackFlutterEngine", "Content Commit Result: FAIL");
return false;
}

public byte[] readStreamFully(InputStream is, int blocksize) {
try {
ByteArrayOutputStream baos = new ByteArrayOutputStream();

byte[] buffer = new byte[blocksize];
while (true) {
int len = is.read(buffer);
if (len == -1) break;
baos.write(buffer, 0, len);
}
return baos.toByteArray();
} catch (Exception e) {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment. Please add a single statement in the try block, and specify a concrete exception.


return new byte[0];
}

// -------- Start: ListenableEditingState watcher implementation -------
@Override
public void didChangeEditingState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import androidx.core.view.inputmethod.EditorInfoCompat;
import io.flutter.Log;
import io.flutter.embedding.android.KeyboardManager;
import io.flutter.embedding.engine.systemchannels.TextInputChannel;
Expand Down Expand Up @@ -330,6 +331,17 @@ public InputConnection createInputConnection(
}
outAttrs.imeOptions |= enterAction;

String[] imgTypeString = new String[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Should this be declared outside the method as a constant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This nit may be obsolete actually, see my comment below.

"image/png",
"image/bmp",
"image/jpg",
"image/tiff",
"image/gif",
"image/jpeg",
"image/webp"
};
EditorInfoCompat.setContentMimeTypes(outAttrs, imgTypeString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that on Android, if try I use Gboard to insert a gif into a field that doesn't support images, it gives me a message saying, "Google does not support image insertion here". Is that message based on this mime type list? If so, maybe we need a way for Flutter developers to specify that a field does not support images.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tried this on a Flutter app (before your changes) and I get the same message. We might want to include a way to support this original behavior. The first thing that comes to mind is adding a contentMimeTypes parameter to EditableText and passing it through to here, would that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know that message is only based on whether the contentCommit hook is overriden in the InputClient. It could be based off the mimeType list, in which case I agree finding a way to pass those to there would be a good idea.

Copy link
Contributor Author

@tneotia tneotia Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinmc so apparently the mime types do nothing in @zlshames's testing... how should we approach this then? Should we just remove this code entirely?

I don't really know why it doesn't affect the content insertion, it seems to me that we have everything set up correctly. If we indeed haven't made any mistakes, imho the best option would be to let the dev handle their desired mimeTypes inside onContentCommit and choose how to notify their users that the content they are inserting is unsupported. Thinking on the iOS side as well, I am not 100% sure you can control the mimeType of items being pasted into an editable text field - so it may be better to do it this way for the sake of consistency.

We could make the default behavior of onContentCommit show a snackbar saying "Image insertion isn't supported here" to replicate the original behavior in case devs don't want to support image insertion at all.


InputConnectionAdaptor connection =
new InputConnectionAdaptor(
view, inputTarget.id, textInputChannel, keyboardManager, mEditable, outAttrs);
Expand Down