From 13249329f70830b25a4ee736f78f7b304deaf010 Mon Sep 17 00:00:00 2001
From: Jimmy <jimmy@spalge.com>
Date: Wed, 25 Apr 2018 16:26:43 +1200
Subject: [PATCH] Greasemonkey: skip window scoping test on webkit

The implementation of Proxy in JSCore used by current QtWebkit (webkit
602.1) doesn't support the `set()` handler for whatever reason. So
instead of testing for a specific behaviour that we can't ensure on that
version let's just skip the tests and handle user complaints with
sympathy.
---
 pytest.ini                                    |  1 +
 .../javascript/greasemonkey_wrapper.js        | 29 ++++++++-----------
 tests/conftest.py                             |  6 +++-
 tests/unit/javascript/test_greasemonkey.py    |  2 ++
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/pytest.ini b/pytest.ini
index 1a3f625e9..c897f0be7 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -30,6 +30,7 @@ markers =
     qtbug60673: Tests which are broken if the conversion from orange selection to real selection  is flaky
     fake_os: Fake utils.is_* to a fake operating system
     unicode_locale: Tests which need an unicode locale to work
+    qtwebkit6021_skip: Tests which would fail on WebKit version 602.1
 qt_log_level_fail = WARNING
 qt_log_ignore =
     ^SpellCheck: .*
diff --git a/qutebrowser/javascript/greasemonkey_wrapper.js b/qutebrowser/javascript/greasemonkey_wrapper.js
index 7f348dbcc..b38aaeec9 100644
--- a/qutebrowser/javascript/greasemonkey_wrapper.js
+++ b/qutebrowser/javascript/greasemonkey_wrapper.js
@@ -145,22 +145,16 @@
         }
     };
 
-    /* TamperMonkey allows assinging to `window` to change the visible global
-     * scope, without breaking other scripts. Eg if the page has set
-     * window.$ for an object for some reason (hello 4chan)
-     * - typeof $ === 'object'
-     * - typeof window.$ == 'undefined'
-     * - window.$ = function() {}
-     * - typeof $ === 'function'
-     * - typeof window.$ == 'function'
-     * Just shadowing `window` won't work because if you try to use '$'
-     * from the global scope you will still get the pages one.
-     * Additionally the userscript expects `window` to actually look
-     * like a fully featured browser window object.
-     *
-     * So let's try to use a Proxy on window and the possibly deprecated
-     * `with` function to make that proxy shadow the global scope.
-     * unsafeWindow should still be the actual global page window.
+    /* 
+     * Try to give userscripts an environment that they expect. Which
+     * seems to be that the global window object should look the same as
+     * the page's one and that if a script writes to an attribute of
+     * window it should be able to access that variable in the global
+     * scope.
+     * Use a Proxy to stop scripts from actually changing the global
+     * window (that's what unsafeWindow is for).
+     * Use the "with" statement to make the proxy provide what looks
+     * like global scope.
      *
      * There are other Proxy functions that we may need to override.
      * set, get and has are definitely required.
@@ -187,7 +181,8 @@
         return key in qute_gm_window_shadow || key in target;
       }
     };
-    const qute_gm_window_proxy = new Proxy(unsafeWindow, qute_gm_windowProxyHandler);
+    const qute_gm_window_proxy = new Proxy(
+      unsafeWindow, qute_gm_windowProxyHandler);
 
     with (qute_gm_window_proxy) {
       // We can't return `this` or `qute_gm_window_proxy` from
diff --git a/tests/conftest.py b/tests/conftest.py
index 0b82bc7f6..97d562337 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -36,7 +36,7 @@ from helpers import logfail
 from helpers.logfail import fail_on_logging
 from helpers.messagemock import message_mock
 from helpers.fixtures import *  # noqa: F403
-from qutebrowser.utils import qtutils, standarddir, usertypes, utils
+from qutebrowser.utils import qtutils, standarddir, usertypes, utils, version
 from qutebrowser.misc import objects
 
 import qutebrowser.app  # To register commands
@@ -77,6 +77,10 @@ def _apply_platform_markers(config, item):
          "https://bugreports.qt.io/browse/QTBUG-60673"),
         ('unicode_locale', sys.getfilesystemencoding() == 'ascii',
          "Skipped because of ASCII locale"),
+        ('qtwebkit6021_skip',
+         version.qWebKitVersion and
+         version.qWebKitVersion() == '602.1',
+         "Broken on WebKit 602.1")
     ]
 
     for searched_marker, condition, default_reason in markers:
diff --git a/tests/unit/javascript/test_greasemonkey.py b/tests/unit/javascript/test_greasemonkey.py
index 7494c5c13..8f780c432 100644
--- a/tests/unit/javascript/test_greasemonkey.py
+++ b/tests/unit/javascript/test_greasemonkey.py
@@ -212,6 +212,8 @@ class TestWindowIsolation:
         page.runJavaScript(self.test_script, callback_checker.callback)
         callback_checker.check(self.expected)
 
+    # The JSCore in 602.1 doesn't fully support Proxy.
+    @pytest.mark.qtwebkit6021_skip
     def test_webkit(self, webview):
         elem = webview.page().mainFrame().documentElement()
         elem.evaluateJavaScript(self.setup_script)