Skip to content

Conversation

absidue
Copy link
Member

@absidue absidue commented May 20, 2024

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

Pull Request Type

  • Bugfix

Related issue

Description

This is an alternative to #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

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 20, 2024 20:47
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 20, 2024
Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Works!

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.

Works!

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, tested on win10

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 20, 2024

This comment was marked as outdated.

Copy link
Contributor

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

Copy link
Contributor

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.

@efb4f5ff-1298-471a-8973-3d47447115dc

doesnt this also address #3220?

Copy link

@iacore iacore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

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

Copy link
Contributor

github-actions bot commented Jun 1, 2025

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

@PikachuEXE
Copy link
Collaborator

Seems working (test after merged with dev
Still WIP?

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Jun 3, 2025

@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

@efb4f5ff-1298-471a-8973-3d47447115dc

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants