Move searching out of WebView subclass

This also makes it work for QtWebEngine.
It also seems to fix #1050 though I'm not 100% sure why.
This commit is contained in:
Florian Bruhin 2016-07-12 16:10:35 +02:00
parent 038f803180
commit f0da508c21
6 changed files with 166 additions and 77 deletions

View File

@ -133,7 +133,7 @@ class AbstractSearch(QObject):
Attributes:
text: The last thing this view was searched for.
_flags: The flags of the last search.
_flags: The flags of the last search (needs to be set by subclasses).
_widget: The underlying WebView widget.
"""
@ -141,9 +141,9 @@ class AbstractSearch(QObject):
super().__init__(parent)
self._widget = None
self.text = None
self._flags = 0
def search(self, text, *, ignore_case=False, wrap=False, reverse=False):
def search(self, text, *, ignore_case=False, wrap=False, reverse=False,
result_cb=None):
"""Find the given text on the page.
Args:
@ -151,6 +151,7 @@ class AbstractSearch(QObject):
ignore_case: Search case-insensitively. (True/False/'smart')
wrap: Wrap around to the top when arriving at the bottom.
reverse: Reverse search direction.
result_cb: Called with a bool indicating whether a match was found.
"""
raise NotImplementedError
@ -158,12 +159,20 @@ class AbstractSearch(QObject):
"""Clear the current search."""
raise NotImplementedError
def prev_result(self):
"""Go to the previous result of the current search."""
def prev_result(self, *, result_cb=None):
"""Go to the previous result of the current search.
Args:
result_cb: Called with a bool indicating whether a match was found.
"""
raise NotImplementedError
def next_result(self):
"""Go to the next result of the current search."""
def next_result(self, *, result_cb=None):
"""Go to the next result of the current search.
Args:
result_cb: Called with a bool indicating whether a match was found.
"""
raise NotImplementedError

View File

@ -1440,6 +1440,47 @@ class CommandDispatcher:
this.dispatchEvent(event);
""".format(webelem.javascript_escape(sel)))
def _search_cb(self, found, *, tab, old_scroll_pos, options, text, prev):
"""Callback called from search/search_next/search_prev.
Args:
found: Whether the text was found.
tab: The AbstractTab in which the search was made.
old_scroll_pos: The scroll position (QPoint) before the search.
options: The options (dict) the search was made with.
text: The text searched for.
prev: Whether we're searching backwards (i.e. :search-prev)
"""
# :search/:search-next without reverse -> down
# :search/:search-next with reverse -> up
# :search-prev without reverse -> up
# :search-prev with reverse -> down
going_up = options['reverse'] ^ prev
if found:
# Check if the scroll position got smaller and show info.
if not going_up and tab.scroll.pos_px().y() < old_scroll_pos.y():
message.info(self._win_id, "Search hit BOTTOM, continuing "
"at TOP", immediately=True)
elif going_up and tab.scroll.pos_px().y() > old_scroll_pos.y():
message.info(self._win_id, "Search hit TOP, continuing at "
"BOTTOM", immediately=True)
else:
# User disabled wrapping; but findText() just returns False. If we
# have a selection, we know there's a match *somewhere* on the page
if not options['wrap'] and tab.caret.has_selection():
if going_up:
message.warning(self._win_id, "Search hit TOP without "
"match for: {}".format(text),
immediately=True)
else:
message.warning(self._win_id, "Search hit BOTTOM without "
"match for: {}".format(text),
immediately=True)
else:
message.warning(self._win_id, "Text '{}' not found on "
"page!".format(text), immediately=True)
@cmdutils.register(instance='command-dispatcher', scope='window',
maxsplit=0)
def search(self, text="", reverse=False):
@ -1458,10 +1499,18 @@ class CommandDispatcher:
'wrap': config.get('general', 'wrap-search'),
'reverse': reverse,
}
tab.search.search(text, **options)
self._tabbed_browser.search_text = text
self._tabbed_browser.search_options = options
self._tabbed_browser.search_options = dict(options)
if text:
cb = functools.partial(self._search_cb, tab=tab,
old_scroll_pos=tab.scroll.pos_px(),
options=options, text=text, prev=False)
else:
cb = None
options['result_cb'] = cb
tab.search.search(text, **options)
@cmdutils.register(instance='command-dispatcher', hide=True,
scope='window')
@ -1476,6 +1525,9 @@ class CommandDispatcher:
window_text = self._tabbed_browser.search_text
window_options = self._tabbed_browser.search_options
if window_text is None:
raise cmdexc.CommandError("No search done yet.")
self.set_mark("'")
if window_text is not None and window_text != tab.search.text:
@ -1483,8 +1535,17 @@ class CommandDispatcher:
tab.search.search(window_text, **window_options)
count -= 1
for _ in range(count):
if count == 0:
return
cb = functools.partial(self._search_cb, tab=tab,
old_scroll_pos=tab.scroll.pos_px(),
options=window_options, text=window_text,
prev=False)
for _ in range(count - 1):
tab.search.next_result()
tab.search.next_result(result_cb=cb)
@cmdutils.register(instance='command-dispatcher', hide=True,
scope='window')
@ -1499,6 +1560,9 @@ class CommandDispatcher:
window_text = self._tabbed_browser.search_text
window_options = self._tabbed_browser.search_options
if window_text is None:
raise cmdexc.CommandError("No search done yet.")
self.set_mark("'")
if window_text is not None and window_text != tab.search.text:
@ -1506,8 +1570,17 @@ class CommandDispatcher:
tab.search.search(window_text, **window_options)
count -= 1
for _ in range(count):
if count == 0:
return
cb = functools.partial(self._search_cb, tab=tab,
old_scroll_pos=tab.scroll.pos_px(),
options=window_options, text=window_text,
prev=True)
for _ in range(count - 1):
tab.search.prev_result()
tab.search.prev_result(result_cb=cb)
@cmdutils.register(instance='command-dispatcher', hide=True,
modes=[KeyMode.caret], scope='window')

View File

@ -61,17 +61,48 @@ class WebEngineSearch(browsertab.AbstractSearch):
"""QtWebEngine implementations related to searching on the page."""
def search(self, text, *, ignore_case=False, wrap=False, reverse=False):
log.stub()
def __init__(self, parent=None):
super().__init__(parent)
self._flags = QWebEnginePage.FindFlags(0)
def _find(self, text, flags, cb=None):
"""Call findText on the widget with optional callback."""
if cb is None:
self._widget.findText(text, flags)
else:
self._widget.findText(text, flags, cb)
def search(self, text, *, ignore_case=False, wrap=False, reverse=False,
result_cb=None):
flags = QWebEnginePage.FindFlags(0)
if ignore_case == 'smart':
if not text.islower():
flags |= QWebEnginePage.FindCaseSensitively
elif not ignore_case:
flags |= QWebEnginePage.FindCaseSensitively
if not wrap:
log.stub('With wrap=False (ignoring)')
if reverse:
flags |= QWebEnginePage.FindBackward
self.text = text
self._flags = flags
self._find(text, flags, result_cb)
def clear(self):
log.stub()
self._widget.findText('')
def prev_result(self):
log.stub()
def prev_result(self, *, result_cb=None):
# The int() here makes sure we get a copy of the flags.
flags = QWebEnginePage.FindFlags(int(self._flags))
if flags & QWebEnginePage.FindBackward:
flags &= ~QWebEnginePage.FindBackward
else:
flags |= QWebEnginePage.FindBackward
self._find(self.text, self._flags, result_cb)
def next_result(self):
log.stub()
def next_result(self, *, result_cb=None):
self._find(self.text, self._flags, result_cb)
class WebEngineCaret(browsertab.AbstractCaret):

View File

@ -64,13 +64,30 @@ class WebKitSearch(browsertab.AbstractSearch):
"""QtWebKit implementations related to searching on the page."""
def __init__(self, parent=None):
super().__init__(parent)
self._flags = QWebPage.FindFlags(0)
def _call_cb(self, callback, found):
"""Call the given callback if it's non-None.
Delays the call via a QTimer so the website is re-rendered in between.
Args:
callback: What to call
found: If the text was found
"""
if callback is not None:
QTimer.singleShot(0, functools.partial(callback, found))
def clear(self):
# We first clear the marked text, then the highlights
self._widget.search('', 0)
self._widget.search('', QWebPage.HighlightAllOccurrences)
self._widget.findText('')
self._widget.findText('', QWebPage.HighlightAllOccurrences)
def search(self, text, *, ignore_case=False, wrap=False, reverse=False):
flags = 0
def search(self, text, *, ignore_case=False, wrap=False, reverse=False,
result_cb=None):
flags = QWebPage.FindFlags(0)
if ignore_case == 'smart':
if not text.islower():
flags |= QWebPage.FindCaseSensitively
@ -82,24 +99,25 @@ class WebKitSearch(browsertab.AbstractSearch):
flags |= QWebPage.FindBackward
# We actually search *twice* - once to highlight everything, then again
# to get a mark so we can navigate.
self._widget.search(text, flags)
self._widget.search(text, flags | QWebPage.HighlightAllOccurrences)
found = self._widget.findText(text, flags)
self._widget.findText(text, flags | QWebPage.HighlightAllOccurrences)
self.text = text
self._flags = flags
self._call_cb(result_cb, found)
def next_result(self):
self._widget.search(self.text, self._flags)
def next_result(self, *, result_cb=None):
found = self._widget.findText(self.text, self._flags)
self._call_cb(result_cb, found)
def prev_result(self):
# The int() here serves as a QFlags constructor to create a copy of the
# QFlags instance rather as a reference. I don't know why it works this
# way, but it does.
flags = int(self._flags)
def prev_result(self, *, result_cb=None):
# The int() here makes sure we get a copy of the flags.
flags = QWebPage.FindFlags(int(self._flags))
if flags & QWebPage.FindBackward:
flags &= ~QWebPage.FindBackward
else:
flags |= QWebPage.FindBackward
self._widget.search(self.text, flags)
found = self._widget.findText(self.text, flags)
self._call_cb(result_cb, found)
class WebKitCaret(browsertab.AbstractCaret):

View File

@ -352,48 +352,6 @@ class WebView(QWebView):
"left.".format(mode))
self.setFocusPolicy(Qt.WheelFocus)
def search(self, text, flags):
"""Search for text in the current page.
Args:
text: The text to search for.
flags: The QWebPage::FindFlags.
"""
log.webview.debug("Searching with text '{}' and flags "
"0x{:04x}.".format(text, int(flags)))
old_scroll_pos = self.scroll_pos
flags = QWebPage.FindFlags(flags)
found = self.findText(text, flags)
backward = flags & QWebPage.FindBackward
if not found and not flags & QWebPage.HighlightAllOccurrences and text:
# User disabled wrapping; but findText() just returns False. If we
# have a selection, we know there's a match *somewhere* on the page
if (not flags & QWebPage.FindWrapsAroundDocument and
self.hasSelection()):
if not backward:
message.warning(self.win_id, "Search hit BOTTOM without "
"match for: {}".format(text),
immediately=True)
else:
message.warning(self.win_id, "Search hit TOP without "
"match for: {}".format(text),
immediately=True)
else:
message.warning(self.win_id, "Text '{}' not found on "
"page!".format(text), immediately=True)
else:
def check_scroll_pos():
"""Check if the scroll position got smaller and show info."""
if not backward and self.scroll_pos < old_scroll_pos:
message.info(self.win_id, "Search hit BOTTOM, continuing "
"at TOP", immediately=True)
elif backward and self.scroll_pos > old_scroll_pos:
message.info(self.win_id, "Search hit TOP, continuing at "
"BOTTOM", immediately=True)
# We first want QWebPage to refresh.
QTimer.singleShot(0, check_scroll_pos)
def createWindow(self, wintype):
"""Called by Qt when a page wants to create a new window.

View File

@ -100,7 +100,7 @@ Feature: Searching on a page
# Make sure there was no search in the same window before
When I open data/search.html in a new window
And I run :search-next
Then no crash should happen
Then the error "No search done yet." should be shown
Scenario: Repeating search in a second tab (issue #940)
When I open data/search.html in a new tab
@ -141,7 +141,7 @@ Feature: Searching on a page
# Make sure there was no search in the same window before
When I open data/search.html in a new window
And I run :search-prev
Then no crash should happen
Then the error "No search done yet." should be shown
## wrapping