From c4bfd848f966a263fc72d84f4237221b83fe7c89 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 5 Dec 2022 19:45:13 +0100 Subject: app/mainwindow: Delay .show() calls until after tabs are added With Qt 6.4, QtWebEngine closes/reopens the main window to switch the RHI rendering mode when a QWebEngineView gets added: https://github.com/qt/qtbase/blob/v6.4.1/src/widgets/kernel/qwidget.cpp#L10706-L10709 To avoid this, we need to make sure we only call .show() *after* adding a tab, similarly to what Qt did too: https://code.qt.io/cgit/qt/qtwebengine.git/commit/?id=d7e0fd5304ebdb12c6f809cdbcf8193b49b9ecd2 See #7504 ---- This commit handles changes around app.py/mainwindow.py, which was probably the most complex one of the whole set (and also involving the oldest code I bet...). This changes a couple of intertwined things: - app._process_args() now needs to take track of the window it opened (if any), so that it can call .show() on it *after* opening pages. * If a session was loaded instead, sessions.py takes care of showing it. * The setActiveWindow call was also moved for simplicitly. Before, it was called even with --nowindow given, but that doesn't really make much sense. * I'm not actually sure why the .setActiveWindow() call is there. Qt docs say "Warning: This function does not set the keyboard focus to the active widget. Call QWidget::activateWindow() instead.". It was added back in 2014: ae44aa01a6d7d6f2e2b2e97a48f49a579f342c9f ("Set initial focused window correctly."). It's possible it's not needed anymore, but that's for a separate commit. - app.process_pos_args() now gets a MainWindow (rather than win_id) from mainwindow.get_window(), and is responsible for calling .show() and .maybe_raise() on it when nothing else does (like open_url called from it). * To preserve existing behavior (and not fail tests), it also still calls .maybe_raise() when a command was given. Maybe we should get rid of this to fix #5094, but that's for a separate commit. * Note it does *not* call .show(). That's taken care of later in _process_args() already. - app.open_url() can directly show the window, because it already opens a URL. It now still returns the MainWindow object for consistency. Also gets rid of some questionable objreg usage just to access a tabbed browser, as a tiny step towards #640. - Similarly, app._open_startpage() could have stayed, but now also takes a MainWindow and uses objreg a bit less. - open_desktopservices_url also takes care of the show/raise (as this gets called isolated from Qt), and also avoids objreg stuff where an attribute access would have sufficed... - mainwindow.get_window() now doesn't show the window anymore, and returns the mainwindow object instead of a window ID. * However, this means it can't actually raise the window anymore (it needs to be shown first). - To keep the decision about whether to raise the window in a central place, MainWindow now has a new should_raise attribute and maybe_raise() method. mainwindow.get_window() sets should_raise accordingly, and the caller is responsible to call .maybe_raise() after showing. --- qutebrowser/app.py | 77 +++++++++++++++++++----------------- qutebrowser/mainwindow/mainwindow.py | 26 +++++++----- 2 files changed, 57 insertions(+), 46 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 6b5c35914..db7eea608 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -200,6 +200,7 @@ def _process_args(args): if not args.override_restore: sessions.load_default(args.session) + new_window = None if not sessions.session_manager.did_load: log.init.debug("Initializing main window...") private = args.target == 'private-window' @@ -210,15 +211,17 @@ def _process_args(args): error.handle_fatal_exc(err, 'Cannot start in private mode', no_err_windows=args.no_err_windows) sys.exit(usertypes.Exit.err_init) - window = mainwindow.MainWindow(private=private) - if not args.nowindow: - window.show() - objects.qapp.setActiveWindow(window) + + new_window = mainwindow.MainWindow(private=private) process_pos_args(args.command) _open_startpage() _open_special_pages(args) + if new_window is not None and not args.nowindow: + new_window.show() + objects.qapp.setActiveWindow(new_window) + delta = datetime.datetime.now() - earlyinit.START_TIME log.init.debug("Init finished after {}s".format(delta.total_seconds())) @@ -242,26 +245,30 @@ def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): if command_target in {'window', 'private-window'}: command_target = 'tab-silent' - win_id: Optional[int] = None + window: Optional[mainwindow.MainWindow] = None if via_ipc and (not args or args == ['']): - win_id = mainwindow.get_window(via_ipc=via_ipc, - target=new_window_target) - _open_startpage(win_id) + window = mainwindow.get_window(via_ipc=via_ipc, target=new_window_target) + _open_startpage(window) + window.show() + window.maybe_raise() return for cmd in args: if cmd.startswith(':'): - if win_id is None: - win_id = mainwindow.get_window(via_ipc=via_ipc, - target=command_target) + if window is None: + window = mainwindow.get_window(via_ipc=via_ipc, target=command_target) + # FIXME preserving old behavior, but we probably shouldn't be + # doing this... + # See https://github.com/qutebrowser/qutebrowser/issues/5094 + window.maybe_raise() + log.init.debug("Startup cmd {!r}".format(cmd)) - commandrunner = runners.CommandRunner(win_id) + commandrunner = runners.CommandRunner(window.win_id) commandrunner.run_safely(cmd[1:]) elif not cmd: log.init.debug("Empty argument") - win_id = mainwindow.get_window(via_ipc=via_ipc, - target=new_window_target) + window = mainwindow.get_window(via_ipc=via_ipc, target=new_window_target) else: if via_ipc and target_arg and target_arg != 'auto': open_target = target_arg @@ -275,7 +282,7 @@ def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): message.error("Error in startup argument '{}': {}".format( cmd, e)) else: - win_id = open_url(url, target=open_target, via_ipc=via_ipc) + window = open_url(url, target=open_target, via_ipc=via_ipc) def open_url(url, target=None, no_raise=False, via_ipc=True): @@ -288,39 +295,37 @@ def open_url(url, target=None, no_raise=False, via_ipc=True): via_ipc: Whether the arguments were transmitted over IPC. Return: - ID of a window that was used to open URL + The MainWindow of a window that was used to open the URL. """ target = target or config.val.new_instance_open_target background = target in {'tab-bg', 'tab-bg-silent'} - win_id = mainwindow.get_window(via_ipc=via_ipc, target=target, - no_raise=no_raise) - tabbed_browser = objreg.get('tabbed-browser', scope='window', - window=win_id) + window = mainwindow.get_window(via_ipc=via_ipc, target=target, no_raise=no_raise) log.init.debug("About to open URL: {}".format(url.toDisplayString())) - tabbed_browser.tabopen(url, background=background, related=False) - return win_id + window.tabbed_browser.tabopen(url, background=background, related=False) + window.show() + window.maybe_raise() + return window -def _open_startpage(win_id=None): +def _open_startpage(window: Optional[mainwindow.MainWindow] = None) -> None: """Open startpage. The startpage is never opened if the given windows are not empty. Args: - win_id: If None, open startpage in all empty windows. + window: If None, open startpage in all empty windows. If set, open the startpage in the given window. """ - if win_id is not None: - window_ids: Iterable[int] = [win_id] + if window is not None: + windows: Iterable[mainwindow.MainWindow] = [window] else: - window_ids = objreg.window_registry - for cur_win_id in list(window_ids): # Copying as the dict could change - tabbed_browser = objreg.get('tabbed-browser', scope='window', - window=cur_win_id) - if tabbed_browser.widget.count() == 0: + windows = objreg.window_registry.values() + + for cur_window in list(windows): # Copying as the dict could change + if cur_window.tabbed_browser.widget.count() == 0: log.init.debug("Opening start pages") for url in config.val.url.start_pages: - tabbed_browser.tabopen(url) + cur_window.tabbed_browser.tabopen(url) def _open_special_pages(args): @@ -425,10 +430,10 @@ def on_focus_changed(_old, new): def open_desktopservices_url(url): """Handler to open a URL via QDesktopServices.""" target = config.val.new_instance_open_target - win_id = mainwindow.get_window(via_ipc=True, target=target) - tabbed_browser = objreg.get('tabbed-browser', scope='window', - window=win_id) - tabbed_browser.tabopen(url) + window = mainwindow.get_window(via_ipc=True, target=target) + window.tabbed_browser.tabopen(url) + window.show() + window.maybe_raise() # This is effectively a @config.change_filter diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 121a94460..9acff5f4b 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -48,7 +48,7 @@ win_id_gen = itertools.count(0) def get_window(*, via_ipc: bool, target: str, - no_raise: bool = False) -> int: + no_raise: bool = False) -> "MainWindow": """Helper function for app.py to get a window id. Args: @@ -58,11 +58,11 @@ def get_window(*, via_ipc: bool, no_raise: suppress target window raising Return: - ID of a window that was used to open URL + The MainWindow that was used to open URL """ if not via_ipc: # Initial main window - return 0 + return objreg.get("main-window", scope="window", window=0) window = None should_raise = False @@ -70,20 +70,16 @@ def get_window(*, via_ipc: bool, # Try to find the existing tab target if opening in a tab if target not in {'window', 'private-window'}: window = get_target_window() - should_raise = target not in {'tab-silent', 'tab-bg-silent'} + window.should_raise = target not in {'tab-silent', 'tab-bg-silent'} and not no_raise is_private = target == 'private-window' # Otherwise, or if no window was found, create a new one if window is None: window = MainWindow(private=is_private) - window.show() - should_raise = True + window.should_raise = not no_raise - if should_raise and not no_raise: - raise_window(window) - - return window.win_id + return window def raise_window(window, alert=True): @@ -133,6 +129,8 @@ class MainWindow(QWidget): status: The StatusBar widget. tabbed_browser: The TabbedBrowser widget. state_before_fullscreen: window state before activation of fullscreen. + should_raise: Whether the window should be raised/activated when maybe_raise() + gets called. _downloadview: The DownloadView widget. _download_model: The DownloadModel instance. _vbox: The main QVBoxLayout. @@ -280,6 +278,8 @@ class MainWindow(QWidget): self._set_decoration(config.val.window.hide_decoration) self.state_before_fullscreen = self.windowState() + self.should_raise = False + stylesheet.set_register(self) def _init_geometry(self, geometry): @@ -666,6 +666,12 @@ class MainWindow(QWidget): return True + def maybe_raise(self) -> None: + """Raise the window if self.should_raise is set.""" + if self.should_raise: + raise_window(self) + self.should_raise = False + def closeEvent(self, e): """Override closeEvent to display a confirmation if needed.""" if crashsignal.crash_handler.is_crashing: -- cgit v1.2.3-54-g00ecf