dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.93k stars 531 forks source link

EditText.getText() isn't bound #9192

Open jonpryor opened 3 months ago

jonpryor commented 3 months ago

Android framework version

net8.0-android, net9.0-android

Affected platform version

.NET 8.0

Description

Context: https://discord.com/channels/732297728826277939/732297837953679412/1272467705534091348 Context: https://stackoverflow.com/questions/26365808/edittext-settext-changes-the-keyboard-type-to-default-from-123-to-abc

Background

A customer came onto Discord asking:

When calling EditText.setText() on android, it resets the keyboard. This is super annoying if you are processing inputs and the user changes from ABC view to symbols view, the keyboard jumps 'back' to ABC view. … The 'workaround' natively is to not call SetText and instead call getText().Clear() and then getText().Append(blah)

The problem is, that's not currently possible, because there is no Text or TextFormatted property that has a .Clear() method. The TextView.TextFormatted property returns ICharSequence, which has no Clear() method.

What's Going On?

What's going on is our favorite, covariant return types: [TextView.getText()](https://developer.android.com/reference/android/widget/TextView#getText()) returns CharSequence, while [EditText.getText()](https://developer.android.com/reference/android/widget/EditText#getText()) returns the Editable interface. However, C# covariant return types only works for classes, not interfaces, and regardless we don't re-bind the Text property on EditText.

Proposal

Use src/Mono.Android/metadata to bind the EditText.getText() method as the EditText.TextEditable property. That would allow the original customer to call myEditText.TextEditable.Clear(), which is not currently possible.

jonpryor commented 3 months ago

One issue is that src/Mono.Android/Profiles/api-35.xml does not contain a getText() method which returns Editable:

% grep 'getText.*Editable' src/Mono.Android/Profiles/api-35.xml
# no match

:shocked-pikachu-face:

I then remember this comment from java-interop#1239, where I recreated the API.xml for android-35/android.jar, and that does contain a getText() method returning Editable (can't use grep as default API.xml output is multi-line):

% dotnet bin/Debug-net8.0/class-parse.dll ~/android-toolchain/sdk/platforms/android-35/android.jar > android-35.xml
% xpath -e '//class[method[@name="getText" and contains(@return, "Editable")]]/@jni-signature' android-35.xml
Found 1 nodes in android-35.xml:
-- NODE --
 jni-signature="Landroid/widget/EditText;"

For starters, this means this isn't a simple metadata fix, as metadata requires that the method already be within api-35.xml, which isn't the case.

Secondly, this re-ups my query elsewhere about how we're generating src/Mono.Android/Profiles/api-35.xml, and it's missing so many members I would otherwise expect, such as the bridge="true" methods.

jonpryor commented 3 months ago

Apropos of nothing is the documentation for [EditText.getText()](https://developer.android.com/reference/android/widget/EditText#getText()):

If setText(java.lang.CharSequence) was called with an argument of BufferType.SPANNABLE or BufferType.EDITABLE, you can cast the return value from this method to Spannable or Editable, respectively.

There are two "minor" problems with that comment:

  1. getText() returns Editable, but Spannable *does not implement Editable. (Rather, Editable implements Scannable.)
  2. Commit 35f41dcc7cf5eb65dc12d7c0a3421e67c2bdb6d6 updates Java.Lang.Object.GetObject<T>() to verify that value returned actually supports the type T.

Consider the hypothetical binding for a EditText.TextEditable property:

/*  1 */    partial class EditText {
/*  2 */        public unsafe Android.Text.IEditable? TextEditable {
/*  3 */            [Register ("getText", "()Landroid/text/Editable;", "GetGetTextHandler")]
/*  4 */            get {
/*  5 */                const string __id = "getText.()Landroid/text/Editable;";
/*  6 */                try {
/*  7 */                    var __rm = _members.InstanceMethods.InvokeVirtualObjectMethod (__id, this, null);
/*  8 */                    return global::Java.Lang.Object.GetObject<Android.Text.IEditable> (__rm.Handle, JniHandleOwnership.TransferLocalRef);
/*  9 */                } finally {
/* 10 */                }
/* 11 */            }
/* 12 */        }
/* 13 */    }

Line 8 may throw if the value returned is a Scannable that doesn't implement Editable, because of: https://github.com/dotnet/android/blob/d101851ea666a0d820e3a4e4e611600f56f2a944/src/Mono.Android/Java.Interop/TypeManager.cs#L338

It might be fine! It might not! Which doesn't make me feel good.

Fortunately the workaround is straightforward: use TextView.TextFormatted instead:

var scannable = edittext.TextFormatted.JavaCast<ISpannable>();
jonpryor commented 3 months ago

Because of "missing methods", there is now an implicit audit of src/Mono.Android/Profiles/api-35.xml to double-check which methods are being removed, and why, and see if any such methods should be re-added and bound. Along with a double-check of interface-based covariant return types: how many such methods are there? If there aren't too many, we might want to "special-case" them as proposed here?

jonpryor commented 3 months ago

Workaround (?) to invoke Editable methods on the return value of the EditText.TextFormatted property: use .JavaCast<T>()!

var editable = editText.TextFormatted.JavaCast<Android.Text.IEditable>();
editable.Clear();
editable.Append(…);