Merge pull request #2015 from kernc/fix-webengine-qt5

[FIX] Highcharts: Fix freezing on Qt5

Orange/widgets/_highcharts/orange-selection.js

@@ -37,7 +37,7 @@ function unselectAllPoints(e) {
           e.target.parentElement.tagName.toLowerCase() == 'svg'))
         return true;
     this.deselectPointsIfNot(false);
-    _highcharts_bridge.on_selected_points([]);
+    pybridge._highcharts_on_selected_points([]);
 }
 
 function clickedPointSelect(e) {
@@ -49,7 +49,7 @@ function clickedPointSelect(e) {
         selected.splice(selected.indexOf(this.index), 1);
     } else
         points[this.series.index].push(this.index);
-    _highcharts_bridge.on_selected_points(points);
+    pybridge._highcharts_on_selected_points(points);
     return true;
 }
 
@@ -83,6 +83,6 @@ function rectSelectPoints(e) {
         }
     }
 
-    _highcharts_bridge.on_selected_points(this.getSelectedPointsForExport());
+    pybridge._highcharts_on_selected_points(this.getSelectedPointsForExport());
     return false;  // Don't zoom
 }

Orange/widgets/highcharts.py

@@ -123,25 +123,52 @@ def __init__(self,
         if enable_select and not selection_callback:
             raise ValueError('enable_select requires selection_callback')
 
+        if enable_select:
+            # We need to make sure the _Bridge object below with the selection
+            # callback is exposed in JS via QWebChannel.registerObject() and
+            # not through WebviewWidget.exposeObject() as the latter mechanism
+            # doesn't transmit QObjects correctly.
+            class _Bridge(QObject):
+                @pyqtSlot('QVariantList')
+                def _highcharts_on_selected_points(self, points):
+                    selection_callback([np.sort(selected).astype(int)
+                                        for selected in points])
+            if bridge is None:
+                bridge = _Bridge()
+            else:
+                # Thus, we patch existing user-passed bridge with our
+                # selection callback method
+                attrs = bridge.__dict__.copy()
+                attrs['_highcharts_on_selected_points'] = _Bridge._highcharts_on_selected_points
+                assert isinstance(bridge, QObject), 'bridge needs to be a QObject'
+                _Bridge = type(bridge.__class__.__name__,
+                               bridge.__class__.__mro__,
+                               attrs)
+                bridge = _Bridge()
+
         super().__init__(parent, bridge, debug=debug)
 
         self.highchart = highchart
         self.enable_zoom = enable_zoom
         enable_point_select = '+' in enable_select
         enable_rect_select = enable_select.replace('+', '')
+
+        self._update_options_dict(options, enable_zoom, enable_select,
+                                  enable_point_select, enable_rect_select,
+                                  kwargs)
+
+        with open(self._HIGHCHARTS_HTML) as html:
+            self.setHtml(html.read() % dict(javascript=javascript,
+                                            options=json(options)),
+                         self.toFileURL(dirname(self._HIGHCHARTS_HTML)) + '/')
+
+    def _update_options_dict(self, options, enable_zoom, enable_select,
+                             enable_point_select, enable_rect_select, kwargs):
         if enable_zoom:
             _merge_dicts(options, _kwargs_options(dict(
                 mapNavigation_enableMouseWheelZoom=True,
                 mapNavigation_enableButtons=False)))
         if enable_select:
-
-            class _Bridge(QObject):
-                @pyqtSlot('QVariantList')
-                def on_selected_points(self, points):
-                    selection_callback([np.sort(selected).astype(int)
-                                        for selected in points])
-
-            self.exposeObject('_highcharts_bridge', _Bridge())
             _merge_dicts(options, _kwargs_options(dict(
                 chart_events_click='/**/unselectAllPoints/**/')))
         if enable_point_select:
@@ -155,11 +182,6 @@ def on_selected_points(self, points):
         if kwargs:
             _merge_dicts(options, _kwargs_options(kwargs))
 
-        with open(self._HIGHCHARTS_HTML) as html:
-            self.setHtml(html.read() % dict(javascript=javascript,
-                                            options=json(options)),
-                         self.toFileURL(dirname(self._HIGHCHARTS_HTML)) + '/')
-
     def contextMenuEvent(self, event):
         """ Zoom out on right click. Also disable context menu."""
         if self.enable_zoom:
@@ -232,7 +254,7 @@ def svg(self):
 
 def main():
     """ A simple test. """
-    from AnyQt.QtGui import QApplication
+    from AnyQt.QtWidgets import QApplication
     app = QApplication([])
 
     def _on_selected_points(points):
@@ -251,4 +273,3 @@ def _on_selected_points(points):
 
 if __name__ == '__main__':
     main()
-

Orange/widgets/tests/test_highcharts.py

@@ -1,8 +1,9 @@
 import time
 import os
+import sys
 import unittest
 
-from AnyQt.QtCore import Qt, QPoint
+from AnyQt.QtCore import Qt, QPoint, QObject
 from AnyQt.QtWidgets import qApp
 from AnyQt.QtTest import QTest
 
@@ -12,23 +13,34 @@
 
 
 class SelectionScatter(Highchart):
-    def __init__(self, selected_indices_callback):
-        super().__init__(enable_select='xy+',
+    def __init__(self, bridge, selected_indices_callback):
+        super().__init__(bridge=bridge,
+                         enable_select='xy+',
                          selection_callback=selected_indices_callback,
                          options=dict(chart=dict(type='scatter')))
 
 
 class HighchartTest(WidgetTest):
     @unittest.skipIf(os.environ.get('APPVEYOR'), 'test stalls on AppVeyor')
+    @unittest.skipIf(sys.version_info[:2] <= (3, 4),
+                     'the second iteration stalls on Travis / Py3.4')
     def test_selection(self):
+
+        class NoopBridge(QObject):
+            pass
+
+        for bridge in (NoopBridge(), None):
+            self._selection_test(bridge)
+
+    def _selection_test(self, bridge):
         data = Table('iris')
         selected_indices = []
 
         def selection_callback(indices):
             nonlocal selected_indices
             selected_indices = indices
 
-        scatter = SelectionScatter(selection_callback)
+        scatter = SelectionScatter(bridge, selection_callback)
         scatter.chart(options=dict(series=[dict(data=data.X[:, :2])]))
         scatter.show()
 
@@ -65,5 +77,6 @@ def selection_callback(indices):
 
         self.assertFalse(len(selected_indices))
 
+        # Test Esc hiding
         QTest.keyClick(scatter, Qt.Key_Escape)
         self.assertTrue(scatter.isHidden())

Orange/widgets/utils/webview.py

@@ -5,6 +5,7 @@
 (extends QWebView), as available.
 """
 import os
+from os.path import join, dirname, abspath
 import warnings
 from random import random
 from collections.abc import Mapping, Set, Sequence, Iterable
@@ -13,7 +14,6 @@
 
 from urllib.parse import urljoin
 from urllib.request import pathname2url
-from os.path import join, dirname, abspath
 
 import numpy as np
 
@@ -60,7 +60,7 @@ def hideWindow(self):
         while isinstance(w, QWidget):
             if w.windowFlags() & (Qt.Window | Qt.Dialog):
                 return w.hide()
-            w = w.parent()
+            w = w.parent() if callable(w.parent) else w.parent
 
 
 if HAVE_WEBENGINE:
@@ -92,7 +92,7 @@ def __init__(self, parent=None, bridge=None, *, debug=False, **kwargs):
                 warnings.warn(
                     'To debug QWebEngineView, set environment variable '
                     'QTWEBENGINE_REMOTE_DEBUGGING={port} and then visit '
-                    'http://localhost:{port}/ in a Chromium-based browser. '
+                    'http://127.0.0.1:{port}/ in a Chromium-based browser. '
                     'See https://doc.qt.io/qt-5/qtwebengine-debugging.html '
                     'This has also been done for you.'.format(port=port))
             super().__init__(parent,
@@ -113,7 +113,7 @@ def __init__(self, parent=None, bridge=None, *, debug=False, **kwargs):
                 with open(_WEBENGINE_INIT_WEBCHANNEL, encoding="utf-8") as f:
                     init_webchannel_src = f.read()
                 self._onloadJS(source + init_webchannel_src %
-                                   dict(exposeObject_prefix=self._EXPOSED_OBJ_PREFIX),
+                               dict(exposeObject_prefix=self._EXPOSED_OBJ_PREFIX),
                                name='webchannel_init',
                                injection_point=QWebEngineScript.DocumentCreation)
             else:
@@ -456,6 +456,11 @@ def __init__(self, parent):
             self._objects = {}
 
         def send_object(self, name, obj):
+            if isinstance(obj, QObject):
+                raise ValueError(
+                    "QWebChannel doesn't transmit QObject instances. If you "
+                    "need a QObject available in JavaScript, pass it as a "
+                    "bridge in WebviewWidget constructor.")
             id = next(self._id_gen)
             value = self._objects[id] = dict(id=id, name=name, obj=obj)
             # Wait till JS is connected to receive objects