From 57155e329ada002245ab3fac45d906f6707c14cf Mon Sep 17 00:00:00 2001 From: mohite-abhi Date: Sun, 23 Jan 2022 19:06:47 +0530 Subject: [PATCH 01/11] Fixes qutebrowser/qutebrowser#6967 by adding win id param in _tabs & using it in delete_tabs As delete_tab was assuming that completion column contains window ID, it was showing exception in case of tab-focus, as it doesn't have the window ID in completion column. So instead a new parameter named current_win_id is used in _tabs which is also passed in all uses of the function. --- qutebrowser/completion/models/miscmodels.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index d8ebafb29..34948a452 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -103,17 +103,21 @@ def session(*, info=None): return model -def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True): +def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, current_win_id): """Helper to get the completion model for tabs/other_tabs. Args: win_id_filter: A filter function for window IDs to include. Should return True for all included windows. add_win_id: Whether to add the window ID to the completion items. + current_win_id: Window ID to be passed from info.win_id """ def delete_tab(data): """Close the selected tab.""" - win_id, tab_index = data[0].split('/') + + win_id = current_win_id + tab_index = data[0].split('/')[-1] # data[0] can be 'tabInd' or 'winID/tabInd' + tabbed_browser = objreg.get('tabbed-browser', scope='window', window=int(win_id)) tabbed_browser.on_tab_close_requested(int(tab_index) - 1) @@ -177,13 +181,13 @@ def other_tabs(*, info): Used for the tab-take command. """ - return _tabs(win_id_filter=lambda win_id: win_id != info.win_id) + return _tabs(win_id_filter=lambda win_id: win_id != info.win_id, current_win_id=info.win_id) def tab_focus(*, info): """A model to complete on open tabs in the current window.""" model = _tabs(win_id_filter=lambda win_id: win_id == info.win_id, - add_win_id=False) + add_win_id=False, current_win_id=info.win_id) special = [ ("last", "Focus the last-focused tab"), From 3b27bf9fb7b968025b9d348b311679c6111d3968 Mon Sep 17 00:00:00 2001 From: mohite-abhi Date: Sun, 23 Jan 2022 21:49:33 +0530 Subject: [PATCH 02/11] Fixed linter errors --- qutebrowser/completion/models/miscmodels.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index 34948a452..f461a0fbb 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -103,7 +103,7 @@ def session(*, info=None): return model -def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, current_win_id): +def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, current_win_id=0): """Helper to get the completion model for tabs/other_tabs. Args: @@ -114,7 +114,6 @@ def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, current_win_id """ def delete_tab(data): """Close the selected tab.""" - win_id = current_win_id tab_index = data[0].split('/')[-1] # data[0] can be 'tabInd' or 'winID/tabInd' From 426b280b1bb8fc7cc336d56e0ae7e9cca584ab89 Mon Sep 17 00:00:00 2001 From: mohite-abhi Date: Sun, 23 Jan 2022 22:33:27 +0530 Subject: [PATCH 03/11] fix more linter issues --- qutebrowser/completion/models/miscmodels.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index f461a0fbb..2b106d904 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -103,19 +103,20 @@ def session(*, info=None): return model -def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, current_win_id=0): +def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, cur_win_id=0): """Helper to get the completion model for tabs/other_tabs. Args: win_id_filter: A filter function for window IDs to include. Should return True for all included windows. add_win_id: Whether to add the window ID to the completion items. - current_win_id: Window ID to be passed from info.win_id + cur_win_id: Window ID to be passed from info.win_id """ def delete_tab(data): """Close the selected tab.""" - win_id = current_win_id - tab_index = data[0].split('/')[-1] # data[0] can be 'tabInd' or 'winID/tabInd' + win_id = cur_win_id + # data[0] can be 'tabInd' or 'winID/tabInd' + tab_index = data[0].split('/')[-1] tabbed_browser = objreg.get('tabbed-browser', scope='window', window=int(win_id)) @@ -180,13 +181,14 @@ def other_tabs(*, info): Used for the tab-take command. """ - return _tabs(win_id_filter=lambda win_id: win_id != info.win_id, current_win_id=info.win_id) + return _tabs(win_id_filter=lambda win_id: win_id != info.win_id, + cur_win_id=info.win_id) def tab_focus(*, info): """A model to complete on open tabs in the current window.""" model = _tabs(win_id_filter=lambda win_id: win_id == info.win_id, - add_win_id=False, current_win_id=info.win_id) + add_win_id=False, cur_win_id=info.win_id) special = [ ("last", "Focus the last-focused tab"), From d89d5853d393390b3a62c33084bf8150858a2fee Mon Sep 17 00:00:00 2001 From: mohite-abhi Date: Sun, 23 Jan 2022 23:43:20 +0530 Subject: [PATCH 04/11] Fix linter errors --- qutebrowser/completion/models/miscmodels.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index 2b106d904..0aa30d184 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -181,8 +181,9 @@ def other_tabs(*, info): Used for the tab-take command. """ - return _tabs(win_id_filter=lambda win_id: win_id != info.win_id, - cur_win_id=info.win_id) + return _tabs( + win_id_filter=lambda win_id: win_id != info.win_id, + cur_win_id=info.win_id) def tab_focus(*, info): From 7395a43e539abc43b3f9842fb620c170dcb67ad2 Mon Sep 17 00:00:00 2001 From: qutebrowser bot Date: Mon, 24 Jan 2022 04:21:13 +0000 Subject: [PATCH 05/11] Update dependencies --- misc/requirements/requirements-check-manifest.txt | 2 +- misc/requirements/requirements-dev.txt | 2 +- misc/requirements/requirements-sphinx.txt | 2 +- misc/requirements/requirements-tests.txt | 6 +++--- misc/requirements/requirements-tox.txt | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/misc/requirements/requirements-check-manifest.txt b/misc/requirements/requirements-check-manifest.txt index bd32971c2..9d42d3a5b 100644 --- a/misc/requirements/requirements-check-manifest.txt +++ b/misc/requirements/requirements-check-manifest.txt @@ -4,6 +4,6 @@ build==0.7.0 check-manifest==0.47 packaging==21.3 pep517==0.12.0 -pyparsing==3.0.6 +pyparsing==3.0.7 toml==0.10.2 tomli==2.0.0 diff --git a/misc/requirements/requirements-dev.txt b/misc/requirements/requirements-dev.txt index c075e7afc..93c9b707f 100644 --- a/misc/requirements/requirements-dev.txt +++ b/misc/requirements/requirements-dev.txt @@ -24,7 +24,7 @@ pkginfo==1.8.2 pycparser==2.21 Pygments==2.11.2 Pympler==1.0.1 -pyparsing==3.0.6 +pyparsing==3.0.7 PyQt-builder==1.12.2 python-dateutil==2.8.2 readme-renderer==32.0 diff --git a/misc/requirements/requirements-sphinx.txt b/misc/requirements/requirements-sphinx.txt index 1ad7e8e0b..ea4dd6430 100644 --- a/misc/requirements/requirements-sphinx.txt +++ b/misc/requirements/requirements-sphinx.txt @@ -12,7 +12,7 @@ Jinja2==3.0.3 MarkupSafe==2.0.1 packaging==21.3 Pygments==2.11.2 -pyparsing==3.0.6 +pyparsing==3.0.7 pytz==2021.3 requests==2.27.1 snowballstemmer==2.2.0 diff --git a/misc/requirements/requirements-tests.txt b/misc/requirements/requirements-tests.txt index 45a5144d3..40b653d97 100644 --- a/misc/requirements/requirements-tests.txt +++ b/misc/requirements/requirements-tests.txt @@ -13,7 +13,7 @@ filelock==3.4.2 ; python_version>="3.7" Flask==2.0.2 glob2==0.7 hunter==3.4.3 -hypothesis==6.35.1 ; python_version>="3.7" +hypothesis==6.36.0 ; python_version>="3.7" icdiff==2.0.4 idna==3.3 iniconfig==1.1.1 @@ -26,13 +26,13 @@ manhole==1.8.0 more-itertools==8.12.0 packaging==21.3 parse==1.19.0 -parse-type==0.5.2 +parse-type==0.6.0 pluggy==1.0.0 pprintpp==0.4.0 py==1.11.0 py-cpuinfo==8.0.0 Pygments==2.11.2 -pyparsing==3.0.6 +pyparsing==3.0.7 pytest==6.2.5 pytest-bdd==4.1.0 pytest-benchmark==3.4.1 diff --git a/misc/requirements/requirements-tox.txt b/misc/requirements/requirements-tox.txt index f348ac86e..1d7c20b8c 100644 --- a/misc/requirements/requirements-tox.txt +++ b/misc/requirements/requirements-tox.txt @@ -7,7 +7,7 @@ pip==21.3.1 platformdirs==2.4.1 ; python_version>="3.7" pluggy==1.0.0 py==1.11.0 -pyparsing==3.0.6 +pyparsing==3.0.7 setuptools==60.5.0 ; python_version>="3.7" six==1.16.0 toml==0.10.2 From ef87b5df725efe89cff68f8b4d1f60905a6f4b8f Mon Sep 17 00:00:00 2001 From: mohite-abhi Date: Tue, 25 Jan 2022 04:22:50 +0530 Subject: [PATCH 06/11] Add tests for tab focus completion delete --- qutebrowser/completion/models/miscmodels.py | 10 +++++--- tests/unit/completion/test_models.py | 28 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index 0aa30d184..77072c720 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -103,7 +103,7 @@ def session(*, info=None): return model -def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, cur_win_id=0): +def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, cur_win_id=None): """Helper to get the completion model for tabs/other_tabs. Args: @@ -114,9 +114,11 @@ def _tabs(*, win_id_filter=lambda _win_id: True, add_win_id=True, cur_win_id=0): """ def delete_tab(data): """Close the selected tab.""" - win_id = cur_win_id - # data[0] can be 'tabInd' or 'winID/tabInd' - tab_index = data[0].split('/')[-1] + if cur_win_id is None: + win_id, tab_index = data[0].split('/') + else: + win_id = cur_win_id + tab_index = data[0] tabbed_browser = objreg.get('tabbed-browser', scope='window', window=int(win_id)) diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index 9e6743083..b94c19a06 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -867,6 +867,34 @@ def test_tab_completion_delete(qtmodeltester, fake_web_tab, win_registry, QUrl('https://duckduckgo.com')] +def test_tab_focus_completion_delete(qtmodeltester, fake_web_tab, win_registry, + tabbed_browser_stubs, info): + """Verify closing a tab by deleting it from the completion widget.""" + tabbed_browser_stubs[0].widget.tabs = [ + fake_web_tab(QUrl('https://github.com'), 'GitHub', 0), + fake_web_tab(QUrl('https://wikipedia.org'), 'Wikipedia', 1), + fake_web_tab(QUrl('https://duckduckgo.com'), 'DuckDuckGo', 2) + ] + tabbed_browser_stubs[1].widget.tabs = [ + fake_web_tab(QUrl('https://wiki.archlinux.org'), 'ArchWiki', 0), + ] + model = miscmodels.tab_focus(info=info) + model.set_pattern('') + qtmodeltester.check(model) + + parent = model.index(0, 0) + idx = model.index(1, 0, parent) + + # # sanity checks + assert model.data(parent) == "Tabs" + assert model.data(idx) == '2' + + model.delete_cur_item(idx) + actual = [tab.url() for tab in tabbed_browser_stubs[0].widget.tabs] + assert actual == [QUrl('https://github.com'), + QUrl('https://duckduckgo.com')] + + def test_tab_completion_not_sorted(qtmodeltester, fake_web_tab, win_registry, tabbed_browser_stubs): """Ensure that the completion row order is the same as tab index order. From 02f331b89c7458d5ae0ccb973a0fdcfbbce21441 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 29 Jan 2022 09:37:53 +0100 Subject: [PATCH 07/11] tests: Update webserver started regex Needed for webserver_sub_ssl.py with bleeding edge CI as it excludes the trailing slash, probably recent Flask/Werkzeug change. --- tests/end2end/fixtures/webserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/fixtures/webserver.py b/tests/end2end/fixtures/webserver.py index 0fc32cd88..81f75b338 100644 --- a/tests/end2end/fixtures/webserver.py +++ b/tests/end2end/fixtures/webserver.py @@ -162,7 +162,7 @@ class WebserverProcess(testprocess.Process): def _parse_line(self, line): self._log(line) - started_re = re.compile(r' \* Running on https?://127\.0\.0\.1:{}/ ' + started_re = re.compile(r' \* Running on https?://127\.0\.0\.1:{}/? ' r'\(Press CTRL\+C to quit\)'.format(self.port)) if started_re.fullmatch(line): self.ready.emit() From fd277275f4e1829e1c9c6d485cd6967b43b3d54b Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 29 Jan 2022 10:26:23 +0100 Subject: [PATCH 08/11] Handle UnicodeDecodeError when reading faulthandler log --- doc/changelog.asciidoc | 2 ++ qutebrowser/misc/crashsignal.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index a446ec534..100d4151f 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -70,6 +70,8 @@ Fixed shown when closing the last window (rather than closing any window, which would continue running that window's downloads). Unfortunately, more issues with `confirm_quit` and multiple windows remain. +- Crash when a previous crash-log file contains non-ASCII characters (which + should never happen unless it was edited manually) [[v2.4.1]] v2.4.1 (unreleased) diff --git a/qutebrowser/misc/crashsignal.py b/qutebrowser/misc/crashsignal.py index f7578a07f..d94d3ec54 100644 --- a/qutebrowser/misc/crashsignal.py +++ b/qutebrowser/misc/crashsignal.py @@ -100,7 +100,7 @@ class CrashHandler(QObject): # There's no log file, so we can use this to display crashes to # the user on the next start. self._init_crashlogfile() - except OSError: + except (OSError, UnicodeDecodeError): log.init.exception("Error while handling crash log file!") self._init_crashlogfile() From 438b8b46094890a28db6bac07ff1ae67bbc5ee78 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 29 Jan 2022 11:31:58 +0100 Subject: [PATCH 09/11] Drop old Debian error page workaround Manual revert of d741bdf2f92b9dcf897b15c9d850fff6442f1cf7 See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882805 See #5078, #3773 --- doc/changelog.asciidoc | 3 +++ qutebrowser/browser/webengine/webenginetab.py | 17 +++++------------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index 100d4151f..b8870af02 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -72,6 +72,9 @@ Fixed with `confirm_quit` and multiple windows remain. - Crash when a previous crash-log file contains non-ASCII characters (which should never happen unless it was edited manually) +- Due to changes in Debian, an old workaround (for broken QtWebEngine patching + on Debian) caused the inferior qutebrowser error page to be displayed, when + Chromium's would have worked fine. The workaround was now dropped. [[v2.4.1]] v2.4.1 (unreleased) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index fa877f560..7fa0b2b65 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -1522,15 +1522,14 @@ class WebEngineTab(browsertab.AbstractTab): } self.renderer_process_terminated.emit(status_map[status], exitcode) - def _error_page_workaround(self, js_enabled, html): + def _error_page_workaround(self, html): """Check if we're displaying a Chromium error page. - This gets called if we got a loadFinished(False), so we can display at - least some error page in situations where Chromium's can't be + This gets called if we got a loadFinished(False) without JavaScript, so + we can display at least some error page, since Chromium's can't be displayed. WORKAROUND for https://bugreports.qt.io/browse/QTBUG-66643 - WORKAROUND for https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882805 """ match = re.search(r'"errorCode":"([^"]*)"', html) if match is None: @@ -1538,11 +1537,6 @@ class WebEngineTab(browsertab.AbstractTab): error = match.group(1) log.webview.error("Load error: {}".format(error)) - - missing_jst = 'jstProcess(' in html and 'jstProcess=' not in html - if js_enabled and not missing_jst: - return - self._show_error_page(self.url(), error=error) @pyqtSlot(int) @@ -1565,9 +1559,8 @@ class WebEngineTab(browsertab.AbstractTab): # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-65223 self._update_load_status(ok) - self.dump_async(functools.partial( - self._error_page_workaround, - self.settings.test_attribute('content.javascript.enabled'))) + if not self.settings.test_attribute('content.javascript.enabled'): + self.dump_async(self._error_page_workaround) @pyqtSlot(certificateerror.CertificateErrorWrapper) def _on_ssl_errors(self, error): From 2dc11c128a67b25029827cb412a37aefb67b52e5 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 29 Jan 2022 11:38:14 +0100 Subject: [PATCH 10/11] Update docs/changelog --- doc/changelog.asciidoc | 2 ++ tests/unit/completion/test_models.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index b8870af02..b07b64fa6 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -75,6 +75,8 @@ Fixed - Due to changes in Debian, an old workaround (for broken QtWebEngine patching on Debian) caused the inferior qutebrowser error page to be displayed, when Chromium's would have worked fine. The workaround was now dropped. +- Crash when using `` (`:completion-item-del`) in the `:tab-focus` + list, rather than `:tab-select`. [[v2.4.1]] v2.4.1 (unreleased) diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index b94c19a06..2c00acf68 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -885,7 +885,7 @@ def test_tab_focus_completion_delete(qtmodeltester, fake_web_tab, win_registry, parent = model.index(0, 0) idx = model.index(1, 0, parent) - # # sanity checks + # sanity checks assert model.data(parent) == "Tabs" assert model.data(idx) == '2' From 86b5bed388544d2d445a3dba151e3c3a4c8814b7 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Sat, 29 Jan 2022 13:31:04 +0100 Subject: [PATCH 11/11] Partially re-revert _error_page_workaround changes The logging part removed in 438b8b46094890a28db6bac07ff1ae67bbc5ee78 is still needed for some tests, and debugging too. --- qutebrowser/browser/webengine/webenginetab.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 7fa0b2b65..7d355d10e 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -1522,11 +1522,11 @@ class WebEngineTab(browsertab.AbstractTab): } self.renderer_process_terminated.emit(status_map[status], exitcode) - def _error_page_workaround(self, html): + def _error_page_workaround(self, js_enabled, html): """Check if we're displaying a Chromium error page. - This gets called if we got a loadFinished(False) without JavaScript, so - we can display at least some error page, since Chromium's can't be + This gets called if we got a loadFinished(False), so we can display at + least some error page in situations where Chromium's can't be displayed. WORKAROUND for https://bugreports.qt.io/browse/QTBUG-66643 @@ -1537,6 +1537,10 @@ class WebEngineTab(browsertab.AbstractTab): error = match.group(1) log.webview.error("Load error: {}".format(error)) + + if js_enabled: + return + self._show_error_page(self.url(), error=error) @pyqtSlot(int) @@ -1559,8 +1563,9 @@ class WebEngineTab(browsertab.AbstractTab): # WORKAROUND for https://bugreports.qt.io/browse/QTBUG-65223 self._update_load_status(ok) - if not self.settings.test_attribute('content.javascript.enabled'): - self.dump_async(self._error_page_workaround) + self.dump_async(functools.partial( + self._error_page_workaround, + self.settings.test_attribute('content.javascript.enabled'))) @pyqtSlot(certificateerror.CertificateErrorWrapper) def _on_ssl_errors(self, error):