Tablet layout encountered after full screen video ends, despite Tablet mode being off

opusforlife2 commented 2 years ago

Checklist

[X] I am able to reproduce the bug with the latest version.
[X] I made sure that there are no existing issues - open or closed - which I could contribute my information to.
[X] I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.
[X] This issue contains only one bug.
[X] I have read and understood the contribution guidelines.

Affected version

0.23.1

Steps to reproduce the bug

Lock system rotation in portrait mode.
Turn off Tablet mode from Newpipe settings.
Play a video in full screen and let it finish.

Expected behavior

The app should fully respect the Tablet mode setting, and go into the phone version of landscape mode after exiting full screen. (Not optimal, but at least it should match the currently expected behaviour.)

Actual behavior

After the video ends, the app exits full screen and goes into the tablet version of the landscape layout, instead of the phone version. Thankfully, the "Tapping the Back button" workaround to go back to portrait mode still works on this tablet version, like it does on the phone version.

I've encountered this behaviour on a phone with Smallest width at 514 dp. This bug does not occur on a different phone which is at 449 dp. Both phones have Display size set to "Smaller" from Display settings. The Smallest width developer setting wasn't directly touched. Changing a user-facing option shouldn't cause this sort of bug.

Screenshots/Screen recordings

No response

Logs

No response

Affected Android/Custom ROM version

No response

Affected device model

No response

Additional information

On the ~500+ dp phone, turn on system auto-rotate. Open a video details fragment. Rapidly tilt the phone so you keep quickly shifting between landscape and portrait modes. You will have a ~50-50 chance of seeing either the phone version or the tablet version of the landscape layout each time the app rotates.

Ensure Autoplay is off. Turn on the "Start player in full screen" setting. Tap on a video thumbnail to start the video. The app will just rotate to the tablet version of landscape. The video doesn't play until you tap the thumbnail. The 449 dp phone correctly goes into full screen landscape and the video starts automatically.

I have a hunch that this entire problem might be technically "solved" by implementing #6067. But if so, it will still not actually address the issue that Newpipe isn't fully compliant with the Tablet mode setting, which was specifically added to avoid this class of problems entirely.

SameenAhnaf commented 2 years ago

Is this different from https://github.com/TeamNewPipe/NewPipe/issues/5967?

opusforlife2 commented 2 years ago

Indeed. Increasing the "Smallest width" setting beyond 480 dp breaks/violates the Tablet mode setting, essentially. That's not related to auto-return to portrait.

opusforlife2 commented 2 years ago

Wew. I tried the player refactor APK from #8170, just to see if this might have been fixed automagically like other issues, but I got a crash instead. It happens when I tap the full screen button while watching a video, or if the app tries to go into full screen automatically on player start. It doesn't happen if system rotation is unlocked, and I enter full screen by tilting the phone. This report was also generated only sometimes. Other times, the app crashed to the home screen without showing any error report.

@Stypox Another missing null check?

Exception

User Action: ui error
Request: ACRA report
Content Country: CH
Content Language: en-IN
App Language: en_IN
Service: none
Version: 0.23.1
OS: Linux Android 12 - 32
Crash log

java.lang.NullPointerException: Attempt to read from field 'android.widget.FrameLayout org.schabi.newpipe.databinding.FragmentVideoDetailBinding.playerPlaceholder' on a null object reference
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.lambda$addVideoPlayerView$18$org-schabi-newpipe-fragments-detail-VideoDetailFragment(VideoDetailFragment.java:1317)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment$$ExternalSyntheticLambda17.accept(Unknown Source:4)
    at j$.util.Optional.ifPresent(Optional.java:159)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.lambda$addVideoPlayerView$19$org-schabi-newpipe-fragments-detail-VideoDetailFragment(VideoDetailFragment.java:1315)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment$$ExternalSyntheticLambda6.run(Unknown Source:2)
    at android.os.Handler.handleCallback(Handler.java:938)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:201)
    at android.os.Looper.loop(Looper.java:288)
    at android.app.ActivityThread.main(ActivityThread.java:7870)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

opusforlife2 commented 2 years ago

Crash using the media session APK from #8678. Interestingly, it's the exact same crash log as https://github.com/TeamNewPipe/NewPipe/pull/8678#issuecomment-1200483145, but the exact same STR as https://github.com/TeamNewPipe/NewPipe/pull/8678#issuecomment-1207270580.

I'm mentioning it in this issue, because it needs the tablet layout of video details to occur. Doesn't occur in phone layout.

opusforlife2 commented 2 years ago

@Stypox Posting a regression in 0.24.0_RC1 here, because it only occurs for the high dp device.

Open a video page in portrait mode (autoplay off).
Tap on thumbnail. App rotates and shows the tablet UI now. Video may or may not play on its own at this stage.
If it doesn't play, tap Back, which rotates the video page back to portrait.
Close the video page with the 2-finger swipe down gesture. Crash:

FATAL EXCEPTION: main
Process: org.schabi.newpipe.v0_24_0, PID: 19100
android.app.RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground(): ServiceRecord{3be0e9b u0 org.schabi.newpipe.v0_24_0/org.schabi.newpipe.player.PlayerService}
    at android.app.ActivityThread.generateForegroundServiceDidNotStartInTimeException(ActivityThread.java:1965)
    at android.app.ActivityThread.throwRemoteServiceException(ActivityThread.java:1936)
    at android.app.ActivityThread.access$2700(ActivityThread.java:256)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2190)
    at android.os.Handler.dispatchMessage(Handler.java:106)
    at android.os.Looper.loopOnce(Looper.java:201)
    at android.os.Looper.loop(Looper.java:288)
    at android.app.ActivityThread.main(ActivityThread.java:7870)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
Caused by: android.app.StackTrace: Last startServiceCommon() call for this service was made here
    at android.app.ContextImpl.startServiceCommon(ContextImpl.java:1877)
    at android.app.ContextImpl.startForegroundService(ContextImpl.java:1832)
    at android.content.ContextWrapper.startForegroundService(ContextWrapper.java:781)
    at androidx.core.content.ContextCompat$Api26Impl.startForegroundService(ContextCompat.java:931)
    at androidx.core.content.ContextCompat.startForegroundService(ContextCompat.java:703)
    at org.schabi.newpipe.player.helper.PlayerHolder.startService(PlayerHolder.java:126)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openMainPlayer(VideoDetailFragment.java:1223)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.$r8$lambda$7cA0nfOW4EXxLpA03fs92UQohwM(Unknown Source:0)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment$$ExternalSyntheticLambda16.run(Unknown Source:2)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.replaceQueueIfUserConfirms(VideoDetailFragment.java:2150)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openVideoPlayer(VideoDetailFragment.java:1190)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.openVideoPlayerAutoFullscreen(VideoDetailFragment.java:1203)
    at org.schabi.newpipe.fragments.detail.VideoDetailFragment.onClick(VideoDetailFragment.java:518)
    at android.view.View.performClick(View.java:7455)
    at android.view.View.performClickInternal(View.java:7432)
    at android.view.View.access$3700(View.java:835)
    at android.view.View$PerformClick.run(View.java:28810)
    at android.os.Handler.handleCallback(Handler.java:938)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    ... 6 more

Stypox commented 2 years ago

Seems like yet another issue with how the player is started. We need to refactor that part in order to really fix these crashes. Since this seems like a rare bug, I think 0.24.0 should be shipped anyway, although I will try to see if I can fix it in time.

opusforlife2 commented 1 year ago

Oh no. #9207 fails to play videos from What's New feed with "start player in fullscreen" toggled on. Interestingly, opening and playing videos from channels and bookmarked playlists is fine:

Exception

User Action: ui error
Request: ACRA report
Content Country: CH
Content Language: en-IN
App Language: en_IN
Service: none
Version: 0.24.0
OS: Linux Android 12 - 32
Crash log

java.lang.IllegalStateException: Fragment SubscriptionFragment{c9da4cf} (3e6853fe-16ec-416b-b887-eccfec287386) not attached to a context.
    at androidx.fragment.app.Fragment.requireContext(Fragment.java:919)
    at androidx.fragment.app.Fragment.getResources(Fragment.java:983)
    at androidx.fragment.app.Fragment.getString(Fragment.java:1005)
    at org.schabi.newpipe.local.subscription.SubscriptionFragment.handleFeedGroups$lambda-16(SubscriptionFragment.kt:433)
    at org.schabi.newpipe.local.subscription.SubscriptionFragment.$r8$lambda$GrRRgvbQRBUpuIhA1TkTKb-6xFY(Unknown Source:0)
    at org.schabi.newpipe.local.subscription.SubscriptionFragment$$ExternalSyntheticLambda0.run(Unknown Source:6)
    at android.os.Handler.handleCallback(Handler.java:938)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:201)
    at android.os.Looper.loop(Looper.java:288)
    at android.app.ActivityThread.main(ActivityThread.java:7870)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

Stypox commented 1 year ago

@opusforlife2 try the APK from the latest commit

opusforlife2 commented 1 year ago

Fixed!

opusforlife2 commented 1 year ago

Another bug: when swiping down on a full screen video, the bottom tabs are missing. But they are present on a low dp screen. Just like so much else here, toggling Tablet mode on and off doesn't affect this behaviour either way.

TeamNewPipe / NewPipe