Selectable behavior of Back button

[avently]

What is it?

• [x] Bugfix (user facing)
• [x] Feature (user facing)
• [ ] Codebase improvement (dev facing)
• [ ] Meta improvement to the project (dev facing)

Description of the changes in your PR

Made an option to change behavior of back button on video fragment. It's something that should be included into initial unified player PR. Now NewPipe users can choose what to do when back button pressed. There are six choices:

• open previous video from history (like now, stays default choice)
• open previous video only in case it was playing from the same playlist as current one
• hide to mini player regardless of what was in the history
• hide into the mini player if the stream was played, close otherwise
• close the player fragment completely but only if nothing is playing or only video is playing. Just hides the player to mini player in case audio or popup are playing
• close even in fullscreen.

Also I noticed that for some reason equals check is not working correctly currently. So I reimplemented it based on urls compare of inner streams (see the first commit).

Fixes the following issue(s)

Fixes #4479

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

• [x] I read the contribution guidelines.

[SameenAhnaf]

Thanks for the amazing PR. It's working flawlessly. If I am not wrong, #4569 and #5590 should also be linked with the PR.

Could you please add an option like "Direct exit from player" so that users can directly exit video from full screen? I know, there's a gesture for that. But it's quite tricky on TV.

[opusforlife2]

@avently Is this intended to close #4569? If option 4 is meant for that, then I'll address it on that basis (I found some problems). If not, then no issues.

[avently]

@SameenAhnaf I'll add

@opusforlife2

@avently Is this intended to close #4569? If option 4 is meant for that, then I'll address it on that basis (I found some problems). If not, then no issues.

Address it, if it is needed for you

[opusforlife2]

Okay, so for option 4, we ideally want the Back button to close the player directly only if backstack size is 0, as stated in #4569. Right now, this PR closes using Back button even if backstack size is non-zero. This happens even if a video is playing.

[avently]

I added two new options:

• hide and close which is basically what @opusforlife2 asks
• close even in fullscreen which is what @SameenAhnaf asks. Ready for merging @Stypox @TobiGr @mhmdanas

[opusforlife2]

I added two new options: hide and close

@avently Bug: Back button closes the player even if the backstack size is non-zero.

• Open a video, but don't play it.
• Open a related video, but don't play that either.
• Tap the Back button.
• Player closed, losing the entire backstack.

What it should do in this case is rewind through the stack, just like the current release does.

To be clear, closing should only happen if both conditions are satisfied: size of backstack == 0 && player is open == false.

[avently]

It was intentional. So what's the point of your idea then? It acts as the default behavior with exception to non-played first item in backstack?

Ok, will make like this.

[SameenAhnaf]

I think, choosing "Close the player even in full screen" should put the app back in portrait on smartphone after clicking 'Back'. Could you please do something about it?

[avently]

Check again, made these two changes you both asked

[SameenAhnaf]

On my device, 'Close the main player even in full screen' is chosen and auto-rotation is disabled. 2 seconds of lag for closing mini player and changing rotation can be seen when I am trying to exit the video from full screen.

Such a lag is not found when getting back into main player both through 'Minimize' and 'Back' buttons. So, this lag is unexpected.

Could you please fix it?

[opusforlife2]

@avently Now the opposite happens. I open a video but don't tap the thumbnail. I tap Back. The page goes into the mini player instead of closing. Since I opened only one video, it's the only one in the backstack, it should have closed directly.

It acts as the default behavior with exception to non-played first item in backstack?

Yes, that's the idea.

[avently]

Such a lag is not found when getting back into main player both through 'Minimize' and 'Back' buttons. So, this lag is unexpected.

"The lag" is equal to 500ms, not two seconds. Player closes first, after that rotation happens. Without 500ms delay rotation mechanism skipping player closing. So nothing special here. If you see the 2 sec delay is because of slow rotation on debug apk. On release apk it will be much faster.

@opusforlife2 could not reproduce. I have disabled autoplay and after I open a video without clicking on thumbnail and then click back button, player closes. Make sure you selected Hide into the mini player if the stream was played, close otherwise

[opusforlife2]

Make sure you selected Hide into the mini player if the stream was played, close otherwise

Ah, indeed. I am a dum-dum. After deleting and installing the latest APK and importing my database, I forgot to set the option again. After setting it to this, the app works exactly as expected.

Thank you, @avently!

[opusforlife2]

If I am not wrong, #4569 and #5590 should also be linked with the PR.

Indeed. Done. 👍

[avently]

Guys, let me know when you'll be ready to merge this. I'll fix merge conflicts then

[opusforlife2]

@avently Could you add the two new options into the PR description? Thanks.

[avently]

@opusforlife2 done

[skyGtm]

This PR also addresses the issue I had.

Issue: I am using NP v0.21.10 When autoplay is disabled

1. play some video
2. scroll down the comments
3. Press back button Video pauses and comments jumps to the top , rather than minimizing the player while still background playing the video.

But this PR fixes this. Thankyou. :tada: :tada:

[ghost]

Hey the default back button works great on touch screen but can the default behavior be changed on TVs to close the player fragment? Thanks for the work @avently

[avently]

@Bugginme as far as I remember there is such option in this PR

[ghost]

@avently oh thanks, this pull request fixes a few issues on TV

[Stypox]

@avently do you need some more time or reviews or anything before proceeding with this PR?

[avently]

@Stypox nope, just a couple of free hours. Will try to do it tomorrow maybe

[avently]

Force-pushed my changes and packed them into one commit.

Here I removed the commit related to equals check because @Stypox asked about that. However this build (as well as dev branch) without this commit works bad. Looks like someone is ruined previous behavior and whenever I choose any video it gets added to the backstack twice which shouldn't happen. The fix is here: #7910

About strings. Thanks to @Stypox for correcting me, I changed some of the strings to match his version of them.

Added more comments and explanations in some places.

Other parts of code are remain the same as before. Let me know if some unexpeced things happen (except duplicated videos in a queue).

[avently]

@Stypox

Added some comments and have a couple of questions (you can let me add the comments myself, just answer the questions if you don't have time)

Answered on the questions. Agree with your comments, they are more detailed and still correct. So you can commit them if you like.

[avently]

Merged 'dev' branch. Now test apk contains the fix related to duplicates in a backstack.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

0.0% Coverage
0.0% Duplication

[Stypox]

Here is a table explaining what each setting does. To me this table looks really really complex and many of the behaviors have nothing to do with the description. This is a possibly a source of many complaints from users: "I expected setting X to do YZ but instead it does AYBZC, I think this is a bug as it's not explained anywhere it should also do ABC!". So either we make the descriptions far more descriptive, or we simplify the conditions. I would go with the latter, since many of these conditions do not make sense to me and would seem completely arbitrary to a user reading the descriptive descriptions. What do you think @opusforlife2 @SameenAhnaf @litetex @avently?

Key	Description	Conditions (ordered by priority)	-> behaviors
BACK_BEHAVIOR_PREV_ANY	Open previous video from backstack of opened videos	(video) player open and in fullscreen	Exit from fullscreen
		Video player open and there is a previous stream in the play queue	Play previous stream
		There is a previous video in the backstack	Go to previous video in the backstack
		This is the last video in the backstack	Restore default orientation and let main activity handle the onBack
BACK_BEHAVIOR_PREV_PLAYLIST	Open previous video only if belonging to a playlist (playlist is to be interpreted as play queue)	(video) player open and in fullscreen	Exit from fullscreen
		Video player open and there is a previous stream in the play queue	Play previous stream
		Otherwise	Collapse the bottom sheet
BACK_BEHAVIOR_HIDE	Minimize into the mini player	(video) player open and in fullscreen	Exit from fullscreen
		Otherwise	Collapse the bottom sheet
BACK_BEHAVIOR_HIDE_OR_CLOSE	Minimize into the mini player if something is playing, close otherwise	(video) player open and in fullscreen	Exit from fullscreen
		Video player open and there is a previous stream in the play queue	Play previous stream
		There is a previous video in the backstack	Go to previous video in the backstack
		This is the last video in the backstack and (player not open or video player open, but with different play queue)	Restore default orientation and hide the bottom sheet
		This is the last video in the backstack and not (^)	Restore default orientation and let main activity handle the onBack
BACK_BEHAVIOR_CLOSE	Close the video player	(video) player open and in fullscreen	Exit from fullscreen
		Player not open or video player selected (not fullscreen)	Close the player if needed and hide the bottom bar
		Background or popup player selected	Collapse the bottom sheet
BACK_BEHAVIOR_CLOSE_FULLSCREEN	Close the video player even in fullscreen	Player not open or video player selected	Exit from fullscreen if needed, close the player if needed and hide the bottom bar
		Background or popup player selected	Collapse the bottom sheet

[SameenAhnaf]

@Stypox https://github.com/TeamNewPipe/NewPipe/issues/7973 is a better solution, I think. https://github.com/TeamNewPipe/NewPipe/issues/7974 for BACK_BEHAVIOR_CLOSE_FULLSCREEN

@avently I updated https://github.com/TeamNewPipe/NewPipe/issues/5940#issuecomment-825590093 for multiple queues.

[opusforlife2]

I think it is pertinent to ask: who asked for each of these different behaviours? For example, BACK_BEHAVIOR_HIDE_OR_CLOSE came from a discussion between @TheAssassin and I. I'm assuming some user or other has asked for each of the other options. Is that the case, @avently?

If so, we probably do need to have a discussion on simplification. Assassin and I came up with the BACK_BEHAVIOR_HIDE_OR_CLOSE solution because that looked like the major grievance for most users who complained about the Back button behaviour in the 0.19.x -> 0.20.x transition.

So we need such users to chime in about what issue they had, (or avently could link to such comments, if you remember them?) so that we can see if each option is actually needed. See next part.

---

It is possible that there are other solutions than modifying the Back button behaviour for some of the grievances users put out.

For example, the full screen one: BACK_BEHAVIOR_CLOSE_FULLSCREEN. Currently, it closes the video player completely, causing you to lose the backstack.

The 0.19.x behaviour was: open the video page -> open the video in fullscreen (use the current fullscreen button only for rotating the video between portrait/landscape) -> close the video, but stay on the video page. -- This would preserve the backstack if the option is modified this way.

So from my POV, this BACK_BEHAVIOR_CLOSE_FULLSCREEN setting, with the 0.19.x behaviour, could be combined with the "Start main player in full screen" option, into a single "Use main player in full screen only". Tapping a thumbnail on the video page would start the video in fullscreen, and tapping Back would close the video entirely, and take you back to the same state, where the video page is opened with the thumbnail, not the embedded portrait player.

This would mean the "Use main player in full screen only" would satisfy the users who preferred the 0.19.x behaviour. And at least one option would be removed from this setting.