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

Android Migrate off mCursorDrawableRes (setting cursor color on TextView) as throws error in api 29 and higher #6240

Closed
mitchcapper opened this issue Jun 13, 2021 · 4 comments · Fixed by #6941
Labels
difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools

Comments

@mitchcapper
Copy link

mitchcapper commented Jun 13, 2021

Current behavior

an exception thrown on access:
Accessing hidden field Landroid/widget/TextView;->mCursorDrawableRes:I (greylist-max-p, reflection, denied) **Java.Lang.NoSuchFieldException:** 'No field mCursorDrawableRes in class Landroid/widget/TextView; (declaration of 'android.widget.TextView' appears in /system/framework/framework.jar!classes3.dex)'

Summary

So

_cursorDrawableResField = textViewClass.GetDeclaredField("mCursorDrawableRes");
in EditTextCursorColorChanger accesses the mCursorDrawableRes field which is not supported. It was graylisted < 29 and per 29 it is blacklisted: https://developer.android.com/about/versions/10/non-sdk-q

This is referenced in #5491 however the PR does not remove the access to this field rather more error handling.

Officially I think the recommendation is: https://developer.android.com/reference/android/widget/TextView#setTextCursorDrawable(int) it was only added in v29 however I understand that is now the minimum version for uno anyway so could probably just swap over to using it exclusively.

@mitchcapper mitchcapper added difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification labels Jun 13, 2021
@jeromelaban jeromelaban added this to the 3.9 milestone Jun 14, 2021
@jeromelaban
Copy link
Member

Thanks for the report!

Curiously, this is not something that we've seen recently in apps running even on recent devices. Is this preventing the app from running, or you've only seen this message in the logs?

@mitchcapper
Copy link
Author

it does not prevent it running, it throws a hard exception in visual studio when debugging and I would assume the code doesn't work given the fact it is blacklisted now that uno targets 29.

@mitchcapper
Copy link
Author

Sorry I misunderstood the android sdk targetting vs minimum sdk version (or really min platform). As uno supports back to Android 5 the existing api switch would need expanding along with some re-work:

if ((int)Build.VERSION.SdkInt < 28) // 28 means BuildVersionCodes.P
{
_cursorDrawableField = _editorField.Get(editText)?.Class.GetDeclaredField("mCursorDrawable");
}
else
{
// set differently in Android P (API 28) and higher
_cursorDrawableField = _editorField.Get(editText)?.Class.GetDeclaredField("mDrawableForCursor");
}

Adding one for 29 and higher using the new SetTextCursorDrawable. The main thing would be removing the initial call to get the old mCursorDrawableRes unless version < 29 to avoid the error.

@jeromelaban
Copy link
Member

Thanks for the investigations! Indeed this was probably done prior to API 29, where there was no replacement for this API.

@jeromelaban jeromelaban added difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. triage/untriaged Indicates an issue requires triaging or verification labels Jun 15, 2021
@jeromelaban jeromelaban removed this from the 3.9 milestone Aug 11, 2021
Hudney added a commit to Hudney/uno that referenced this issue Aug 27, 2021
teras pushed a commit to Hudney/uno that referenced this issue Aug 31, 2021
teras pushed a commit to Hudney/uno that referenced this issue Sep 2, 2021
Hudney added a commit to Hudney/uno that referenced this issue Sep 2, 2021
@mergify mergify bot closed this as completed in #6941 Sep 8, 2021
MartinZikmund pushed a commit to MartinZikmund/Uno that referenced this issue Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/medium 🤔 Categorizes an issue for which the difficulty level is reachable with a good understanding of WinUI kind/bug Something isn't working platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform project/core-tools 🛠️ Categorizes an issue or PR as relevant to core and tools
Projects
None yet
2 participants