FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Fix player full screen causing the app to start in full screen

Open absidue opened this issue 1 year ago • 8 comments

Fix player full screen causing the app to start in full screen

Pull Request Type

  • [x] Bugfix

Related issue

  • closes #4534
  • closes #4649
  • closes #3220

Description

This is an alternative to https://github.com/FreeTubeApp/FreeTube/pull/4649 that should hopefully work more consistently (the creator of that pull request is aware that I am making this one).

When the app is closed while in full screen regardless of whether only the player or the entire window was full-screened, we save the full screen state and restore it at start up. For people that full-screened the app that is great, but for people that full-screened the player it's less desirable to have the entire app start in full screen.

Testing

Full screen doesn't persist if player is full-screened

  1. Open a video
  2. Full screen the player, either through the button or the F hotkey
  3. Close the app e.g. Ctrl+Q or Alt+F4
  4. Open the app again and make sure it launches with normal window dimensions

Full screen persists if you full screen the window (regression test)

  1. Full screen the window e.g. F11
  2. Close the app e.g. Ctrl+Q or Alt+F4
  3. Open the app again and make sure it launches in full screen.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.20.0

absidue avatar May 20 '24 20:05 absidue

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar May 20 '24 23:05 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar May 21 '24 21:05 github-actions[bot]

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Jun 19 '24 01:06 github-actions[bot]

doesnt this also address https://github.com/FreeTubeApp/FreeTube/issues/3220?

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Jan 20 '25 14:01 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Jun 01 '25 21:06 github-actions[bot]

Seems working (test after merged with dev Still WIP?

PikachuEXE avatar Jun 03 '25 01:06 PikachuEXE

@PikachuEXE

Full screen persists if you full screen the window and if player is fullscreened

  1. Full screen the window e.g. F11
  2. Full screen the player, either through the button or the F hotkey
  3. Close the app e.g. Ctrl+Q or Alt+F4
  4. Open the app again and make sure it launches in full screen.

App unfortunately doesn't launch in fullscreen

maybe usefull chagpt prompt we

Thanks for sharing the screenshot and the test cases. From what you're describing and what I see in the code diff, here's what's happening and how to fix the case where both the window and the player are full-screened:


✅ Current Fix (What the patch does):

The patch avoids saving fullscreen state if it was triggered by the video player (htmlFullScreenWindowIds tracks this). That works well for:

  • Only player fullscreen → doesn't persist ✔️
  • Only window fullscreen → does persist ✔️

❌ Failing Case:

Both window and player are fullscreened → window fullscreen is ignored on next launch ❌

This happens because the htmlFullScreenWindowIds logic overrides the check and causes the app not to save the window.fullScreen state — even if it was actually fullscreen at the window level.


🛠️ Fix:

We need to detect whether both are true and ensure we still save the window.fullScreen state if the window is in fullscreen, even if the player also was.

✅ Fix Suggestion:

Replace this block:

fullScreen: newWindow.isFullScreen() && !htmlFullScreen,

With something smarter like:

const isWindowActuallyFullScreen = newWindow.isFullScreen();
const isHtmlFullScreen = htmlFullScreenWindowIds.has(newWindow.id);

// Save fullscreen only if it was true window fullscreen (not just player fullscreen)
fullScreen: isWindowActuallyFullScreen && !(isHtmlFullScreen && !newWindow.isMenuBarVisible()),

!newWindow.isMenuBarVisible() is an example heuristic — adjust based on Electron API or UI clues if applicable.


🔍 Why This Works:

You're distinguishing between true fullscreen (like from F11) and HTML fullscreen (like a video player). If both were active, this check preserves the window's fullscreen status.


✅ Regression-Safe?

Yes, as long as isFullScreen() remains reliable and htmlFullScreenWindowIds is properly maintained.


Let me know your platform or Electron version if you want help refining this logic further.