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
This commit is contained in:
Florian Bruhin 2018-02-06 21:25:08 +01:00
parent 8c39e8f764
commit 16375f20d5
4 changed files with 10 additions and 25 deletions

View File

@ -392,9 +392,6 @@ class AbstractCaret(QObject):
def drop_selection(self): def drop_selection(self):
raise NotImplementedError raise NotImplementedError
def has_selection(self):
raise NotImplementedError
def selection(self, html=False, callback=None): def selection(self, html=False, callback=None):
raise NotImplementedError raise NotImplementedError

View File

@ -853,7 +853,7 @@ class CommandDispatcher:
what = 'URL' # For printing what = 'URL' # For printing
elif what == 'selection': elif what == 'selection':
def _selection_callback(s): def _selection_callback(s):
if not self._current_widget().caret.has_selection() or not s: if not s:
message.info("Nothing to yank") message.info("Nothing to yank")
return return
self._yank_to_target(s, sel, what, keep) self._yank_to_target(s, sel, what, keep)
@ -1267,7 +1267,7 @@ class CommandDispatcher:
env['QUTE_TITLE'] = self._tabbed_browser.page_title(idx) env['QUTE_TITLE'] = self._tabbed_browser.page_title(idx)
tab = self._tabbed_browser.currentWidget() 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 env['QUTE_SELECTED_TEXT'] = selection
try: try:
env['QUTE_SELECTED_HTML'] = tab.caret.selection(html=True) env['QUTE_SELECTED_HTML'] = tab.caret.selection(html=True)

View File

@ -289,24 +289,15 @@ class WebEngineCaret(browsertab.AbstractCaret):
def drop_selection(self): def drop_selection(self):
self._js_call('dropSelection') 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): def selection(self, html=False, callback=None):
if html: if html:
raise browsertab.UnsupportedOperationError raise browsertab.UnsupportedOperationError
if qtutils.version_check('5.10'): # Not using selectedText() as WORKAROUND for
callback(self._widget.selectedText())
else:
# WORKAROUND for
# https://bugreports.qt.io/browse/QTBUG-53134 # https://bugreports.qt.io/browse/QTBUG-53134
self._tab.run_js_async( # Even on Qt 5.10 selectedText() seems to work poorly, see
javascript.assemble('caret', 'getSelection'), callback) # 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): def _follow_selected_cb(self, js_elem, tab=False):
"""Callback for javascript which clicks the selected element. """Callback for javascript which clicks the selected element.

View File

@ -360,9 +360,6 @@ class WebKitCaret(browsertab.AbstractCaret):
def drop_selection(self): def drop_selection(self):
self._widget.triggerPageAction(QWebPage.MoveToNextChar) self._widget.triggerPageAction(QWebPage.MoveToNextChar)
def has_selection(self):
return self._widget.hasSelection()
def selection(self, html=False, callback=False): def selection(self, html=False, callback=False):
callback(self._selection(html)) callback(self._selection(html))
@ -373,8 +370,6 @@ class WebKitCaret(browsertab.AbstractCaret):
return self._widget.selectedText() return self._widget.selectedText()
def follow_selected(self, *, tab=False): def follow_selected(self, *, tab=False):
if not self.has_selection():
return
if QWebSettings.globalSettings().testAttribute( if QWebSettings.globalSettings().testAttribute(
QWebSettings.JavascriptEnabled): QWebSettings.JavascriptEnabled):
if tab: if tab:
@ -383,6 +378,8 @@ class WebKitCaret(browsertab.AbstractCaret):
'window.getSelection().anchorNode.parentNode.click()') 'window.getSelection().anchorNode.parentNode.click()')
else: else:
selection = self._selection(html=True) selection = self._selection(html=True)
if not selection:
return
try: try:
selected_element = xml.etree.ElementTree.fromstring( selected_element = xml.etree.ElementTree.fromstring(
'<html>{}</html>'.format(selection)).find('a') '<html>{}</html>'.format(selection)).find('a')