[Android] 28+ Make non-visible pickers work again

[jfversluis]

Description of Change

Checks if the TextView control is clickable, if not, takes a different route but still shows the picker dialog.

As a bonus I added the fix for #7311. The back button will not open the Picker again when it is still focused.

Issues Resolved

• fixes [Android] Calling Focus on all Pickers running an API 28 devices no longer opens Picker #5159
• fixes [Bug] [Android] Error back hardware button with Picker #7311

API Changes

None

Platforms Affected

• Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

PR Checklist

• Has automated tests
• Rebased on top of the target branch at time of PR
• Changes adhere to coding standard

[PureWeen]

Looks like android is failing this one
Xamarin.Forms.Controls.Issues.Issue530.Issue530TestsLoadAsync_lg_nexus_5-4_4_4

iOS
OpeningPickerPressingBackButtonTwiceShouldNotOpenPickerAgain_apple_iphone_xs

[jfversluis]

Moved the iOS one behind the __ANDROID__ flag, not needed on iOS.

For the other failing test, not sure what the cause might be. I've added a little code to hide the picker dialog again after the test is done. Don't see how that should be the problem, but let's see.

It seems the app crashes for some reason after the last "picker test" and before the next test which is the failing one you pointed out. It's way too coincidental that this happens now, so my tests are probably the cause, but not sure why.

[PureWeen]

It seems the app crashes for some reason after the last "picker test" and before the next test which is the failing one you pointed out.

Any unhandled exceptions in the device logs?

[jfversluis]

Any unhandled exceptions in the device logs?

Besides some exceptions (916 times) that do not seem to have to do anything with this, I do see this one:

I/MonoDroid(23390): System.NotSupportedException: Unable to activate instance of type Xamarin.Forms.Platform.Android.TimePickerRenderer from native handle 0xe9500025 (key_handle 0x42b2bbd8). ---> System.MissingMethodException: No constructor found for Xamarin.Forms.Platform.Android.TimePickerRenderer::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership) ---> Java.Interop.JavaLocationException: Exception of type 'Java.Interop.JavaLocationException' was thrown.
I/MonoDroid(23390): --- End of inner exception stack trace ---
I/MonoDroid(23390): at Java.Interop.TypeManager.CreateProxy (System.Type type, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) [0x00055] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType) [0x00116] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): --- End of inner exception stack trace ---
I/MonoDroid(23390): at Java.Interop.TypeManager.CreateInstance (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type targetType) [0x00182] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Java.Lang.Object.GetObject (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer, System.Type type) [0x000c1] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Java.Lang.Object._GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) [0x00017] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Java.Lang.Object.GetObject[T] (System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) [0x00000] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Java.Lang.Object.GetObject[T] (System.IntPtr jnienv, System.IntPtr handle, Android.Runtime.JniHandleOwnership transfer) [0x00006] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at Android.App.TimePickerDialog+IOnTimeSetListenerInvoker.n_OnTimeSet_Landroid_widget_TimePicker_II (System.IntPtr jnienv, System.IntPtr native__this, System.IntPtr native_view, System.Int32 hourOfDay, System.Int32 minute) [0x00000] in <1ee35b7607e5467fa07b155162b2c8be>:0
I/MonoDroid(23390): at (wrapper dynamic-method) System.Object.85(intptr,intptr,intptr,int,int)

But not sure what that is supposed to mean. And clearly this doesn't happen all the time.

Seeing some other issues with similar error messages (#2444, #6550) it might have something to do with some value being null?

With the objects named in the exception I traced it back to here in TimePickerRenderer.cs on Android.

void TimePickerDialog.IOnTimeSetListener.OnTimeSet(ATimePicker view, int hourOfDay, int minute)
{
	ElementController.SetValueFromRenderer(TimePicker.TimeProperty, new TimeSpan(hourOfDay, minute, 0));
	ElementController.SetValueFromRenderer(VisualElement.IsFocusedPropertyKey, false);

	if (Forms.IsLollipopOrNewer)
		_dialog.CancelEvent -= OnCancelButtonClicked;

	_dialog = null;
}

I will try to reproduce it and from there see to fix it. The only other thing I can think of is to just put this full of null checks. @PureWeen any thoughts?

[PureWeen]

@jfversluis

System.NotSupportedException: Unable to activate instance of type

Means that a Renderer or View has been disposed but then some action/event is trying to reuse that View or Renderer.

For example

• View Exists
• Event wired up
• View disposed
• Event fires
• Mono sees the View has been disposed because the HandleId is zero
• Mono tries to just recreate the view itself so it can pass the event to someone
• Boom System.NotSupportedException: Unable to activate instance of type because we don't supply the constructors Mono need to instantiate these types itself

So it sounds like something with your changes isn't unwiring an event. This Listenerer probably needs to be removed inside dispose?

void TimePickerDialog.IOnTimeSetListener.OnTimeSet

[kingces95]

Just to be clear, Android documentation states that a non-visible view cannot be focused and when a XF user calls Focus() on such a view (in 9.0) it will not focus the view.

With this change we will pop up the DateTime picker dialog before the underlying OS has decided if the DateTime EditText will gain focus. And we're gonna push that behavior across all the picker views, not just revert the DateTime behavior?

[samhouts]

@kingces95 We do have problems with Focus on all platforms that must be addressed. We are not following guidelines, and unexpected behavior is occurring for most controls. It'll be a larger effort, and I expect it will introduce breaking changes that we'll have to mitigate somehow.

As it turns out, the dialogs popping up on Focus was previously working on all Picker types, and our customers took a dependency on that behavior. So, we need to restore that behavior to help our customers get back into a working state.

I agree with the strategy that you and I have discussed. We should fix Focus on all platforms so that it doesn't actually do anything for invisible views (or views that have no height/width), and instead allow the dialogs to be opened with a new API call. Again, since this is a breaking change, it will need to be communicated well and may need to wait until the next major version.

[kingces95]

So be it.

[jfversluis]

@PureWeen I think the test run for the latest change is here: https://appcenter.ms/orgs/xtc-Xamarin-Forms/apps/AndroidControlGallery-1/test/series/pull-7289/runs/543e63f2-0253-4194-8c76-ad1568555b81

And seems all green?

[PureWeen]

Can we dispose of this _dialog as well?

https://github.com/xamarin/Xamarin.Forms/pull/7289/files#diff-5a9dfdd3c76f522505c6dd5b89180802R104

_dialog?.Dispose()
_dialog = null