From 583c7e41983d5118248e0d5f317f9114787c6e32 Mon Sep 17 00:00:00 2001 From: Jesko Date: Wed, 15 Aug 2018 00:30:54 +0200 Subject: [PATCH 01/13] catch the exception --- qutebrowser/browser/commands.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index d5e1797ac..9b779be4a 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -2085,7 +2085,10 @@ class CommandDispatcher: raise cmdexc.CommandError(str(e)) widget = self._current_widget() - widget.run_js_async(js_code, callback=jseval_cb, world=world) + try: + widget.run_js_async(js_code, callback=jseval_cb, world=world) + except OverflowError as e: + raise cmdexc.CommandError("World Id not in valid range") @cmdutils.register(instance='command-dispatcher', scope='window') def fake_key(self, keystring, global_=False): From 8aa682e76993d72305c3bb524446dae9422ed740 Mon Sep 17 00:00:00 2001 From: Jesko Date: Wed, 15 Aug 2018 22:52:47 +0200 Subject: [PATCH 02/13] catching greasemonkey errors aswell --- qutebrowser/browser/commands.py | 2 +- qutebrowser/browser/webengine/webenginetab.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 9b779be4a..794497ed9 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -2087,7 +2087,7 @@ class CommandDispatcher: widget = self._current_widget() try: widget.run_js_async(js_code, callback=jseval_cb, world=world) - except OverflowError as e: + except OverflowError: raise cmdexc.CommandError("World Id not in valid range") @cmdutils.register(instance='command-dispatcher', scope='window') diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index cfb809097..d1935d125 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -948,6 +948,11 @@ class _WebEngineScripts(QObject): new_script = QWebEngineScript() try: world = int(script.jsworld) + if not 0 <= world < 256: + log.greasemonkey.error( + "script {} has invalid value for '@qute-js-world'" + ": {}".format(script.name, script.jsworld)) + continue except ValueError: try: world = _JS_WORLD_MAP[usertypes.JsWorld[ From 0c38cbcbdd65e20778e0bcc2ff0a9b1849feb7f6 Mon Sep 17 00:00:00 2001 From: Jesko Date: Wed, 15 Aug 2018 23:06:45 +0200 Subject: [PATCH 03/13] putting in current borders --- qutebrowser/browser/webengine/webenginetab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index d1935d125..143926ee3 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -948,7 +948,7 @@ class _WebEngineScripts(QObject): new_script = QWebEngineScript() try: world = int(script.jsworld) - if not 0 <= world < 256: + if not 0 <= world <= 11: log.greasemonkey.error( "script {} has invalid value for '@qute-js-world'" ": {}".format(script.name, script.jsworld)) From ee9b1f4950564d1a25abb65345790b83e51f2973 Mon Sep 17 00:00:00 2001 From: Jesko Date: Thu, 16 Aug 2018 12:29:27 +0200 Subject: [PATCH 04/13] adding tests and improving error messages --- qutebrowser/browser/commands.py | 4 +++- qutebrowser/browser/webengine/webenginetab.py | 11 ++++++++-- tests/end2end/features/misc.feature | 20 +++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 794497ed9..ae5e38e22 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -41,6 +41,8 @@ from qutebrowser.misc import editor, guiprocess from qutebrowser.completion.models import urlmodel, miscmodels from qutebrowser.mainwindow import mainwindow +# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904 +MAX_WORLD_ID = 256 if qtutils.version_check('5.11.2') else 11 class CommandDispatcher: @@ -2088,7 +2090,7 @@ class CommandDispatcher: try: widget.run_js_async(js_code, callback=jseval_cb, world=world) except OverflowError: - raise cmdexc.CommandError("World Id not in valid range") + raise cmdexc.CommandError("World ID should be between 0 and " + str(MAX_WORLD_ID)) @cmdutils.register(instance='command-dispatcher', scope='window') def fake_key(self, keystring, global_=False): diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 143926ee3..7a096dd8f 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -47,6 +47,10 @@ from qutebrowser.qt import sip _qute_scheme_handler = None +# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904 +MAX_WORLD_ID = 256 if qtutils.version_check('5.11.2') else 11 + + def init(): """Initialize QtWebEngine-specific modules.""" # For some reason we need to keep a reference, otherwise the scheme handler @@ -948,10 +952,11 @@ class _WebEngineScripts(QObject): new_script = QWebEngineScript() try: world = int(script.jsworld) - if not 0 <= world <= 11: + if not 0 <= world <= MAX_WORLD_ID: log.greasemonkey.error( "script {} has invalid value for '@qute-js-world'" - ": {}".format(script.name, script.jsworld)) + ": {}, should be between 0 and " + "{}".format(script.name, script.jsworld, MAX_WORLD_ID)) continue except ValueError: try: @@ -1070,6 +1075,8 @@ class WebEngineTab(browsertab.AbstractTab): world_id = QWebEngineScript.ApplicationWorld elif isinstance(world, int): world_id = world + if not 0 <= world_id <= MAX_WORLD_ID: + raise OverflowError else: world_id = _JS_WORLD_MAP[world] diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 5f0035b8b..254ab7150 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -118,6 +118,26 @@ Feature: Various utility commands. Then the javascript message "Hello from the page!" should be logged And "No output or error" should be logged + @qtwebkit_skip @qt>=5.11.2 + Scenario: :jseval using too high of a world id + When I run :jseval --world=257 console.log("Hello from JS!"); + Then the error "World ID should be between 0 and 256" should be shown + + @qtwebkit_skip @qt<5.11.2 + Scenario: :jseval using too high of a world id + When I run :jseval --world=12 console.log("Hello from JS!"); + Then the error "World ID should be between 0 and 11" should be shown + + @qtwebkit_skip @qt>=5.11.2 + Scenario: :jseval using a negative world id + When I run :jseval --world=-1 console.log("Hello from JS!"); + Then the error "World ID should be between 0 and 256" should be shown + + @qtwebkit_skip @qt<5.11.2 + Scenario: :jseval using a negative world id + When I run :jseval --world=-1 console.log("Hello from JS!"); + Then the error "World ID should be between 0 and 11" should be shown + Scenario: :jseval --file using a file that exists as js-code When I run :jseval --file (testdata)/misc/jseval_file.js Then the javascript message "Hello from JS!" should be logged From 8ef3d90b1ae9770d545ef4e4393630e798239d70 Mon Sep 17 00:00:00 2001 From: Jesko Date: Thu, 16 Aug 2018 12:33:07 +0200 Subject: [PATCH 05/13] removing unwanted space --- tests/end2end/features/misc.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index 254ab7150..ac3788d8e 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -122,7 +122,7 @@ Feature: Various utility commands. Scenario: :jseval using too high of a world id When I run :jseval --world=257 console.log("Hello from JS!"); Then the error "World ID should be between 0 and 256" should be shown - + @qtwebkit_skip @qt<5.11.2 Scenario: :jseval using too high of a world id When I run :jseval --world=12 console.log("Hello from JS!"); From abea603119f3cbda81bf4b4be34e6df2bfbf76b3 Mon Sep 17 00:00:00 2001 From: Jesko Date: Sat, 18 Aug 2018 14:19:05 +0200 Subject: [PATCH 06/13] moving MAX_WORLD_ID to qtutils, changing test names, fixing linter errors, changing error type to WebTabError --- qutebrowser/browser/commands.py | 6 ++---- qutebrowser/browser/webengine/webenginetab.py | 19 ++++++++++--------- qutebrowser/utils/qtutils.py | 5 +++++ tests/end2end/features/misc.feature | 8 ++++---- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index ae5e38e22..838862038 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -41,8 +41,6 @@ from qutebrowser.misc import editor, guiprocess from qutebrowser.completion.models import urlmodel, miscmodels from qutebrowser.mainwindow import mainwindow -# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904 -MAX_WORLD_ID = 256 if qtutils.version_check('5.11.2') else 11 class CommandDispatcher: @@ -2089,8 +2087,8 @@ class CommandDispatcher: widget = self._current_widget() try: widget.run_js_async(js_code, callback=jseval_cb, world=world) - except OverflowError: - raise cmdexc.CommandError("World ID should be between 0 and " + str(MAX_WORLD_ID)) + except browsertab.WebTabError as e: + raise cmdexc.CommandError(str(e)) @cmdutils.register(instance='command-dispatcher', scope='window') def fake_key(self, keystring, global_=False): diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 7a096dd8f..85c3879d4 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -47,10 +47,6 @@ from qutebrowser.qt import sip _qute_scheme_handler = None -# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904 -MAX_WORLD_ID = 256 if qtutils.version_check('5.11.2') else 11 - - def init(): """Initialize QtWebEngine-specific modules.""" # For some reason we need to keep a reference, otherwise the scheme handler @@ -952,11 +948,14 @@ class _WebEngineScripts(QObject): new_script = QWebEngineScript() try: world = int(script.jsworld) - if not 0 <= world <= MAX_WORLD_ID: + if not 0 <= world <= qtutils.MAX_WORLD_ID: log.greasemonkey.error( "script {} has invalid value for '@qute-js-world'" - ": {}, should be between 0 and " - "{}".format(script.name, script.jsworld, MAX_WORLD_ID)) + ": {}, should be between 0 and {}" + .format( + script.name, + script.jsworld, + qtutils.MAX_WORLD_ID)) continue except ValueError: try: @@ -1075,8 +1074,10 @@ class WebEngineTab(browsertab.AbstractTab): world_id = QWebEngineScript.ApplicationWorld elif isinstance(world, int): world_id = world - if not 0 <= world_id <= MAX_WORLD_ID: - raise OverflowError + if not 0 <= world_id <= qtutils.MAX_WORLD_ID: + raise browsertab.WebTabError( + "World ID should be between 0 and {}" + .format(qtutils.MAX_WORLD_ID)) else: world_id = _JS_WORLD_MAP[world] diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index 8a42fb073..1f87d1a90 100644 --- a/qutebrowser/utils/qtutils.py +++ b/qutebrowser/utils/qtutils.py @@ -24,6 +24,7 @@ Module attributes: value. MINVALS: A dictionary of C/Qt types (as string) mapped to their minimum value. + MAX_WORLD_ID: The highest World ID allowed in this version of QtWebEngine """ @@ -98,6 +99,10 @@ def version_check(version, exact=False, compiled=True): return result +# WORKAROUND for https://bugreports.qt.io/browse/QTBUG-69904 +MAX_WORLD_ID = 256 if version_check('5.11.2') else 11 + + def is_new_qtwebkit(): """Check if the given version is a new QtWebKit.""" assert qWebKitVersion is not None diff --git a/tests/end2end/features/misc.feature b/tests/end2end/features/misc.feature index ac3788d8e..6893e143a 100644 --- a/tests/end2end/features/misc.feature +++ b/tests/end2end/features/misc.feature @@ -119,22 +119,22 @@ Feature: Various utility commands. And "No output or error" should be logged @qtwebkit_skip @qt>=5.11.2 - Scenario: :jseval using too high of a world id + Scenario: :jseval using too high of a world id in Qt versions bigger than 5.11.2 When I run :jseval --world=257 console.log("Hello from JS!"); Then the error "World ID should be between 0 and 256" should be shown @qtwebkit_skip @qt<5.11.2 - Scenario: :jseval using too high of a world id + Scenario: :jseval using too high of a world id in Qt versions smaller than 5.11.2 When I run :jseval --world=12 console.log("Hello from JS!"); Then the error "World ID should be between 0 and 11" should be shown @qtwebkit_skip @qt>=5.11.2 - Scenario: :jseval using a negative world id + Scenario: :jseval using a negative world id in Qt versions bigger than 5.11.2 When I run :jseval --world=-1 console.log("Hello from JS!"); Then the error "World ID should be between 0 and 256" should be shown @qtwebkit_skip @qt<5.11.2 - Scenario: :jseval using a negative world id + Scenario: :jseval using a negative world id in Qt versions smaller than 5.11.2 When I run :jseval --world=-1 console.log("Hello from JS!"); Then the error "World ID should be between 0 and 11" should be shown From 32268ae66a116aa2e1bab4a9826365a7b0c3f3cc Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Sep 2018 14:00:53 +1200 Subject: [PATCH 07/13] Split _inject_greasemonkey_scripts to separate requirements. Because flake8 was complaining about cyclomatic complexity. --- qutebrowser/browser/webengine/webenginetab.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 85c3879d4..8fb7e4c9c 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -911,6 +911,15 @@ class _WebEngineScripts(QObject): scripts = self._greasemonkey.all_scripts() self._inject_greasemonkey_scripts(scripts) + def _remove_all_greasemonkey_scripts(self): + page_scripts = self._widget.page().scripts() + for script in page_scripts.toList(): + if script.name().startswith("GM-"): + log.greasemonkey.debug('Removing script: {}' + .format(script.name())) + removed = page_scripts.remove(script) + assert removed, script.name() + def _inject_greasemonkey_scripts(self, scripts=None, injection_point=None, remove_first=True): """Register user JavaScript files with the current tab. @@ -934,12 +943,7 @@ class _WebEngineScripts(QObject): # have been added elsewhere, like the one for stylesheets. page_scripts = self._widget.page().scripts() if remove_first: - for script in page_scripts.toList(): - if script.name().startswith("GM-"): - log.greasemonkey.debug('Removing script: {}' - .format(script.name())) - removed = page_scripts.remove(script) - assert removed, script.name() + self._remove_all_greasemonkey_scripts() if not scripts: return From 5252541fe340628300a2c934a8956291d890b76a Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Sep 2018 14:27:37 +1200 Subject: [PATCH 08/13] greasemonkey: better handle scripts without metadata Previously calling `script.code()` would fail if the script didn't have a `name`. This wasn't being hit in practice because the only place that constructs GreasemonkeyScripts was checking for that condition and add the filename there as a fallback. This change make the `name` attribute more explicitly mandatory by failing with a `ValueError` if it is not provided and make it still possible to use the filename fallback in that case by adding a `filename` keyward argument to `__init__()`. Additionally where `script_meta` is used in `script.code()` a fallback to and emptry string was added so it doesn't fail for raw javascript files without greasemonkey metadata. --- qutebrowser/browser/greasemonkey.py | 28 ++++++++++++++++------ tests/unit/javascript/test_greasemonkey.py | 15 ++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/qutebrowser/browser/greasemonkey.py b/qutebrowser/browser/greasemonkey.py index 7e5ff7f5b..7606e6d64 100644 --- a/qutebrowser/browser/greasemonkey.py +++ b/qutebrowser/browser/greasemonkey.py @@ -46,7 +46,8 @@ class GreasemonkeyScript: """Container class for userscripts, parses metadata blocks.""" - def __init__(self, properties, code): + def __init__(self, properties, code, # noqa: C901 pragma: no mccabe + filename=None): self._code = code self.includes = [] self.matches = [] @@ -81,11 +82,19 @@ class GreasemonkeyScript: elif name == 'qute-js-world': self.jsworld = value + if not self.name: + if filename: + self.name = filename + else: + raise ValueError( + "@name key required or pass filename to init." + ) + HEADER_REGEX = r'// ==UserScript==|\n+// ==/UserScript==\n' PROPS_REGEX = r'// @(?P[^\s]+)\s*(?P.*)' @classmethod - def parse(cls, source): + def parse(cls, source, filename=None): """GreasemonkeyScript factory. Takes a userscript source and returns a GreasemonkeyScript. @@ -97,7 +106,11 @@ class GreasemonkeyScript: _head, props, _code = matches except ValueError: props = "" - script = cls(re.findall(cls.PROPS_REGEX, props), source) + script = cls( + re.findall(cls.PROPS_REGEX, props), + source, + filename=filename + ) script.script_meta = props if not script.includes and not script.matches: script.includes = ['*'] @@ -121,7 +134,7 @@ class GreasemonkeyScript: scriptName=javascript.string_escape( "/".join([self.namespace or '', self.name])), scriptInfo=self._meta_json(), - scriptMeta=javascript.string_escape(self.script_meta), + scriptMeta=javascript.string_escape(self.script_meta or ''), scriptSource=self._code, use_proxy=use_proxy) @@ -235,9 +248,10 @@ class GreasemonkeyManager(QObject): continue script_path = os.path.join(scripts_dir, script_filename) with open(script_path, encoding='utf-8') as script_file: - script = GreasemonkeyScript.parse(script_file.read()) - if not script.name: - script.name = script_filename + script = GreasemonkeyScript.parse( + script_file.read(), + filename=script_filename, + ) self.add_script(script, force) self.scripts_reloaded.emit() diff --git a/tests/unit/javascript/test_greasemonkey.py b/tests/unit/javascript/test_greasemonkey.py index 15fa4321e..e85edd15b 100644 --- a/tests/unit/javascript/test_greasemonkey.py +++ b/tests/unit/javascript/test_greasemonkey.py @@ -113,6 +113,21 @@ def test_no_metadata(caplog): assert len(scripts.end) == 1 +def test_no_name(): + """Ensure that GreaseMonkeyScripts must have a name.""" + msg = "@name key required or pass filename to init." + with pytest.raises(ValueError, match=msg): + greasemonkey.GreasemonkeyScript([("something", "else")], "") + + +def test_no_name_with_fallback(): + """Ensure that script's name can fallback to the provided filename.""" + script = greasemonkey.GreasemonkeyScript( + [("something", "else")], "", filename=r"C:\COM1") + assert script + assert script.name == r"C:\COM1" + + def test_bad_scheme(caplog): """qute:// isn't in the list of allowed schemes.""" _save_script("var nothing = true;\n", 'nothing.user.js') From 7a73c0f264b0553079a308b65c30093cfe8e4e15 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Sep 2018 14:35:00 +1200 Subject: [PATCH 09/13] Add tests for greasemonkey worldid in webenginetab. Adds a new file for tests relating to WebEngineTab, it looks a little sparse right now but I'm sure there will be more soon. Tests that scripts with invalid world IDs are rejected appropriately and the ones with valid world IDs aren't rejected. The `FakeWidget` thing is so `sip.isdeleted` didn't complain about being passed the wrong type. --- .../browser/webengine/test_webenginetab.py | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/unit/browser/webengine/test_webenginetab.py diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py new file mode 100644 index 000000000..d6ecc2b5e --- /dev/null +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -0,0 +1,107 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2018 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with qutebrowser. If not, see . + +"""Test webenginetab.""" + +from unittest import mock +import logging + +from PyQt5.QtCore import QObject +from PyQt5.QtWebEngineWidgets import QWebEnginePage, QWebEngineScriptCollection +import pytest + +from qutebrowser.browser.webengine.webenginetab import _WebEngineScripts +from qutebrowser.browser.greasemonkey import GreasemonkeyScript + +pytestmark = pytest.mark.usefixtures('greasemonkey_manager') + + +class TestWebengineScripts: + """Test the _WebEngineScripts utility class.""" + + class FakeWidget(QObject): + """Fake widget for _WebengineScripts to use.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.scripts = [] + self.page = mock.create_autospec(QWebEnginePage) + self.scripts_mock = mock.create_autospec( + QWebEngineScriptCollection + ) + self.scripts_mock.toList.return_value = self.scripts + self.page.return_value.scripts.return_value = self.scripts_mock + + def test_greasemonkey_undefined_world(self, fake_web_tab, caplog): + """Make sure scripts with non-existent worlds are rejected.""" + uut = _WebEngineScripts(fake_web_tab) + uut._widget = self.FakeWidget() + scripts = [ + GreasemonkeyScript([('qute-js-world', 'Mars')], None) + ] + + with caplog.at_level(logging.ERROR, 'greasemonkey'): + uut._inject_greasemonkey_scripts(scripts) + + assert len(caplog.records) == 1 + msg = caplog.records[0].message + assert "has invalid value for '@qute-js-world': Mars" in msg + uut._widget.scripts_mock.insert.assert_not_called() + + @pytest.mark.parametrize("worldid", [ + -1, 257 + ]) + def test_greasemonkey_out_of_range_world(self, worldid, + fake_web_tab, caplog): + """Make sure scripts with out-of-range worlds are rejected.""" + uut = _WebEngineScripts(fake_web_tab) + uut._widget = self.FakeWidget() + scripts = [ + GreasemonkeyScript([('qute-js-world', worldid)], None) + ] + + with caplog.at_level(logging.ERROR, 'greasemonkey'): + uut._inject_greasemonkey_scripts(scripts) + + assert len(caplog.records) == 1 + msg = caplog.records[0].message + assert "has invalid value for '@qute-js-world': " in msg + assert "should be between 0 and" in msg + uut._widget.scripts_mock.insert.assert_not_called() + + @pytest.mark.parametrize("worldid", [ + 0, 10 + ]) + def test_greasemonkey_good_worlds_are_passed(self, worldid, + fake_web_tab, caplog): + """Make sure scripts with valid worlds have it set.""" + uut = _WebEngineScripts(fake_web_tab) + uut._widget = self.FakeWidget() + scripts = [ + GreasemonkeyScript( + [('name', 'foo'), ('qute-js-world', worldid)], None + ) + ] + + with caplog.at_level(logging.ERROR, 'greasemonkey'): + uut._inject_greasemonkey_scripts(scripts) + + calls = uut._widget.scripts_mock.insert.call_args_list + assert len(calls) == 1 + assert calls[0][0][0].worldId() == worldid From f22fb30ef39699c2b76bc81ad8d38be8babf4a3c Mon Sep 17 00:00:00 2001 From: Jesko Date: Wed, 5 Sep 2018 13:54:58 +0200 Subject: [PATCH 10/13] fixing non controversial changes --- qutebrowser/utils/qtutils.py | 2 +- .../browser/webengine/test_webenginetab.py | 24 ++++++++----------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/qutebrowser/utils/qtutils.py b/qutebrowser/utils/qtutils.py index 1f87d1a90..c634eb95f 100644 --- a/qutebrowser/utils/qtutils.py +++ b/qutebrowser/utils/qtutils.py @@ -24,7 +24,7 @@ Module attributes: value. MINVALS: A dictionary of C/Qt types (as string) mapped to their minimum value. - MAX_WORLD_ID: The highest World ID allowed in this version of QtWebEngine + MAX_WORLD_ID: The highest world ID allowed in this version of QtWebEngine. """ diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py index d6ecc2b5e..fd2d7bf61 100644 --- a/tests/unit/browser/webengine/test_webenginetab.py +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -26,8 +26,8 @@ from PyQt5.QtCore import QObject from PyQt5.QtWebEngineWidgets import QWebEnginePage, QWebEngineScriptCollection import pytest -from qutebrowser.browser.webengine.webenginetab import _WebEngineScripts -from qutebrowser.browser.greasemonkey import GreasemonkeyScript +from qutebrowser.browser.webengine import webenginetab +from qutebrowser.browser import greasemonkey pytestmark = pytest.mark.usefixtures('greasemonkey_manager') @@ -50,10 +50,10 @@ class TestWebengineScripts: def test_greasemonkey_undefined_world(self, fake_web_tab, caplog): """Make sure scripts with non-existent worlds are rejected.""" - uut = _WebEngineScripts(fake_web_tab) + uut = webenginetab._WebEngineScripts(fake_web_tab) uut._widget = self.FakeWidget() scripts = [ - GreasemonkeyScript([('qute-js-world', 'Mars')], None) + greasemonkey.GreasemonkeyScript([('qute-js-world', 'Mars')], None) ] with caplog.at_level(logging.ERROR, 'greasemonkey'): @@ -64,16 +64,14 @@ class TestWebengineScripts: assert "has invalid value for '@qute-js-world': Mars" in msg uut._widget.scripts_mock.insert.assert_not_called() - @pytest.mark.parametrize("worldid", [ - -1, 257 - ]) + @pytest.mark.parametrize("worldid", [-1, 257]) def test_greasemonkey_out_of_range_world(self, worldid, fake_web_tab, caplog): """Make sure scripts with out-of-range worlds are rejected.""" - uut = _WebEngineScripts(fake_web_tab) + uut = webenginetab._WebEngineScripts(fake_web_tab) uut._widget = self.FakeWidget() scripts = [ - GreasemonkeyScript([('qute-js-world', worldid)], None) + greasemonkey.GreasemonkeyScript([('qute-js-world', worldid)], None) ] with caplog.at_level(logging.ERROR, 'greasemonkey'): @@ -85,16 +83,14 @@ class TestWebengineScripts: assert "should be between 0 and" in msg uut._widget.scripts_mock.insert.assert_not_called() - @pytest.mark.parametrize("worldid", [ - 0, 10 - ]) + @pytest.mark.parametrize("worldid", [0, 10]) def test_greasemonkey_good_worlds_are_passed(self, worldid, fake_web_tab, caplog): """Make sure scripts with valid worlds have it set.""" - uut = _WebEngineScripts(fake_web_tab) + uut = webenginetab._WebEngineScripts(fake_web_tab) uut._widget = self.FakeWidget() scripts = [ - GreasemonkeyScript( + greasemonkey.GreasemonkeyScript( [('name', 'foo'), ('qute-js-world', worldid)], None ) ] From 02f485cbc8b8745ee1f581ffc68aecd40a01722f Mon Sep 17 00:00:00 2001 From: Jesko Date: Fri, 24 Aug 2018 12:15:27 +0200 Subject: [PATCH 11/13] making import of QtWebEngine in corresponding tests conditional --- tests/unit/browser/webengine/test_webenginetab.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py index fd2d7bf61..605eff75b 100644 --- a/tests/unit/browser/webengine/test_webenginetab.py +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -23,8 +23,10 @@ from unittest import mock import logging from PyQt5.QtCore import QObject -from PyQt5.QtWebEngineWidgets import QWebEnginePage, QWebEngineScriptCollection import pytest +QtWebEngineWidgets = pytest.importorskip("PyQt5.QtWebEngineWidgets") +QWebEnginePage = QtWebEngineWidgets.QWebEnginePage +QWebEngineScriptCollection = QtWebEngineWidgets.QWebEngineScriptCollection from qutebrowser.browser.webengine import webenginetab from qutebrowser.browser import greasemonkey From 33cc8c11ba40f2c8cb1b1044c606b0653a3dfe75 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sun, 9 Sep 2018 15:02:42 +1200 Subject: [PATCH 12/13] Pull common code into a fixture. --- .../browser/webengine/test_webenginetab.py | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/tests/unit/browser/webengine/test_webenginetab.py b/tests/unit/browser/webengine/test_webenginetab.py index 605eff75b..1828d9c47 100644 --- a/tests/unit/browser/webengine/test_webenginetab.py +++ b/tests/unit/browser/webengine/test_webenginetab.py @@ -50,47 +50,49 @@ class TestWebengineScripts: self.scripts_mock.toList.return_value = self.scripts self.page.return_value.scripts.return_value = self.scripts_mock - def test_greasemonkey_undefined_world(self, fake_web_tab, caplog): + @pytest.fixture + def mocked_scripts(self, fake_web_tab): + scripts = webenginetab._WebEngineScripts(fake_web_tab) + scripts._widget = self.FakeWidget() + return scripts + + def test_greasemonkey_undefined_world(self, mocked_scripts, caplog): """Make sure scripts with non-existent worlds are rejected.""" - uut = webenginetab._WebEngineScripts(fake_web_tab) - uut._widget = self.FakeWidget() scripts = [ - greasemonkey.GreasemonkeyScript([('qute-js-world', 'Mars')], None) + greasemonkey.GreasemonkeyScript( + [('qute-js-world', 'Mars'), ('name', 'test')], None) ] with caplog.at_level(logging.ERROR, 'greasemonkey'): - uut._inject_greasemonkey_scripts(scripts) + mocked_scripts._inject_greasemonkey_scripts(scripts) assert len(caplog.records) == 1 msg = caplog.records[0].message assert "has invalid value for '@qute-js-world': Mars" in msg - uut._widget.scripts_mock.insert.assert_not_called() + mocked_scripts._widget.scripts_mock.insert.assert_not_called() @pytest.mark.parametrize("worldid", [-1, 257]) def test_greasemonkey_out_of_range_world(self, worldid, - fake_web_tab, caplog): + mocked_scripts, caplog): """Make sure scripts with out-of-range worlds are rejected.""" - uut = webenginetab._WebEngineScripts(fake_web_tab) - uut._widget = self.FakeWidget() scripts = [ - greasemonkey.GreasemonkeyScript([('qute-js-world', worldid)], None) + greasemonkey.GreasemonkeyScript( + [('qute-js-world', worldid), ('name', 'test')], None) ] with caplog.at_level(logging.ERROR, 'greasemonkey'): - uut._inject_greasemonkey_scripts(scripts) + mocked_scripts._inject_greasemonkey_scripts(scripts) assert len(caplog.records) == 1 msg = caplog.records[0].message assert "has invalid value for '@qute-js-world': " in msg assert "should be between 0 and" in msg - uut._widget.scripts_mock.insert.assert_not_called() + mocked_scripts._widget.scripts_mock.insert.assert_not_called() @pytest.mark.parametrize("worldid", [0, 10]) def test_greasemonkey_good_worlds_are_passed(self, worldid, - fake_web_tab, caplog): + mocked_scripts, caplog): """Make sure scripts with valid worlds have it set.""" - uut = webenginetab._WebEngineScripts(fake_web_tab) - uut._widget = self.FakeWidget() scripts = [ greasemonkey.GreasemonkeyScript( [('name', 'foo'), ('qute-js-world', worldid)], None @@ -98,8 +100,8 @@ class TestWebengineScripts: ] with caplog.at_level(logging.ERROR, 'greasemonkey'): - uut._inject_greasemonkey_scripts(scripts) + mocked_scripts._inject_greasemonkey_scripts(scripts) - calls = uut._widget.scripts_mock.insert.call_args_list + calls = mocked_scripts._widget.scripts_mock.insert.call_args_list assert len(calls) == 1 assert calls[0][0][0].worldId() == worldid From bb35285914010f5754506fe56c5dbe76a571257c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 5 Oct 2018 19:25:19 +0200 Subject: [PATCH 13/13] Fix passing filename to GreasemonkeyScript --- qutebrowser/browser/greasemonkey.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/greasemonkey.py b/qutebrowser/browser/greasemonkey.py index b10c213db..a2ae73aab 100644 --- a/qutebrowser/browser/greasemonkey.py +++ b/qutebrowser/browser/greasemonkey.py @@ -248,7 +248,8 @@ class GreasemonkeyManager(QObject): continue script_path = os.path.join(scripts_dir, script_filename) with open(script_path, encoding='utf-8-sig') as script_file: - script = GreasemonkeyScript.parse(script_file.read()) + script = GreasemonkeyScript.parse(script_file.read(), + script_filename) if not script.name: script.name = script_filename self.add_script(script, force)