Repository: qutebrowser/qutebrowser
Pull request: Prevent exiting video player from exiting fullscreen
…ullscreen)

fixes #4349 on linux/Xorg/KDE.

Can someone test this on MacOS?

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/qutebrowser/qutebrowser/4364)
<!-- Reviewable:end -->


Problem statement:
Exiting video player exits fullscreen
**Version info (see `:version`)**:
qutebrowser v1.5.1
Git commit: c7a21b37e-dirty (2018-10-10 08:23:17 +0200)
Backend: QtWebEngine (Chromium 65.0.3325.230)

CPython: 3.7.0
Qt: 5.11.2
PyQt: 5.11.3

sip: 4.19.13
colorama: 0.3.9
pypeg2: 2.15
jinja2: 2.10
pygments: 2.2.0
yaml: 3.13
cssutils: 1.0.2 $Id$
attr: 18.2.0
PyQt5.QtWebEngineWidgets: yes
PyQt5.QtWebKitWidgets: no
pdf.js: 1.10.100 (bundled)
sqlite: 3.24.0
QtNetwork SSL: Secure Transport, macOS 10.14

Style: QMacStyle
Platform: Darwin-18.0.0-x86_64-i386-64bit, 64bit
Frozen: True
Imported from /Applications/qutebrowser.app/Contents/MacOS/qutebrowser
Using Python from /Applications/qutebrowser.app/Contents/MacOS/qutebrowser
Qt library executable path: /Applications/qutebrowser.app/Contents/MacOS/PyQt5/Qt/libexec, data path: /Applications/qutebrowser.app/Contents/MacOS/PyQt5/Qt
OS Version: 10.14, x86_64

**Description**
On MacOS, when exiting fullscreen mode in any video player the whole browser will exit fullscreen mode. 
**How to reproduce**
Go to any video playing site. E.g youtube.com
Fullscreen the video
Exit the fullscreen

Hints from issue discussion:
It also does this on linux under Xorg.
I noticed this too, thanks for reporting.

The code to fix this is [here](https://github.com/qutebrowser/qutebrowser/blob/master/qutebrowser/mainwindow/mainwindow.py#L516-L521).

Weirdly, it seems to have been introduced in 559059d244ccf95c56bd1c6f982f700ce13aa038, which was added in #3482, a PR which was trying to solve this problem...


cc @helbling
I can confirm the bug.
In 559059d and cd20c329 I got rid of the state variable window.state_before_fullscreen as the behaviour seemed to be correct at the time.
This is definitely no longer the case and we need to revert these commits.

First tests look good.
I will prepare a pull request.

Commit message: revert 559059d and cd20c32 to fix #4349 (Exiting video player exits fullscreen)

Patch to review:
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index a3f301ae51c..8ea3c08a5cf 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -35,7 +35,7 @@
                                  webelem, downloads)
 from qutebrowser.keyinput import modeman, keyutils
 from qutebrowser.utils import (message, usertypes, log, qtutils, urlutils,
-                               objreg, utils, standarddir)
+                               objreg, utils, standarddir, debug)
 from qutebrowser.utils.usertypes import KeyMode
 from qutebrowser.misc import editor, guiprocess
 from qutebrowser.completion.models import urlmodel, miscmodels
@@ -2234,7 +2234,16 @@ def fullscreen(self, leave=False):
             return
 
         window = self._tabbed_browser.widget.window()
-        window.setWindowState(window.windowState() ^ Qt.WindowFullScreen)
+        if window.isFullScreen():
+            window.setWindowState(
+                window.state_before_fullscreen & ~Qt.WindowFullScreen)
+        else:
+            window.state_before_fullscreen = window.windowState()
+            window.setWindowState(
+                window.state_before_fullscreen | Qt.WindowFullScreen)
+        log.misc.debug('state before fullscreen: {}'.format(
+            debug.qflags_key(Qt, window.state_before_fullscreen)))
+
 
     @cmdutils.register(instance='command-dispatcher', scope='window',
                        name='tab-mute')
diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py
index 3fdad13d1c5..249e6e81d24 100644
--- a/qutebrowser/mainwindow/mainwindow.py
+++ b/qutebrowser/mainwindow/mainwindow.py
@@ -31,7 +31,7 @@
 from qutebrowser.commands import runners, cmdutils
 from qutebrowser.config import config, configfiles
 from qutebrowser.utils import (message, log, usertypes, qtutils, objreg, utils,
-                               jinja)
+                               jinja, debug)
 from qutebrowser.mainwindow import messageview, prompt
 from qutebrowser.completion import completionwidget, completer
 from qutebrowser.keyinput import modeman
@@ -136,6 +136,7 @@ class MainWindow(QWidget):
     Attributes:
         status: The StatusBar widget.
         tabbed_browser: The TabbedBrowser widget.
+        state_before_fullscreen: window state before activation of fullscreen.
         _downloadview: The DownloadView widget.
         _vbox: The main QVBoxLayout.
         _commandrunner: The main CommandRunner instance.
@@ -237,6 +238,8 @@ def __init__(self, *, private, geometry=None, parent=None):
         objreg.get("app").new_window.emit(self)
         self._set_decoration(config.val.window.hide_decoration)
 
+        self.state_before_fullscreen = self.windowState()
+
     def _init_geometry(self, geometry):
         """Initialize the window geometry or load it from disk."""
         if geometry is not None:
@@ -516,9 +519,13 @@ def _set_decoration(self, hidden):
     def _on_fullscreen_requested(self, on):
         if not config.val.content.windowed_fullscreen:
             if on:
-                self.setWindowState(self.windowState() | Qt.WindowFullScreen)
+                self.state_before_fullscreen = self.windowState()
+                self.setWindowState(
+                    Qt.WindowFullScreen | self.state_before_fullscreen)
             elif self.isFullScreen():
-                self.setWindowState(self.windowState() & ~Qt.WindowFullScreen)
+                self.setWindowState(self.state_before_fullscreen)
+        log.misc.debug('on: {}, state before fullscreen: {}'.format(
+            on, debug.qflags_key(Qt, self.state_before_fullscreen)))
 
     @cmdutils.register(instance='main-window', scope='window')
     @pyqtSlot()


Code review comments:
diff_hunk: @@ -2234,7 +2234,16 @@ def fullscreen(self, leave=False):
             return
 
         window = self._tabbed_browser.widget.window()
-        window.setWindowState(window.windowState() ^ Qt.WindowFullScreen)
+        if window.isFullScreen():
path: qutebrowser/browser/commands.py
text: @user1:
I think the `setWindowState` part of this if statement is the same as the xor before, right? I do like this more because it's a little more explicit though...

@author:
The other option would be to do it like this:
```
        if not window.isFullScreen():
            window.state_before_fullscreen = window.windowState()
        window.setWindowState(window.windowState() ^ Qt.WindowFullScreen)
```
The behaviour should be the same.
I actually like the xor version a bit better as it separates the code for state_before_fullscreen and the actual fullscreen toggling. And it does it with less code. 
Should I switch to that one?

@user2:
It depends on what each person is familiar with. Personally, I think it's obvious that xor == toggle.

@user1:
@author I'm fine with either one, I was just a little thrown off why this part was changed at first. :)