From 5a0e9ddfe7185f98de4d50ea6bf73db4e7a44840 Mon Sep 17 00:00:00 2001 From: toofar Date: Wed, 7 Feb 2024 17:57:10 +1300 Subject: Be explicit about tab position when loading from sessions With, presumably, rare combinations of settings tab order in windows loaded from session can be different from the window the session was saved from. In particular with `tabs.new_position.related=prev` the tabs would be loaded in reverse order! This commit changes tabs loaded from sessions to 1) override positioning related kwargs to tabopen so that it doesn't look at user sessions 2) provide an explicit index to use for new tabs. This particular test passes with either one of the newly passed kwargs. We don't strictly need both of them, but while being explicit we may as well override everything to be extra clear. While looking at this code you may be asking why background is set to True for all the tabs when only one of them is going to be focused? Good question, I don't think it should be, see https://github.com/qutebrowser/qutebrowser/issues/6983#issuecomment-1025011641 But since that is explicitly set it should probably be a separate PR to change it. Looks like the original PR to fix that got caught up in wider scope and trying to do a new style of e2e-but-python tests. It should be easy enough to write an e2e that loads a page with JS that reports that visibility state. TODO: * add a similar test for tree tabs when the scaffolding is there (with `new_position.tree.new_toplevel=prev` too) * override sibling too? --- qutebrowser/misc/sessions.py | 12 ++++++++++-- tests/end2end/features/sessions.feature | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/qutebrowser/misc/sessions.py b/qutebrowser/misc/sessions.py index 78209dde2..df3b028f7 100644 --- a/qutebrowser/misc/sessions.py +++ b/qutebrowser/misc/sessions.py @@ -537,7 +537,11 @@ class SessionManager(QObject): if tab_data.get('active'): tab_to_focus = index - new_tab = tabbed_browser.tabopen(background=False) + new_tab = tabbed_browser.tabopen( + background=False, + related=False, + idx=index, + ) self._load_tab(new_tab, tab_data) new_tab.node.parent = root_node @@ -603,7 +607,11 @@ class SessionManager(QObject): tab_to_focus = self._load_tree(tabbed_browser, tree_data) elif not legacy_tree_loaded: for i, tab in enumerate(tabs): - new_tab = tabbed_browser.tabopen(background=False) + new_tab = tabbed_browser.tabopen( + background=False, + related=False, + idx=i, + ) self._load_tab(new_tab, tab) if tab.get('active', False): tab_to_focus = i diff --git a/tests/end2end/features/sessions.feature b/tests/end2end/features/sessions.feature index 9a61baf61..2f121132f 100644 --- a/tests/end2end/features/sessions.feature +++ b/tests/end2end/features/sessions.feature @@ -440,3 +440,18 @@ Feature: Saving and loading sessions - data/numbers/2.txt (active) (pinned) - data/numbers/4.txt - data/numbers/3.txt + + # Make sure the new_position.related setting doesn't change the tab order + # when loading from a session. + Scenario: Loading a session with tabs.new_position.related=prev + When I open data/numbers/1.txt + And I open data/numbers/2.txt in a new tab + And I open data/numbers/3.txt in a new tab + And I run :session-save foo + And I set tabs.new_position.related to prev + And I run :session-load -c foo + And I wait until data/numbers/3.txt is loaded + Then the following tabs should be open: + - data/numbers/1.txt + - data/numbers/2.txt + - data/numbers/3.txt (active) -- cgit v1.2.3-54-g00ecf