From 16375f20d560f2d1fcccc00153168148f1e99a19 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Tue, 6 Feb 2018 21:25:08 +0100 Subject: [PATCH] Always use JavaScript to get selection It looks like getting the selection via the widget has issues even with Qt 5.10. On Windows, we always get wrong results. On Linux, it seems to be flaky. I first thought this was because of a race between JavaScript setting the selection and Qt getting it, as now we don't use JS to get the selection anymore, so it's possible that we get it before the older JS code finished running. However, even calling selectedText() from a JS callback didn't seem to help... Since has_selection also is flawed and it taking a callback would make code more complex as well, let's just assume there is a selection if the text is not empty. In fact, that is exactly what QtWebEngine does for hasSelection anyways! Fixes #3523 --- qutebrowser/browser/browsertab.py | 3 --- qutebrowser/browser/commands.py | 4 ++-- qutebrowser/browser/webengine/webenginetab.py | 21 ++++++------------- qutebrowser/browser/webkit/webkittab.py | 7 ++----- 4 files changed, 10 insertions(+), 25 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 1a514f4a3..add08e66f 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -392,9 +392,6 @@ class AbstractCaret(QObject): def drop_selection(self): raise NotImplementedError - def has_selection(self): - raise NotImplementedError - def selection(self, html=False, callback=None): raise NotImplementedError diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 0ebcf3240..57f6cce47 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -853,7 +853,7 @@ class CommandDispatcher: what = 'URL' # For printing elif what == 'selection': def _selection_callback(s): - if not self._current_widget().caret.has_selection() or not s: + if not s: message.info("Nothing to yank") return self._yank_to_target(s, sel, what, keep) @@ -1267,7 +1267,7 @@ class CommandDispatcher: env['QUTE_TITLE'] = self._tabbed_browser.page_title(idx) tab = self._tabbed_browser.currentWidget() - if tab is not None and tab.caret.has_selection(): + if tab is not None: env['QUTE_SELECTED_TEXT'] = selection try: env['QUTE_SELECTED_HTML'] = tab.caret.selection(html=True) diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index ecbe81922..c90481c21 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -289,24 +289,15 @@ class WebEngineCaret(browsertab.AbstractCaret): def drop_selection(self): self._js_call('dropSelection') - def has_selection(self): - if qtutils.version_check('5.10'): - return self._widget.hasSelection() - else: - # WORKAROUND for - # https://bugreports.qt.io/browse/QTBUG-53134 - return True - def selection(self, html=False, callback=None): if html: raise browsertab.UnsupportedOperationError - if qtutils.version_check('5.10'): - callback(self._widget.selectedText()) - else: - # WORKAROUND for - # https://bugreports.qt.io/browse/QTBUG-53134 - self._tab.run_js_async( - javascript.assemble('caret', 'getSelection'), callback) + # Not using selectedText() as WORKAROUND for + # https://bugreports.qt.io/browse/QTBUG-53134 + # Even on Qt 5.10 selectedText() seems to work poorly, see + # https://github.com/qutebrowser/qutebrowser/issues/3523 + self._tab.run_js_async(javascript.assemble('caret', 'getSelection'), + callback) def _follow_selected_cb(self, js_elem, tab=False): """Callback for javascript which clicks the selected element. diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 4ad177a2a..a493ce495 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -360,9 +360,6 @@ class WebKitCaret(browsertab.AbstractCaret): def drop_selection(self): self._widget.triggerPageAction(QWebPage.MoveToNextChar) - def has_selection(self): - return self._widget.hasSelection() - def selection(self, html=False, callback=False): callback(self._selection(html)) @@ -373,8 +370,6 @@ class WebKitCaret(browsertab.AbstractCaret): return self._widget.selectedText() def follow_selected(self, *, tab=False): - if not self.has_selection(): - return if QWebSettings.globalSettings().testAttribute( QWebSettings.JavascriptEnabled): if tab: @@ -383,6 +378,8 @@ class WebKitCaret(browsertab.AbstractCaret): 'window.getSelection().anchorNode.parentNode.click()') else: selection = self._selection(html=True) + if not selection: + return try: selected_element = xml.etree.ElementTree.fromstring( '{}'.format(selection)).find('a')