Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EditText.getText() isn't bound #9192

Open
jonpryor opened this issue Aug 12, 2024 · 4 comments
Open

EditText.getText() isn't bound #9192

jonpryor opened this issue Aug 12, 2024 · 4 comments
Assignees
Labels
Area: Mono.Android Issues with the Android API binding (Mono.Android.dll).

Comments

@jonpryor
Copy link
Member

jonpryor commented Aug 12, 2024

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() returns CharSequence, while 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 jonpryor added Area: Mono.Android Issues with the Android API binding (Mono.Android.dll). needs-triage Issues that need to be assigned. labels Aug 12, 2024
@jonpryor
Copy link
Member Author

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
Copy link
Member Author

Apropos of nothing is the documentation for 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 35f41dc 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:

if (!JniEnvironment.Types.IsAssignableFrom (handleClass, typeClass)) {

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
Copy link
Member Author

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
Copy link
Member Author

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();

@jpobst jpobst removed the needs-triage Issues that need to be assigned. label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Mono.Android Issues with the Android API binding (Mono.Android.dll).
Projects
None yet
Development

No branches or pull requests

2 participants