[Bug] Pull to refresh triggered on full screen videos

[hkaancaliskan]

Steps to reproduce

Open a video in full screen on any website (for me it's YouTube)
Swipe down to trigger pull to refresh

Expected behavior

Pull to refresh not triggered while on full screen.

Actual behavior

It's triggered on full screen.

Device information

• Android device: Xperia XZ1 Android 10
• Fenix version: Nightly

┆Issue is synchronized with this Jira Task

[Mugurell]

I think in this case the solution would be to just add another check for fullscreen media to the canChildScrollUp method as the UNHANDLED result from PZC is the correct one.

[hkaancaliskan]

@rocketsroger I've updated to 3/12 and its not working on youtube :(

[hkaancaliskan]

@Mugurell issue closed but not fixed

[Mugurell]

I can also reproduce it on the latest Nightly.
Leaving to Roger to decide if this should be reopened or the work could continue on a new ticket.

@rocketsroger Seems like that check for HomeActivity.isImmersive is not being done at the right time.
Maybe we could could have a check to enable/disable pull to refresh in the fullscreenChanged method?

[mawen7]

Can confirm the suggestion by @Mugurell works.
Can open a PR if you'd like @rocketsroger

[rocketsroger]

I can reproduce as well. I think the reason that this sneaked passed my testing is that for some reason the first time pull to refresh is not disabled. After the first pull to refresh this then works correctly.

[rocketsroger]

Maybe we could could have a check to enable/disable pull to refresh in the fullscreenChanged method?

I was hoping my solution is cleaner then keeping track in the fullscreenChanged method. I'll do some more investigation to see if I can salvage the isImmersive check. If not I think you're right that fullscreenChanged method is a good spot. Thanks,

[rocketsroger]

Can confirm the suggestion by @Mugurell works.
Can open a PR if you'd like @rocketsroger

@mawen7 Thanks for offering your help. But I think since this is my mistake I should clean it up 😜

[mawen7]

Sure I understand that completely :)
Allow me one small hint: Since you can now use inFullScreen you can drop the isImmersive check.

IsImmersive was always false for me so I actually had to drop it.

[rocketsroger]

IsImmersive was always false for me so I actually had to drop it.

That's interesting. For me isImmersive works except like it was mentioned the check was not called in fullscreen.

Allow me one small hint: Since you can now use inFullScreen you can drop the isImmersive check.

Yes, what I'm testing is to pass the inFullscreen into the check. I agree that since we're using fullscreenChanged we don't need isImmersive anymore. Thanks