rotorgames / Rg.Plugins.Popup

Xamarin Forms popup plugin
MIT License
1.15k stars 337 forks source link

AndroidTalkbackAccessibilityWorkaround set to true causes System.ArgumentOutOfRangeException #681

Closed IeuanWalker closed 3 years ago

IeuanWalker commented 3 years ago

🐛 Bug Report

Upgraded to NuGet version 2.0.0.12 added AndroidTalkbackAccessibilityWorkaround="True" to the popup page, when the popup is pushed i get System.ArgumentOutOfRangeException exception.

Expected behaviour

No crash

Reproduction steps

Pop page is quite basic -

XAML -

<?xml version="1.0" encoding="utf-8" ?>
<pages:PopupPage x:Class="CardiffMobileApp.Pages.General.MessagePopupPage"
                 xmlns="http://xamarin.com/schemas/2014/forms"
                 xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
                 xmlns:buttons="clr-namespace:CardiffMobileApp.Controls.Buttons"
                 xmlns:converter="clr-namespace:CardiffMobileApp.Converter;assembly=CardiffMobileApp"
                 xmlns:pages="clr-namespace:Rg.Plugins.Popup.Pages;assembly=Rg.Plugins.Popup"
                 AndroidTalkbackAccessibilityWorkaround="True">
    <pages:PopupPage.Resources>
        <ResourceDictionary>
            <converter:LinkPhoneNumberLabelConverter x:Key="LinkPhoneNumberLabelConverter" />
        </ResourceDictionary>
    </pages:PopupPage.Resources>
    <ScrollView>
        <Frame Margin="15"
               Padding="0"
               BackgroundColor="{DynamicResource PrimaryBackgroundColour}"
               CornerRadius="6"
               HorizontalOptions="Center"
               IsClippedToBounds="True"
               VerticalOptions="Center">
            <!-- IMPORTANT: StackLayout needed to stretch popup -->
            <StackLayout HorizontalOptions="FillAndExpand" VerticalOptions="StartAndExpand">
                <Grid Margin="15,15,15,5"
                      ColumnDefinitions="*,40"
                      RowDefinitions="auto,auto,auto">
                    <!-- Title -->
                    <Label x:Name="LblTitle"
                           Grid.Row="0"
                           Grid.Column="0"
                           HorizontalOptions="Start"
                           HorizontalTextAlignment="Start"
                           Style="{DynamicResource TextSection}"
                           Text="{Binding Title}"
                           VerticalOptions="CenterAndExpand"
                           VerticalTextAlignment="Center" />

                    <!-- Close Button -->
                    <buttons:Close Grid.Row="0"
                                   Grid.Column="1"
                                   Margin="0"
                                   Clicked="BtnClose_OnClicked"
                                   VerticalOptions="Start" />

                    <!-- Message -->
                    <Label Grid.Row="1"
                           Grid.Column="0"
                           Grid.ColumnSpan="2"
                           FormattedText="{Binding Message, Converter={StaticResource LinkPhoneNumberLabelConverter}}"
                           HorizontalOptions="Center"
                           HorizontalTextAlignment="Start"
                           Style="{DynamicResource TextBody}"
                           VerticalOptions="Start" />

                    <!-- Button -->
                    <buttons:Full Grid.Row="2"
                                  Grid.Column="0"
                                  Grid.ColumnSpan="2"
                                  Margin="-16,10,-16,-5"
                                  Clicked="BtnPopup_OnClick"
                                  IsVisible="{Binding Button, Converter={StaticResource HasDataConverter}}"
                                  Text="{Binding Button}" />
                </Grid>
            </StackLayout>
        </Frame>
    </ScrollView>
</pages:PopupPage>

C# -

using Accessibility;
using Rg.Plugins.Popup.Pages;
using Rg.Plugins.Popup.Services;
using System;
using Xamarin.Forms;
using Xamarin.Forms.Xaml;

namespace CardiffMobileApp.Pages.General
{
    [XamlCompilation(XamlCompilationOptions.Compile)]
    public partial class MessagePopupPage : PopupPage
    {
        // event callback
        public event EventHandler CallbackEvent;

        public string Button { get; set; }
        public new string Title { get; set; }
        public string Message { get; set; }

        private readonly IAccessibilityService _accessibilityService;

        public MessagePopupPage(string title, string message, string button = null, EventHandler callbackEvent = null)
        {
            Title = title;
            Message = message;
            Button = button;
            CallbackEvent = callbackEvent;

            InitializeComponent();
            _accessibilityService = DependencyService.Get<IAccessibilityService>();

            BindingContext = this;

        }

        protected override void OnAppearingAnimationEnd()
        {
            base.OnAppearingAnimationEnd();
            _accessibilityService.SetFocus(LblTitle);
        }

        private async void BtnClose_OnClicked(object sender, EventArgs e)
        {
            await PopupNavigation.Instance.PopAllAsync();
        }

        private async void BtnPopup_OnClick(object sender, EventArgs e)
        {
            CallbackEvent?.Invoke(this, e);
            await PopupNavigation.Instance.PopAllAsync();
        }
    }
}

Configuration

Version: 2.0.0.12

Platform:

LuckyDucko commented 3 years ago

@IeuanWalker Let me investigate this, I thought it went all to smoothly, the workaround.

I enabled it on my private app just then, and it seems to have worked just fine, Could you provide the entire exception callback? it may have something to do with _accessibiltyservice.

(Best of all would be to create a project that shows the error I can download and tinker with directly.)

IeuanWalker commented 3 years ago

@LuckyDucko Commenting out the _accessibiltyservice (both the DependencyService.Get and SetFocus) still crashes

Here is the output -

[InputMethodManager] startInputInner - mService.startInputOrWindowGainedFocus
[InputTransport] Input channel constructed: fd=111
[InputTransport] Input channel destroyed: fd=101
[ViewRootImpl@327c771[MainActivity]] MSG_WINDOW_FOCUS_CHANGED 0 1
[InputMethodManager] prepareNavigationBarInfo() DecorView@1948646[MainActivity]
[InputMethodManager] getNavigationBarColor() -855310
[diffGov.Androi] Explicit concurrent copying GC freed 23573(851KB) AllocSpace objects, 0(0B) LOS objects, 57% free, 4MB/10MB, paused 75us total 35.162ms
[libEGL] EGLNativeWindowType 0x7bad2d5010 disconnect failed
[OpenGLRenderer] eglDestroySurface = 0x7bde4d7d80, 0x7bad2d5000
[ViewRootImpl@69c4aeb[MainActivity]] dispatchDetachedFromWindow
[ViewRootImpl@69c4aeb[MainActivity]] Surface release. android.view.ViewRootImpl.doDie:7964 android.view.ViewRootImpl.die:7932 android.view.WindowManagerGlobal.removeViewLocked:497 android.view.WindowManagerGlobal.removeView:435 android.view.WindowManagerImpl.removeViewImmediate:124 android.app.Dialog.dismissDialog:518 android.app.Dialog.dismiss:501 mono.java.lang.RunnableImplementor.n_run:-2 
[InputTransport] Input channel destroyed: fd=107
[ViewRootImpl@327c771[MainActivity]] MSG_WINDOW_FOCUS_CHANGED 1 1
**System.ArgumentOutOfRangeException:** 'Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index'

[InputMethodManager] prepareNavigationBarInfo() DecorView@1948646[MainActivity]
[InputMethodManager] getNavigationBarColor() -855310
[InputMethodManager] prepareNavigationBarInfo() DecorView@1948646[MainActivity]
[InputMethodManager] getNavigationBarColor() -855310
[InputMethodManager] Starting input: tba=uk.gov.cardiff.CardiffGov.Android ic=null mNaviBarColor -855310 mIsGetNaviBarColorSuccess true , NavVisible : true , NavTrans : false
[InputMethodManager] startInputInner - Id : 0
[InputMethodManager] startInputInner - mService.startInputOrWindowGainedFocus
[InputTransport] Input channel constructed: fd=101
[InputTransport] Input channel destroyed: fd=111
[ViewRootImpl] sendUserActionEvent() returned.
Thread finished: <Thread Pool> #6
The thread 0x6 has exited with code 0 (0x0).
The thread 0xb has exited with code 0 (0x0).
Thread finished: <Thread Pool> #11
[AppCenterCrashes] Unhandled Exception from source=AndroidEnvironment
[AppCenterCrashes] System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
[AppCenterCrashes] Parameter name: index
[AppCenterCrashes]   at System.Collections.Generic.List`1[T].get_Item (System.Int32 index) [0x00009] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/Common/src/CoreLib/System/Collections/Generic/List.cs:161 
[AppCenterCrashes]   at Rg.Plugins.Popup.Droid.Impl.PopupPlatformDroid.<AddAsync>g__HandleAccessibilityWorkaround|9_0 (Rg.Plugins.Popup.Pages.PopupPage page) [0x00022] in Z:\Documents\OpenSource\Rg.Plugins.Popup\Rg.Plugins.Popup\Platforms\Android\Impl\PopupPlatformDroid.cs:60 
[AppCenterCrashes]   at Rg.Plugins.Popup.Droid.Impl.PopupPlatformDroid.AddAsync (Rg.Plugins.Popup.Pages.PopupPage page) [0x00006] in Z:\Documents\OpenSource\Rg.Plugins.Popup\Rg.Plugins.Popup\Platforms\Android\Impl\PopupPlatformDroid.cs:47 
[AppCenterCrashes]   at Rg.Plugins.Popup.Services.PopupNavigationImpl.AddAsync (Rg.Plugins.Popup.Pages.PopupPage page) [0x00000] in Z:\Documents\OpenSource\Rg.Plugins.Popup\Rg.Plugins.Popup\Services\PopupNavigationImpl.cs:177 
[AppCenterCrashes]   at Rg.Plugins.Popup.Services.PopupNavigationImpl+<>c__DisplayClass20_0.<PushAsync>b__0 () [0x00047] in Z:\Documents\OpenSource\Rg.Plugins.Popup\Rg.Plugins.Popup\Services\PopupNavigationImpl.cs:76 
[AppCenterCrashes]   at Rg.Plugins.Popup.Services.PopupNavigationImpl+<>c__DisplayClass28_0.<InvokeThreadSafe>b__0 () [0x00025] in Z:\Documents\OpenSource\Rg.Plugins.Popup\Rg.Plugins.Popup\Services\PopupNavigationImpl.cs:212 
[AppCenterCrashes]   at CardiffMobileApp.Pages.WasteManegement.HWRCBooking.MenuPage.OnAppearing () [0x00325] in D:\WS\Azure\App\CardiffMobileApp\CardiffMobileApp\Pages\WasteManegement\HWRCBooking\MenuPage.xaml.cs:67 
[AppCenterCrashes]   at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021 
[AppCenterCrashes]   at Android.App.SyncContext+<>c__DisplayClass2_0.<Post>b__0 () [0x00000] in /Users/builder/azdo/_work/2/s/xamarin-android/src/Mono.Android/Android.App/SyncContext.cs:36 
[AppCenterCrashes]   at Java.Lang.Thread+RunnableImplementor.Run () [0x00008] in /Users/builder/azdo/_work/2/s/xamarin-android/src/Mono.Android/Java.Lang/Thread.cs:36 
[AppCenterCrashes]   at Java.Lang.IRunnableInvoker.n_Run (System.IntPtr jnienv, System.IntPtr native__this) [0x00008] in /Users/builder/azdo/_work/2/s/xamarin-android/src/Mono.Android/obj/Release/monoandroid10/android-30/mcw/Java.Lang.IRunnable.cs:90 
[AppCenterCrashes]   at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.48(intptr,intptr)
[AppCenterCrashes] Saving uncaught exception.
[AppCenterCrashes] Saved JSON content for ingestion into /data/user/0/uk.gov.cardiff.CardiffGov.Android/files/error/1d57b2a7-921d-4ff9-8f3c-2afd1da4e6ee.json
[AppCenterCrashes] Saved raw wrapper exception data into /data/user/0/uk.gov.cardiff.CardiffGov.Android/files/error/1d57b2a7-921d-4ff9-8f3c-2afd1da4e6ee.dat
**System.ArgumentOutOfRangeException:** 'Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index'

And here is the Appcenter crash report - image

I'll try and get a repo project for you today.

thanks.

IeuanWalker commented 3 years ago

@LuckyDucko here is a slimmed-down version of our project that has the issue -

AppPopupCrashRepo.zip

LuckyDucko commented 3 years ago

@IeuanWalker I fixed this error, if you want to try it out pre-nuget release, download branch accessibilityupgrade585 and link it. (https://user-images.githubusercontent.com/17778102/114110334-a1f01f80-991a-11eb-96bc-5227f33c52da.mp4)

There is still an issue where a screen reader will attempt to read things in the background, I am trying to work out a way around this. One way is to make the background white, but we only want that when people are actually using the screenreader if it were to be a fix.

if you have any success in that area, let me know.

IeuanWalker commented 3 years ago

@LuckyDucko that's fixed the crashing issue.

But now there are 2 issues -

Here is a video of it -

https://user-images.githubusercontent.com/6544051/126136185-acfa3aa2-c451-4403-b04b-9de72c7787d0.mp4

LuckyDucko commented 3 years ago

Yeah, the underlining page title being selectable is an issue I am unsure how to fix.

in testing I made the actual popup page completely hide all its features for accessibility, and STILL that went off. If I knew how to hook into the accessibility screen reader to tell me what its reading, I could have a better chance. that ones annoying.

The second part is new. Let me sort it out.

Edit: Fixed the issue, give it another try and let me know what you think. Issue 1 still remains

IeuanWalker commented 3 years ago

Thanks @LuckyDucko thats fixed the main issue :)

If I have time ill look into the page title issues, will let you know if I find anytyhing

IeuanWalker commented 3 years ago

@LuckyDucko think I've found a way to fix the title issue.

Do you want me to create a PR once the fixes you have done are on the develop branch? (only done a quick test, will do more testing on it tomorrow, if I get a chance)

LuckyDucko commented 3 years ago

That would be great thanks leuan, the fixes I made are minor, so ill merge them across now

LuckyDucko commented 3 years ago

@IeuanWalker merged in, there was an interesting point made in #585

IeuanWalker commented 3 years ago

@LuckyDucko just wondering if you could do a release for the crashing issue? (no rush on the PR I submitted) 😃

LuckyDucko commented 3 years ago

ah, @martijn00 handles the releases for this repo, as he has access to the nuget

However, if you run your branch, and then reference it like I show in the video in #585 that should sort out the issue, and I can get Martijn to release your PR as the latest. I just havent found the time to do the testing that I have wanted to, might have to be done Thursday or Friday unfortunately. Sorry for the delays about it.