Video playback pauses when app toolbar is scrolled offscreen

mcomella commented 5 years ago

Steps to reproduce

Load a page with a video (I used buzzfeed videos or youtube)
Play the video without scrolling the app toolbar offscreen. The content will continue to play
Scroll down so the toolbar goes offscreen

Expected behavior

Video continues to play

Actual behavior

Video stops playing & video controls are unresponsive. Tapping the video does not hide or show video controls.

Here's a video.

Device information

Echo Show device (1st gen, 2nd gen, or 5): 5 (others untested)
Latest affected Firefox version: up/master 9f86dfefee5909eed42cae7f0d584d47976b7037 (will be v1.4 release-ish; this also occurs before we changed the zoom behavior)
Earliest affected Firefox version: ?

It might be valuable to do a quick regression test here: we haven't landed that many changes recently.

mcomella commented 4 years ago

I cannot reproduce on the FFES gen 2. I still haven't tested gen 1.

Bisection: v1.3: bad eafecb05a1723f31e7702cddfcc7e103965a09ef: bad e0f942cb86312bdabe7abf516651a3fd3c8b4100: bad e4da2a75ea95c90a3f794ac12aaed2bed4bb2435: bad 986826e0782b3df4babde905b4549feec438d28f: good 8952c0e11cc1073bcf7da628e8bdc515cc3d453b: good v1.2: good

This looks like a regression from #117 (which actually landed before the new device was out 😁). There were a lot of changes in that PR, including moving the whole homescreen into a fragment, so I'm not sure how feasible it is determine what caused the issue by looking at what changed.

mcomella commented 4 years ago

Disabling toolbar scroll fixes the issues (probably because the toolbar doesn't move offscreen) but this is info to help debugging: we wouldn't want to do this in practice.

I started to bisect the the PR into sub-commits: 7072bb4d: bad 55fd925b: bad 77150077: good

It looks like the regression was introduced in 55fd925b: with just one commit, maybe I'll have an easier time identifying what code change caused the error.

mcomella commented 4 years ago

From activity_main.xml:

    <android.support.design.widget.AppBarLayout
        android:id="@+id/appBarLayout"
        android:layout_width="match_parent"
        android:layout_height="@dimen/appbar_height"
        android:background="@color/photonGrey70"
        app:elevation="0dp">

If I remove the elevation statement, it fixes the issue. If I set the elevation to 1dp, the issue is not fixed.

Both of these changes causes the AppBar to appear over the NavigationOverlay's transparent black background so we'll need to adjust more elevation to fix the issue and keep other visuals the same.

mcomella commented 4 years ago

After testing (with values 0, 1, 8, 16), it appears if I set any elevation value in the XML layout, it will cause the WebView to stop playing when the toolbar is scrolled offscreen for an unknown reason. Setting the elevation in code with setTargetElevation (which is deprecated but actually works for AppBar, unlike elevation) has the same problem.

To work around this, I think I'll have to change the elevation of all the views around the default AppBar elevation which will fix the layering issues but will not disable the AppBar shadow like we'd prefer to do.

mcomella commented 4 years ago

And apparently no matter what elevation (i.e. 100dp) I set on the semi opaque background in the NavigationOverlayFragment, it does not draw over the AppBarLayout and gives the user access to touch it. :( I also tried reordering the views in the layout so the NavigationOverlayFragment container layout is before the app bar layout but it did not draw over first.

I think we need to do one of:

Disable toolbar scroll on this device (not really viable: it's a tiny device)
Selectively enable and disable the elevation when the toolbar is scrolled on or off screen (I'm not sure there's a listener for this or that there's any value we can set in code that would prevent this from breaking)
Use another mechanism to obscure the AppBarLayout (e.g. disable touch events, or draw another view on top of it (maybe by being inside the appBarLayout)) on this device

mcomella commented 4 years ago

Looking at the AppBarLayout source, it looks like the StateListAnimator that applies the elevation only gets set when app:elevation is set or targetElevation is used which would explain why it only freezes when changing the elevation. Unfortunately, nulling the state list animator doesn't appear to invert the effect so we'll have to look at option 3.

mcomella commented 4 years ago

Option 3 is working and I have a working WIP: https://github.com/mcomella/firefox-echo-show/tree/305-video-pauses I just need to clean up the commits and handle some last TODOs.

Potential alternative solution: remove the elevation have the toolbar animate offscreen when the navigation overlay is opened, at least on the Echo Show 5 where the bug is. It might look janky though. I'm going to stick with option 3 but my code is pretty hacky.

mcomella commented 4 years ago

QA: please follow the STR for testing. Please also verify the VoiceView screen reader is working as expected when clicking the menu button that opens the NavigationOverlay and dismissing it.

mcomella commented 4 years ago

Closed with https://github.com/mozilla-mobile/firefox-echo-show/commit/c0430da530179196a51c9b278d81425a802ef7cd.

nojunpark commented 4 years ago

This bug is no longer reproducible, but discovered that with Accessibility enabled, one cannot scroll down the pages with video. I will open a separate bug.

nojunpark commented 4 years ago

On 2nd look, this looks like it's due to how Echo show handles accessibility menu. (You can't scroll in general)

mcomella commented 4 years ago

@npark-mozilla I don't understand the accessibility comment: my understanding is you should be able to scroll on all WebView pages by using something like 2 or 3 fingers.

nojunpark commented 4 years ago

When Voiceview is enabled, and on youtube mobile site, it scrolls initially with three fingers (one echo show 5), but soon after it freezes unless i disable voiceview with my command. it feels like voiceview is making the app response slower.

mcomella commented 4 years ago

it feels like voiceview is making the app response slower.

I have also experienced this: both Fire TV (particularly the Stick) and the Echo Show seem to take a non-trivial perf hit when VoiceView is enabled.

mozilla-mobile / firefox-echo-show