xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.63k stars 1.87k forks source link

[Bug] Nested scroll causes Gesture Recognizer to intercept scroll events #7727

Open craigwi opened 5 years ago

craigwi commented 5 years ago

Description

A simple horizontal scroll view nested in a vertical scroll view results in modal behavior in the latest 4.1 and 3.6 (and maybe other versions). In latest version 4.2.0.815419, this example crashes and VS times out trying to get the exception info.

Steps to Reproduce

  1. paste the attached xaml into mainpage.xaml in an otherwise blank Android Xamarin Forms app.
  2. in 4.2, this will crash when run
  3. in 4.1 and 3.6, the modal behavior is as follows: if one taps and drags the light gray rectangle at the bottom up and down, all is good. if one taps and drags the black/green/etc. rectangles at the top left and right, one cannot tap and drag the light gray rectangle at the bottom up and down. The white area to the left or right of the light gray rectangle can be used to scroll up and down. If one tabs the top rectangles (black/green/etc) and moves them vertically, this restores the vertical scrolling of the whole screen including tapping on the light gray rectangle.

MainPage.xaml.txt

  1. removing the TapGestureRecognizer eliminates the incorrect modal behavior for 4.1 and 3.6. Doing this does not eliminate the crash in 4.2.

Basic Information

craigwi commented 5 years ago

A workaround for 3.6 or 4.1 would be nice.

craigwi commented 5 years ago

BTW, I verified the modal scroll behavior using the official Xamarin.Forms repo by changing PagesGallery.app.xaml.cs to create the MainPage attached here. I have not been able to reproduce the crash in 4.2.

PureWeen commented 5 years ago

@craigwi I'm not seeing the crash

Can you attach a repro with the 4.2 crash so I can see that one?

craigwi commented 5 years ago

I'll create a complete project showing the crash.

In the mean time, what would be a workaround for the modal behavior?

PureWeen commented 5 years ago

I'm not quite sure what you mean by Modal behavior

As far as the GestureRecognizer and the scroll.

Right now the behavior of Gestures and scrolls aren't very smooth and we have a number of enhancements to make those interactions better.

I'm guessing once you initiate a nested scroll on Android the orchestration of the underlying events changes.

I'm hoping PRs like this will help https://github.com/xamarin/Xamarin.Forms/pull/7324

So this one will most likely get logged as an enhancement and then if there is a crash then we will fix that quickly

craigwi commented 5 years ago

The PR looks like it will help. I did get a repro with 4.2.0.815419 and captured the exception:

Unhandled Exception:

System.TypeLoadException: Could not load type of field 'Xamarin.Forms.Platform.Android.RendererPool:_freeRenderers' (0) due to: Could not resolve type with token 01000273 from typeref (expected class 'System.Collections.Generic.Stack1' in assembly 'mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e') assembly:mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e type:System.Collections.Generic.Stack1 member:(null) occurred

The exception does not occur in the next release 4.2.0.848062. So maybe this has been fixed already.

When I get to a different machine I will package up the project and post it.

As for the model behavior, when I post the project with the crash, you can also use it to experience the model described above.

PureWeen commented 5 years ago

aw yes @craigwi that's actually a different exception related to using VS 2017 with 4.2.0.815419

We fixed that in the latest set of releases

craigwi commented 5 years ago

Using the attached project you can observe the modal behavior as follows:

  1. compile and run app; I have a Galaxy s7 running Android 8
  2. tap and drag vertically light gray rectangle; this works
  3. tap and drag to the left the black and/or green rectangle at top and this works too
  4. after doing step 3, the step 2 does not work any more; i.e., on cannot drag the light gray rectangle vertically
  5. tap and drag VERTICALLY the black or green rectangle at the top; this work AND now the light gray rectangle can be dragged vertically again.

AndroidScrollIssue.zip

PureWeen commented 5 years ago

@craigwi Alright I see the issue you are referring to and I'm hoping the issues I linked in will let you orchestrate these but my guess is what we're seeing here is the relationship of how NestedSCrollViews work in Android

At about 3.6 we switched the ScrollViewRenderer to use NestedScrollView instead of just ScrollView

NestedScrollView operates by handing off who's in charge of scrolling actions so I'm guessing when you scroll the black one it causes the way the events bubble to change. I'm not sure of a workaround off hand.

The paths I would take to workaround would either be

craigwi commented 5 years ago

While debugging the modified PageGallery example which exhibits the modal behavior (mentioned above), I noticed that both ScrollViewRenderer (which inherits from NestedScrollView) and AHorizontalScrollView (which inherits from HorizontalScrollView) are both involved here. The comments in OnTouchEvent are similarly, but the logic is different enough to warrant further investigation. Why are they different?

craigwi commented 5 years ago

In another debugging session, I stopped on all methods named OnTouchEvent. There is quite a bg difference between the two cases involving the lower gray rectangle -- when it moves and when it doesn't.

Do you have any additional pointers on a workaround?

luccasclezar commented 4 years ago

I managed to workaround this issue, but it could break something. I tested with some layouts and didn't notice any kind of wrong behaviours though.

public class ScrollViewRenderer : Xamarin.Forms.Platform.Android.ScrollViewRenderer
{
    public ScrollViewRenderer(Context context) : base(context) { }

    public override bool OnInterceptTouchEvent(MotionEvent ev)
    {
        OnTouchEvent(ev);
        return base.OnInterceptTouchEvent(ev);
    }
}

That's it. After overriding OnInterceptTouchEvent and always calling OnTouchEvent inside it, it works flawlessly. I don't think doing this is advisable, but it's better than not scrolling.

craigwi commented 4 years ago

Thanks @luccasclezar. I'll give that a try and report back on my experience.

craigwi commented 4 years ago

I tried this out and so far so good. After inspecting the Xamarin code around this, I went with the following override:

    public override bool OnInterceptTouchEvent(MotionEvent ev)
    {
        bool intercept = base.OnInterceptTouchEvent(ev);
        if (!intercept)
            OnTouchEvent(ev);

        return intercept;
    }

On the premise that if base.OnInterceptTouchEvent() returns true, we don't want to call the OnTouchEvent() twice.

craigwi commented 4 years ago

Any update on this?