From fb19a2815d2eaec1dd5a58939863f50e2e0aecd8 Mon Sep 17 00:00:00 2001 From: svetelna Date: Mon, 20 Apr 2020 15:42:04 +0200 Subject: [PATCH 01/17] implementing visual-line-mode on webengine side --- qutebrowser/browser/browsertab.py | 24 ++++- qutebrowser/browser/webengine/webenginetab.py | 17 +++- qutebrowser/components/caretcommands.py | 10 +- qutebrowser/config/configdata.yml | 1 + qutebrowser/javascript/caret.js | 91 +++++++++++++++---- qutebrowser/mainwindow/statusbar/bar.py | 12 ++- qutebrowser/mainwindow/tabbedbrowser.py | 2 +- 7 files changed, 127 insertions(+), 30 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 00d46c813..16ddeef8b 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -427,13 +427,22 @@ class AbstractZoom(QObject): self._set_factor_internal(self._zoom_factor) +class SelectionState(enum.IntEnum): + + """Possible states of selection in Caret mode.""" + + none = 1 + normal = 2 + line = 3 + + class AbstractCaret(QObject): """Attribute ``caret`` of AbstractTab for caret browsing.""" #: Signal emitted when the selection was toggled. #: (argument - whether the selection is now active) - selection_toggled = pyqtSignal(bool) + selection_toggled = pyqtSignal(SelectionState) #: Emitted when a ``follow_selection`` action is done. follow_selected_done = pyqtSignal() @@ -444,7 +453,7 @@ class AbstractCaret(QObject): super().__init__(parent) self._tab = tab self._widget = typing.cast(QWidget, None) - self.selection_enabled = False + self.selection_state = SelectionState.none self._mode_manager = mode_manager mode_manager.entered.connect(self._on_mode_entered) mode_manager.left.connect(self._on_mode_left) @@ -500,7 +509,7 @@ class AbstractCaret(QObject): def move_to_end_of_document(self) -> None: raise NotImplementedError - def toggle_selection(self) -> None: + def toggle_selection(self, line: bool = False) -> None: raise NotImplementedError def drop_selection(self) -> None: @@ -826,6 +835,15 @@ class AbstractTabPrivate: def shutdown(self) -> None: raise NotImplementedError + def run_js_sync(self, code: str) -> None: + """Run javascript sync. + + Result will be returned when running JS is complete. + This is only implemented for QtWebKit. + For QtWebEngine, always raises UnsupportedOperationError. + """ + raise NotImplementedError + class AbstractTab(QWidget): diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 647fa60ab..5b0721c18 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -369,7 +369,10 @@ class WebEngineCaret(browsertab.AbstractCaret): if enabled is None: log.webview.debug("Ignoring selection status None") return - self.selection_toggled.emit(enabled) + if enabled: + self.selection_toggled.emit(browsertab.SelectionState.normal) + else: + self.selection_toggled.emit(browsertab.SelectionState.none) @pyqtSlot(usertypes.KeyMode) def _on_mode_left(self, mode): @@ -424,8 +427,9 @@ class WebEngineCaret(browsertab.AbstractCaret): def move_to_end_of_document(self): self._js_call('moveToEndOfDocument') - def toggle_selection(self): - self._js_call('toggleSelection', callback=self.selection_toggled.emit) + def toggle_selection(self, line=False): + self._js_call('toggleSelection', line, + callback=self._toggle_sel_translate) def drop_selection(self): self._js_call('dropSelection') @@ -500,6 +504,10 @@ class WebEngineCaret(browsertab.AbstractCaret): code = javascript.assemble('caret', command, *args) self._tab.run_js_async(code, callback) + def _toggle_sel_translate(self, state_int): + state = browsertab.SelectionState(state_int) + self.selection_toggled.emit(state) + class WebEngineScroller(browsertab.AbstractScroller): @@ -1231,6 +1239,9 @@ class WebEngineTabPrivate(browsertab.AbstractTabPrivate): self._tab.action.exit_fullscreen() self._widget.shutdown() + def run_js_sync(self, code): + raise browsertab.UnsupportedOperationError + class WebEngineTab(browsertab.AbstractTab): diff --git a/qutebrowser/components/caretcommands.py b/qutebrowser/components/caretcommands.py index 173653bd9..966b193de 100644 --- a/qutebrowser/components/caretcommands.py +++ b/qutebrowser/components/caretcommands.py @@ -185,9 +185,13 @@ def move_to_end_of_document(tab: apitypes.Tab) -> None: @cmdutils.register(modes=[cmdutils.KeyMode.caret]) @cmdutils.argument('tab', value=cmdutils.Value.cur_tab) -def toggle_selection(tab: apitypes.Tab) -> None: - """Toggle caret selection mode.""" - tab.caret.toggle_selection() +def toggle_selection(tab: apitypes.Tab, line: bool = False) -> None: + """Toggle caret selection mode. + + Args: + line: Enables line-selection. + """ + tab.caret.toggle_selection(line) @cmdutils.register(modes=[cmdutils.KeyMode.caret]) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index 38db52304..df4efee06 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -3051,6 +3051,7 @@ bindings.default: : leave-mode caret: v: toggle-selection + V: toggle-selection --line : toggle-selection : drop-selection c: enter-mode normal diff --git a/qutebrowser/javascript/caret.js b/qutebrowser/javascript/caret.js index 55ff6a8b5..6d7a3bb00 100644 --- a/qutebrowser/javascript/caret.js +++ b/qutebrowser/javascript/caret.js @@ -705,6 +705,16 @@ window._qutebrowser.caret = (function() { */ CaretBrowsing.isCaretVisible = false; + /** + * selection modes + * @type {enum} + */ + CaretBrowsing.SelectionState = { + "NONE": 1, + "NORMAL": 2, + "LINE": 3, + }; + /** * The actual caret element, an absolute-positioned flashing line. * @type {Element} @@ -887,7 +897,11 @@ window._qutebrowser.caret = (function() { CaretBrowsing.injectCaretStyles(); CaretBrowsing.toggle(); CaretBrowsing.initiated = true; - CaretBrowsing.selectionEnabled = selectionRange > 0; + if (selectionRange > 0) { + CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NORMAL; + } else { + CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NONE; + } }; /** @@ -1145,16 +1159,52 @@ window._qutebrowser.caret = (function() { } }; + CaretBrowsing.reverseSelection = () => { + const sel = window.getSelection(); + sel.setBaseAndExtent( + sel.extentNode, sel.extentOffset, sel.baseNode, + sel.baseOffset + ); + }; + + CaretBrowsing.selectLine = function() { + const sel = window.getSelection(); + sel.modify("extend", "right", "lineboundary"); + CaretBrowsing.reverseSelection(); + sel.modify("extend", "left", "lineboundary"); + CaretBrowsing.reverseSelection(); + }; + + CaretBrowsing.updateLineSelection = function(direction, granularity) { + if (!(granularity === "character") && !(granularity === "word")) { + window. + getSelection(). + modify("extend", direction, granularity); + CaretBrowsing.selectLine(); + } + }; + + CaretBrowsing.selectionEnabled = function() { + if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.NONE) { + return false; + } + return true; + }; + CaretBrowsing.move = function(direction, granularity, count = 1) { let action = "move"; - if (CaretBrowsing.selectionEnabled) { + if (CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NONE) { action = "extend"; } for (let i = 0; i < count; i++) { - window. - getSelection(). - modify(action, direction, granularity); + if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.LINE) { + CaretBrowsing.updateLineSelection(direction, granularity); + } else { + window. + getSelection(). + modify(action, direction, granularity); + } } if (CaretBrowsing.isWindows && @@ -1174,7 +1224,7 @@ window._qutebrowser.caret = (function() { CaretBrowsing.moveToBlock = function(paragraph, boundary, count = 1) { let action = "move"; - if (CaretBrowsing.selectionEnabled) { + if (CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NONE) { action = "extend"; } for (let i = 0; i < count; i++) { @@ -1185,6 +1235,10 @@ window._qutebrowser.caret = (function() { window. getSelection(). modify(action, boundary, "paragraphboundary"); + + if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.LINE) { + CaretBrowsing.selectLine(); + } } }; @@ -1294,14 +1348,14 @@ window._qutebrowser.caret = (function() { funcs.setInitialCursor = () => { if (!CaretBrowsing.initiated) { CaretBrowsing.setInitialCursor(); - return CaretBrowsing.selectionEnabled; + return CaretBrowsing.selectionEnabled(); } if (window.getSelection().toString().length === 0) { positionCaret(); } CaretBrowsing.toggle(); - return CaretBrowsing.selectionEnabled; + return CaretBrowsing.selectionEnabled(); }; funcs.setFlags = (flags) => { @@ -1399,17 +1453,22 @@ window._qutebrowser.caret = (function() { funcs.getSelection = () => window.getSelection().toString(); - funcs.toggleSelection = () => { - CaretBrowsing.selectionEnabled = !CaretBrowsing.selectionEnabled; - return CaretBrowsing.selectionEnabled; + funcs.toggleSelection = (line) => { + if (line) { + CaretBrowsing.selectionState = + CaretBrowsing.SelectionState.LINE; + CaretBrowsing.selectLine(); + CaretBrowsing.finishMove(); + } else if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.NONE) { + CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NORMAL; + } else { + CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NONE; + } + return CaretBrowsing.selectionState; }; funcs.reverseSelection = () => { - const sel = window.getSelection(); - sel.setBaseAndExtent( - sel.extentNode, sel.extentOffset, sel.baseNode, - sel.baseOffset - ); + CaretBrowsing.reverseSelection(); }; return funcs; diff --git a/qutebrowser/mainwindow/statusbar/bar.py b/qutebrowser/mainwindow/statusbar/bar.py index b1aa4da38..95022803d 100644 --- a/qutebrowser/mainwindow/statusbar/bar.py +++ b/qutebrowser/mainwindow/statusbar/bar.py @@ -372,13 +372,17 @@ class StatusBar(QWidget): self.maybe_hide() assert tab.is_private == self._color_flags.private - @pyqtSlot(bool) - def on_caret_selection_toggled(self, selection): + @pyqtSlot(browsertab.SelectionState) + def on_caret_selection_toggled(self, selection_state): """Update the statusbar when entering/leaving caret selection mode.""" - log.statusbar.debug("Setting caret selection {}".format(selection)) - if selection: + log.statusbar.debug("Setting caret selection {}" + .format(selection_state)) + if selection_state is browsertab.SelectionState.normal: self._set_mode_text("caret selection") self._color_flags.caret = ColorFlags.CaretMode.selection + elif selection_state is browsertab.SelectionState.line: + self._set_mode_text("caret line selection") + self._color_flags.caret = ColorFlags.CaretMode.selection else: self._set_mode_text("caret") self._color_flags.caret = ColorFlags.CaretMode.on diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index f5dc3277b..e7756d321 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -189,7 +189,7 @@ class TabbedBrowser(QWidget): cur_scroll_perc_changed = pyqtSignal(int, int) cur_load_status_changed = pyqtSignal(usertypes.LoadStatus) cur_fullscreen_requested = pyqtSignal(bool) - cur_caret_selection_toggled = pyqtSignal(bool) + cur_caret_selection_toggled = pyqtSignal(browsertab.SelectionState) close_window = pyqtSignal() resized = pyqtSignal('QRect') current_tab_changed = pyqtSignal(browsertab.AbstractTab) From 90028d21d6ecf6d7c2271167f17bd264cec68eb7 Mon Sep 17 00:00:00 2001 From: svetelna Date: Sun, 26 Apr 2020 13:55:18 +0200 Subject: [PATCH 02/17] adding webkit part of line selection --- qutebrowser/browser/webkit/webkittab.py | 199 ++++++++++++++++-------- 1 file changed, 137 insertions(+), 62 deletions(-) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index d1122b78e..0a60e073b 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -177,8 +177,11 @@ class WebKitCaret(browsertab.AbstractCaret): if mode != usertypes.KeyMode.caret: return - self.selection_enabled = self._widget.hasSelection() - self.selection_toggled.emit(self.selection_enabled) + if self._widget.hasSelection(): + self.selection_state = browsertab.SelectionState.normal + else: + self.selection_state = browsertab.SelectionState.none + self.selection_toggled.emit(self.selection_state) settings = self._widget.settings() settings.setAttribute(QWebSettings.CaretBrowsingEnabled, True) @@ -193,7 +196,7 @@ class WebKitCaret(browsertab.AbstractCaret): # # Note: We can't use hasSelection() here, as that's always # true in caret mode. - if not self.selection_enabled: + if self.selection_state is browsertab.SelectionState.none: self._widget.page().currentFrame().evaluateJavaScript( utils.read_file('javascript/position_caret.js')) @@ -201,151 +204,190 @@ class WebKitCaret(browsertab.AbstractCaret): def _on_mode_left(self, _mode): settings = self._widget.settings() if settings.testAttribute(QWebSettings.CaretBrowsingEnabled): - if self.selection_enabled and self._widget.hasSelection(): + if (self.selection_state is not + browsertab.SelectionState.none and + self._widget.hasSelection()): # Remove selection if it exists self._widget.triggerPageAction(QWebPage.MoveToNextChar) settings.setAttribute(QWebSettings.CaretBrowsingEnabled, False) - self.selection_enabled = False + self.selection_state = browsertab.SelectionState.none def move_to_next_line(self, count=1): - if not self.selection_enabled: - act = QWebPage.MoveToNextLine - else: + if self.selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectNextLine + else: + act = QWebPage.MoveToNextLine for _ in range(count): self._widget.triggerPageAction(act) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_end() def move_to_prev_line(self, count=1): - if not self.selection_enabled: - act = QWebPage.MoveToPreviousLine - else: + if self.selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectPreviousLine + else: + act = QWebPage.MoveToPreviousLine for _ in range(count): self._widget.triggerPageAction(act) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_start() def move_to_next_char(self, count=1): - if not self.selection_enabled: - act = QWebPage.MoveToNextChar - else: + if self.selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectNextChar + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = QWebPage.MoveToNextChar for _ in range(count): self._widget.triggerPageAction(act) def move_to_prev_char(self, count=1): - if not self.selection_enabled: - act = QWebPage.MoveToPreviousChar - else: + if self.selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectPreviousChar + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = QWebPage.MoveToPreviousChar for _ in range(count): self._widget.triggerPageAction(act) def move_to_end_of_word(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToNextWord] - if utils.is_windows: # pragma: no cover - act.append(QWebPage.MoveToPreviousChar) - else: + if self.selection_state is browsertab.SelectionState.normal: act = [QWebPage.SelectNextWord] if utils.is_windows: # pragma: no cover act.append(QWebPage.SelectPreviousChar) + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = [QWebPage.MoveToNextWord] + if utils.is_windows: # pragma: no cover + act.append(QWebPage.MoveToPreviousChar) for _ in range(count): for a in act: self._widget.triggerPageAction(a) def move_to_next_word(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToNextWord] - if not utils.is_windows: # pragma: no branch - act.append(QWebPage.MoveToNextChar) - else: + if self.selection_state is browsertab.SelectionState.normal: act = [QWebPage.SelectNextWord] if not utils.is_windows: # pragma: no branch act.append(QWebPage.SelectNextChar) + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = [QWebPage.MoveToNextWord] + if not utils.is_windows: # pragma: no branch + act.append(QWebPage.MoveToNextChar) for _ in range(count): for a in act: self._widget.triggerPageAction(a) def move_to_prev_word(self, count=1): - if not self.selection_enabled: - act = QWebPage.MoveToPreviousWord - else: + if self.selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectPreviousWord + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = QWebPage.MoveToPreviousWord for _ in range(count): self._widget.triggerPageAction(act) def move_to_start_of_line(self): - if not self.selection_enabled: - act = QWebPage.MoveToStartOfLine - else: + if self.selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectStartOfLine + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = QWebPage.MoveToStartOfLine self._widget.triggerPageAction(act) def move_to_end_of_line(self): - if not self.selection_enabled: - act = QWebPage.MoveToEndOfLine - else: + if self.selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectEndOfLine + elif self.selection_state is browsertab.SelectionState.line: + return + else: + act = QWebPage.MoveToEndOfLine self._widget.triggerPageAction(act) def move_to_start_of_next_block(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToNextLine, - QWebPage.MoveToStartOfBlock] - else: + if self.selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectNextLine, QWebPage.SelectStartOfBlock] + else: + act = [QWebPage.MoveToNextLine, + QWebPage.MoveToStartOfBlock] for _ in range(count): for a in act: self._widget.triggerPageAction(a) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_end() def move_to_start_of_prev_block(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToPreviousLine, - QWebPage.MoveToStartOfBlock] - else: + if self.selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectPreviousLine, QWebPage.SelectStartOfBlock] + else: + act = [QWebPage.MoveToPreviousLine, + QWebPage.MoveToStartOfBlock] for _ in range(count): for a in act: self._widget.triggerPageAction(a) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_start() def move_to_end_of_next_block(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToNextLine, - QWebPage.MoveToEndOfBlock] - else: + if self.selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectNextLine, QWebPage.SelectEndOfBlock] + else: + act = [QWebPage.MoveToNextLine, + QWebPage.MoveToEndOfBlock] for _ in range(count): for a in act: self._widget.triggerPageAction(a) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_end() def move_to_end_of_prev_block(self, count=1): - if not self.selection_enabled: - act = [QWebPage.MoveToPreviousLine, QWebPage.MoveToEndOfBlock] - else: + if self.selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectPreviousLine, QWebPage.SelectEndOfBlock] + else: + act = [QWebPage.MoveToPreviousLine, QWebPage.MoveToEndOfBlock] for _ in range(count): for a in act: self._widget.triggerPageAction(a) + if self.selection_state is browsertab.SelectionState.line: + self._select_line_to_start() def move_to_start_of_document(self): - if not self.selection_enabled: - act = QWebPage.MoveToStartOfDocument - else: + if self.selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectStartOfDocument + else: + act = QWebPage.MoveToStartOfDocument self._widget.triggerPageAction(act) + if self.selection_state is browsertab.SelectionState.line: + self._select_line() def move_to_end_of_document(self): - if not self.selection_enabled: - act = QWebPage.MoveToEndOfDocument - else: + if self.selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectEndOfDocument + else: + act = QWebPage.MoveToEndOfDocument self._widget.triggerPageAction(act) - def toggle_selection(self): - self.selection_enabled = not self.selection_enabled - self.selection_toggled.emit(self.selection_enabled) + def toggle_selection(self, line=False): + if line: + self.selection_state = browsertab.SelectionState.line + self._select_line() + self.reverse_selection() + self._select_line() + self.reverse_selection() + elif self.selection_state is not browsertab.SelectionState.normal: + self.selection_state = browsertab.SelectionState.normal + else: + self.selection_state = browsertab.SelectionState.none + self.selection_toggled.emit(self.selection_state) def drop_selection(self): self._widget.triggerPageAction(QWebPage.MoveToNextChar) @@ -362,6 +404,35 @@ class WebKitCaret(browsertab.AbstractCaret): ); }""") + def _select_line(self): + self._widget.triggerPageAction(QWebPage.SelectStartOfLine) + self.reverse_selection() + self._widget.triggerPageAction(QWebPage.SelectEndOfLine) + self.reverse_selection() + + def _select_line_to_end(self): + # direction of selection (if anchor is to the left or right + # of focus) has to be checked before moving selection + # to the end of line + direction = self._js_selection_direction() + if direction: + self._widget.triggerPageAction(QWebPage.SelectEndOfLine) + + def _select_line_to_start(self): + direction = self._js_selection_direction() + if not direction: + self._widget.triggerPageAction(QWebPage.SelectStartOfLine) + + def _js_selection_direction(self): + # return true if selection's direction + # is left to right else false + return self._tab.private_api.run_js_sync(""" + var sel = window.getSelection(); + var position = sel.anchorNode.compareDocumentPosition(sel.focusNode); + (!position && sel.anchorOffset < sel.focusOffset || + position === Node.DOCUMENT_POSITION_FOLLOWING); + """) + def _follow_selected(self, *, tab=False): if QWebSettings.globalSettings().testAttribute( QWebSettings.JavascriptEnabled): @@ -693,6 +764,11 @@ class WebKitTabPrivate(browsertab.AbstractTabPrivate): def shutdown(self): self._widget.shutdown() + def run_js_sync(self, code): + document_element = self._widget.page().mainFrame().documentElement() + result = document_element.evaluateJavaScript(code) + return result + class WebKitTab(browsertab.AbstractTab): @@ -751,8 +827,7 @@ class WebKitTab(browsertab.AbstractTab): def run_js_async(self, code, callback=None, *, world=None): if world is not None and world != usertypes.JsWorld.jseval: log.webview.warning("Ignoring world ID {}".format(world)) - document_element = self._widget.page().mainFrame().documentElement() - result = document_element.evaluateJavaScript(code) + result = self.private_api.run_js_sync(code) if callback is not None: callback(result) From 7db846e6443ef3f8955a554dc92e94cd5f88657c Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Mon, 27 Apr 2020 17:45:31 +0200 Subject: [PATCH 03/17] Harden caret tests and always show window With newer Qt versions, it looks like the caret window always needs to be shown for the selection to work correctly. However, the test silently passed when no selection was available. --- tests/unit/browser/test_caret.py | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 9b817c4ac..fb940f7c0 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -29,6 +29,8 @@ from qutebrowser.utils import utils, qtutils, usertypes @pytest.fixture def caret(web_tab, qtbot, mode_manager): + web_tab.container.expose() + with qtbot.wait_signal(web_tab.load_finished): web_tab.load_url(QUrl('qute://testdata/data/caret.html')) @@ -61,9 +63,13 @@ class Selection: selection = selection.strip() assert selection == expected return + elif not selection and not expected: + return self._qtbot.wait(50) + assert False, 'Failed to get selection!' + def check_multiline(self, expected, *, strip=False): self.check(textwrap.dedent(expected).strip(), strip=strip) @@ -287,17 +293,6 @@ def test_drop_selection(caret, selection): class TestSearch: - @pytest.fixture(autouse=True) - def expose(self, web_tab): - """Expose the web view if needed. - - With QtWebEngine 5.13 on macOS/Windows, searching fails (callback - called with False) when the view isn't exposed. - """ - if qtutils.version_check('5.13') and not utils.is_linux: - web_tab.container.expose() - web_tab.show() - # https://bugreports.qt.io/browse/QTBUG-60673 @pytest.mark.qtbug60673 @@ -340,15 +335,6 @@ class TestFollowSelected: def toggle_js(self, request, config_stub): config_stub.val.content.javascript.enabled = request.param - @pytest.fixture(autouse=True) - def expose(self, web_tab): - """Expose the web view if needed. - - On QtWebKit, or Qt < 5.11 and > 5.12 on QtWebEngine, we need to - show the tab for selections to work properly. - """ - web_tab.container.expose() - def test_follow_selected_without_a_selection(self, qtbot, caret, selection, web_tab, mode_manager): caret.move_to_next_word() # Move cursor away from the link From e0a9088f6395fe791568dff8c157fb06d208165f Mon Sep 17 00:00:00 2001 From: svetelna Date: Tue, 28 Apr 2020 17:45:34 +0200 Subject: [PATCH 04/17] added tests --- tests/unit/browser/test_caret.py | 93 ++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index fb940f7c0..e5ab90708 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -24,7 +24,7 @@ import textwrap import pytest from PyQt5.QtCore import QUrl -from qutebrowser.utils import utils, qtutils, usertypes +from qutebrowser.utils import usertypes @pytest.fixture @@ -73,9 +73,9 @@ class Selection: def check_multiline(self, expected, *, strip=False): self.check(textwrap.dedent(expected).strip(), strip=strip) - def toggle(self): + def toggle(self, line=False): with self._qtbot.wait_signal(self._caret.selection_toggled): - self._caret.toggle_selection() + self._caret.toggle_selection(line=line) @pytest.fixture @@ -391,3 +391,90 @@ class TestReverse: caret.reverse_selection() caret.move_to_start_of_line() selection.check("one two three") + + +class TestLineSelection: + + def test_toggle(self, caret, selection): + selection.toggle(True) + selection.check("one two three") + + def test_toggle_untoggle(self, caret, selection): + selection.toggle() + selection.check("") + selection.toggle(True) + selection.check("one two three") + selection.toggle() + selection.check("one two three") + + def test_from_center(self, caret, selection): + caret.move_to_next_char(4) + selection.toggle(True) + selection.check("one two three") + + def test_more_lines(self, caret, selection): + selection.toggle(True) + caret.move_to_next_line(2) + selection.check_multiline(""" + one two three + eins zwei drei + + four five six + """, strip=True) + + def test_not_selecting_char(self, caret, selection): + selection.toggle(True) + caret.move_to_next_char() + selection.check("one two three") + caret.move_to_prev_char() + selection.check("one two three") + + def test_selecting_prev_next_word(self, caret, selection): + selection.toggle(True) + caret.move_to_next_word() + selection.check("one two three") + caret.move_to_prev_word() + selection.check("one two three") + + def test_selecting_end_word(self, caret, selection): + selection.toggle(True) + caret.move_to_end_of_word() + selection.check("one two three") + + def test_selecting_prev_next_line(self, caret, selection): + selection.toggle(True) + caret.move_to_next_line() + selection.check_multiline(""" + one two three + eins zwei drei + """, strip=True) + caret.move_to_prev_line() + selection.check("one two three") + + def test_not_selecting_start_end_line(self, caret, selection): + selection.toggle(True) + caret.move_to_end_of_line() + selection.check("one two three") + caret.move_to_start_of_line() + selection.check("one two three") + + def test_selecting_block(self, caret, selection): + selection.toggle(True) + caret.move_to_end_of_next_block() + selection.check_multiline(""" + one two three + eins zwei drei + """, strip=True) + + def test_selecting_start_end_document(self, caret, selection): + selection.toggle(True) + caret.move_to_end_of_document() + selection.check_multiline(""" + one two three + eins zwei drei + + four five six + vier fünf sechs + """, strip=True) + caret.move_to_start_of_document() + selection.check("one two three") From 31af9cc54b835064ddd1eb12ab38293e1eb902a1 Mon Sep 17 00:00:00 2001 From: svetelna Date: Mon, 11 May 2020 10:48:43 +0200 Subject: [PATCH 05/17] removed selectionEnabled function in caret.js --- qutebrowser/javascript/caret.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/qutebrowser/javascript/caret.js b/qutebrowser/javascript/caret.js index 6d7a3bb00..51a1cf7cf 100644 --- a/qutebrowser/javascript/caret.js +++ b/qutebrowser/javascript/caret.js @@ -1184,13 +1184,6 @@ window._qutebrowser.caret = (function() { } }; - CaretBrowsing.selectionEnabled = function() { - if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.NONE) { - return false; - } - return true; - }; - CaretBrowsing.move = function(direction, granularity, count = 1) { let action = "move"; if (CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NONE) { @@ -1348,14 +1341,14 @@ window._qutebrowser.caret = (function() { funcs.setInitialCursor = () => { if (!CaretBrowsing.initiated) { CaretBrowsing.setInitialCursor(); - return CaretBrowsing.selectionEnabled(); + return CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NONE; } if (window.getSelection().toString().length === 0) { positionCaret(); } CaretBrowsing.toggle(); - return CaretBrowsing.selectionEnabled(); + return CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NONE; }; funcs.setFlags = (flags) => { From 9b8767b64eea1498e383ae9ef790cb377bfb7508 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 17:46:45 +0200 Subject: [PATCH 06/17] tests: Make sure WidgetContainer.expose() sets focus on widget This is needed to convert the searched string into a selection properly. Fixes #5363 See #5393 --- tests/helpers/fixtures.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py index 51fd98272..eb4186894 100644 --- a/tests/helpers/fixtures.py +++ b/tests/helpers/fixtures.py @@ -64,14 +64,17 @@ class WidgetContainer(QWidget): self._qtbot = qtbot self.vbox = QVBoxLayout(self) qtbot.add_widget(self) + self._widget = None def set_widget(self, widget): self.vbox.addWidget(widget) widget.container = self + self._widget = widget def expose(self): with self._qtbot.waitExposed(self): self.show() + self._widget.setFocus() @pytest.fixture From d2f3bc4bad6b519b972472b9fb32dcb0b4f2f85a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 18:06:54 +0200 Subject: [PATCH 07/17] tests: Increase timeout for caret.html loading For some odd reason, loading the file sometimes takes >5s on macOS... See https://github.com/qutebrowser/qutebrowser/issues/5390#issuecomment-629269038 --- tests/unit/browser/test_caret.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index e5ab90708..13994a654 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -31,7 +31,7 @@ from qutebrowser.utils import usertypes def caret(web_tab, qtbot, mode_manager): web_tab.container.expose() - with qtbot.wait_signal(web_tab.load_finished): + with qtbot.wait_signal(web_tab.load_finished, timeout=10000): web_tab.load_url(QUrl('qute://testdata/data/caret.html')) mode_manager.enter(usertypes.KeyMode.caret) From 781a68a10e10f559575f23c6ed8faa2fb46ab7d9 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 18:26:58 +0200 Subject: [PATCH 08/17] tests: make sure webengine_tab/webkit_tab take care of shutdown This still doesn't help with this inside Xvfb: XIO: fatal IO error 0 (Success) on X server ":1001" But at least it prevents an unknown segfault inside QtWebEngine. --- tests/helpers/fixtures.py | 23 ++++++++++++++++++++--- tests/helpers/stubs.py | 4 ++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py index eb4186894..0624ef698 100644 --- a/tests/helpers/fixtures.py +++ b/tests/helpers/fixtures.py @@ -45,11 +45,12 @@ import helpers.stubs as stubsmod from qutebrowser.config import (config, configdata, configtypes, configexc, configfiles, configcache, stylesheet) from qutebrowser.api import config as configapi -from qutebrowser.utils import objreg, standarddir, utils, usertypes +from qutebrowser.utils import objreg, standarddir, utils, usertypes, qtutils from qutebrowser.browser import greasemonkey, history, qutescheme from qutebrowser.browser.webkit import cookies, cache from qutebrowser.misc import savemanager, sql, objects, sessions from qutebrowser.keyinput import modeman +from qutebrowser.qt import sip _qute_scheme_handler = None @@ -207,14 +208,17 @@ def web_tab_setup(qtbot, tab_registry, session_manager_stub, @pytest.fixture def webkit_tab(web_tab_setup, qtbot, cookiejar_and_cache, mode_manager, - widget_container, webpage): + widget_container, download_stub, webpage): webkittab = pytest.importorskip('qutebrowser.browser.webkit.webkittab') tab = webkittab.WebKitTab(win_id=0, mode_manager=mode_manager, private=False) widget_container.set_widget(tab) - return tab + yield tab + + # Make sure the tab shuts itself down properly + tab.private_api.shutdown() @pytest.fixture @@ -230,11 +234,24 @@ def webengine_tab(web_tab_setup, qtbot, redirect_webengine_data, tab = webenginetab.WebEngineTab(win_id=0, mode_manager=mode_manager, private=False) widget_container.set_widget(tab) + yield tab + # If a page is still loading here, _on_load_finished could get called # during teardown when session_manager_stub is already deleted. tab.stop() + # Make sure the tab shuts itself down properly + tab.private_api.shutdown() + + # If we wait for the GC to clean things up, there's a segfault inside + # QtWebEngine sometimes (e.g. if we only run + # tests/unit/browser/test_caret.py). + # However, with Qt < 5.12, doing this here will lead to an immediate + # segfault... + if qtutils.version_check('5.12'): + sip.delete(tab._widget) + @pytest.fixture(params=['webkit', 'webengine']) def web_tab(request): diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index caa7aac3f..f775092f5 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -615,6 +615,10 @@ class FakeDownloadManager: self.downloads.append(download_item) return download_item + def has_downloads_with_nam(self, _nam): + """Needed during WebView.shutdown().""" + return False + class FakeHistoryProgress: From 0f839b848b9e7738d14bce3311e3cacf8e02019f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 19:12:49 +0200 Subject: [PATCH 09/17] caret: Style improvements for line mode --- qutebrowser/browser/browsertab.py | 1 - qutebrowser/browser/webkit/webkittab.py | 24 ++++++++++-------------- qutebrowser/javascript/caret.js | 2 +- tests/unit/browser/test_caret.py | 25 +++++++++++++------------ 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 16ddeef8b..f37d143b6 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -441,7 +441,6 @@ class AbstractCaret(QObject): """Attribute ``caret`` of AbstractTab for caret browsing.""" #: Signal emitted when the selection was toggled. - #: (argument - whether the selection is now active) selection_toggled = pyqtSignal(SelectionState) #: Emitted when a ``follow_selection`` action is done. follow_selected_done = pyqtSignal() diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index 0a60e073b..e73833f67 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -204,8 +204,7 @@ class WebKitCaret(browsertab.AbstractCaret): def _on_mode_left(self, _mode): settings = self._widget.settings() if settings.testAttribute(QWebSettings.CaretBrowsingEnabled): - if (self.selection_state is not - browsertab.SelectionState.none and + if (self.selection_state is not browsertab.SelectionState.none and self._widget.hasSelection()): # Remove selection if it exists self._widget.triggerPageAction(QWebPage.MoveToNextChar) @@ -414,24 +413,21 @@ class WebKitCaret(browsertab.AbstractCaret): # direction of selection (if anchor is to the left or right # of focus) has to be checked before moving selection # to the end of line - direction = self._js_selection_direction() - if direction: + if self._js_selection_left_to_right(): self._widget.triggerPageAction(QWebPage.SelectEndOfLine) def _select_line_to_start(self): - direction = self._js_selection_direction() - if not direction: + if not self._js_selection_left_to_right(): self._widget.triggerPageAction(QWebPage.SelectStartOfLine) - def _js_selection_direction(self): - # return true if selection's direction - # is left to right else false + def _js_selection_left_to_right(self): + """Return True iff the selection's direction is left to right.""" return self._tab.private_api.run_js_sync(""" - var sel = window.getSelection(); - var position = sel.anchorNode.compareDocumentPosition(sel.focusNode); - (!position && sel.anchorOffset < sel.focusOffset || - position === Node.DOCUMENT_POSITION_FOLLOWING); - """) + var sel = window.getSelection(); + var position = sel.anchorNode.compareDocumentPosition(sel.focusNode); + (!position && sel.anchorOffset < sel.focusOffset || + position === Node.DOCUMENT_POSITION_FOLLOWING); + """) def _follow_selected(self, *, tab=False): if QWebSettings.globalSettings().testAttribute( diff --git a/qutebrowser/javascript/caret.js b/qutebrowser/javascript/caret.js index 51a1cf7cf..e2063a2d4 100644 --- a/qutebrowser/javascript/caret.js +++ b/qutebrowser/javascript/caret.js @@ -1176,7 +1176,7 @@ window._qutebrowser.caret = (function() { }; CaretBrowsing.updateLineSelection = function(direction, granularity) { - if (!(granularity === "character") && !(granularity === "word")) { + if (granularity !== "character" && granularity !== "word") { window. getSelection(). modify("extend", direction, granularity); diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 13994a654..2b65081f0 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -73,7 +73,7 @@ class Selection: def check_multiline(self, expected, *, strip=False): self.check(textwrap.dedent(expected).strip(), strip=strip) - def toggle(self, line=False): + def toggle(self, *, line=False): with self._qtbot.wait_signal(self._caret.selection_toggled): self._caret.toggle_selection(line=line) @@ -396,24 +396,24 @@ class TestReverse: class TestLineSelection: def test_toggle(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) selection.check("one two three") def test_toggle_untoggle(self, caret, selection): selection.toggle() selection.check("") - selection.toggle(True) + selection.toggle(line=True) selection.check("one two three") selection.toggle() selection.check("one two three") def test_from_center(self, caret, selection): caret.move_to_next_char(4) - selection.toggle(True) + selection.toggle(line=True) selection.check("one two three") def test_more_lines(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_next_line(2) selection.check_multiline(""" one two three @@ -423,26 +423,26 @@ class TestLineSelection: """, strip=True) def test_not_selecting_char(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_next_char() selection.check("one two three") caret.move_to_prev_char() selection.check("one two three") def test_selecting_prev_next_word(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_next_word() selection.check("one two three") caret.move_to_prev_word() selection.check("one two three") def test_selecting_end_word(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_end_of_word() selection.check("one two three") def test_selecting_prev_next_line(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_next_line() selection.check_multiline(""" one two three @@ -452,14 +452,14 @@ class TestLineSelection: selection.check("one two three") def test_not_selecting_start_end_line(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_end_of_line() selection.check("one two three") caret.move_to_start_of_line() selection.check("one two three") def test_selecting_block(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_end_of_next_block() selection.check_multiline(""" one two three @@ -467,7 +467,7 @@ class TestLineSelection: """, strip=True) def test_selecting_start_end_document(self, caret, selection): - selection.toggle(True) + selection.toggle(line=True) caret.move_to_end_of_document() selection.check_multiline(""" one two three @@ -476,5 +476,6 @@ class TestLineSelection: four five six vier fünf sechs """, strip=True) + caret.move_to_start_of_document() selection.check("one two three") From 118882b861105428fc698d2c5cea6aecbd59b2ee Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 19:13:31 +0200 Subject: [PATCH 10/17] tests: Skip failing caret test on macOS See #5459 --- tests/unit/browser/test_caret.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 2b65081f0..3aff8cce1 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -466,6 +466,8 @@ class TestLineSelection: eins zwei drei """, strip=True) + @pytest.mark.not_mac( + reason='https://github.com/qutebrowser/qutebrowser/issues/5459') def test_selecting_start_end_document(self, caret, selection): selection.toggle(line=True) caret.move_to_end_of_document() From 374d28de77d926d207b9658b23cd16dd1990a9fc Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 19:50:32 +0200 Subject: [PATCH 11/17] webkittab: Make selection_state private The previous selection_enabled boolean was public, but didn't need to be since e50068021d084cf01507cd20693318189193e073. --- qutebrowser/browser/browsertab.py | 1 - qutebrowser/browser/webkit/webkittab.py | 89 ++++++++++++++----------- 2 files changed, 49 insertions(+), 41 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index f37d143b6..d11ec2fa1 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -452,7 +452,6 @@ class AbstractCaret(QObject): super().__init__(parent) self._tab = tab self._widget = typing.cast(QWidget, None) - self.selection_state = SelectionState.none self._mode_manager = mode_manager mode_manager.entered.connect(self._on_mode_entered) mode_manager.left.connect(self._on_mode_left) diff --git a/qutebrowser/browser/webkit/webkittab.py b/qutebrowser/browser/webkit/webkittab.py index e73833f67..ac10539e8 100644 --- a/qutebrowser/browser/webkit/webkittab.py +++ b/qutebrowser/browser/webkit/webkittab.py @@ -25,6 +25,7 @@ import xml.etree.ElementTree from PyQt5.QtCore import pyqtSlot, Qt, QUrl, QPoint, QTimer, QSizeF, QSize from PyQt5.QtGui import QIcon +from PyQt5.QtWidgets import QWidget from PyQt5.QtWebKitWidgets import QWebPage, QWebFrame from PyQt5.QtWebKit import QWebSettings from PyQt5.QtPrintSupport import QPrinter @@ -34,6 +35,7 @@ from qutebrowser.browser.webkit import (webview, tabhistory, webkitelem, webkitsettings) from qutebrowser.utils import qtutils, usertypes, utils, log, debug from qutebrowser.qt import sip +from qutebrowser.keyinput import modeman class WebKitAction(browsertab.AbstractAction): @@ -172,16 +174,23 @@ class WebKitCaret(browsertab.AbstractCaret): """QtWebKit implementations related to moving the cursor/selection.""" + def __init__(self, + tab: browsertab.AbstractTab, + mode_manager: modeman.ModeManager, + parent: QWidget = None) -> None: + super().__init__(tab, mode_manager, parent) + self._selection_state = browsertab.SelectionState.none + @pyqtSlot(usertypes.KeyMode) def _on_mode_entered(self, mode): if mode != usertypes.KeyMode.caret: return if self._widget.hasSelection(): - self.selection_state = browsertab.SelectionState.normal + self._selection_state = browsertab.SelectionState.normal else: - self.selection_state = browsertab.SelectionState.none - self.selection_toggled.emit(self.selection_state) + self._selection_state = browsertab.SelectionState.none + self.selection_toggled.emit(self._selection_state) settings = self._widget.settings() settings.setAttribute(QWebSettings.CaretBrowsingEnabled, True) @@ -196,7 +205,7 @@ class WebKitCaret(browsertab.AbstractCaret): # # Note: We can't use hasSelection() here, as that's always # true in caret mode. - if self.selection_state is browsertab.SelectionState.none: + if self._selection_state is browsertab.SelectionState.none: self._widget.page().currentFrame().evaluateJavaScript( utils.read_file('javascript/position_caret.js')) @@ -204,37 +213,37 @@ class WebKitCaret(browsertab.AbstractCaret): def _on_mode_left(self, _mode): settings = self._widget.settings() if settings.testAttribute(QWebSettings.CaretBrowsingEnabled): - if (self.selection_state is not browsertab.SelectionState.none and + if (self._selection_state is not browsertab.SelectionState.none and self._widget.hasSelection()): # Remove selection if it exists self._widget.triggerPageAction(QWebPage.MoveToNextChar) settings.setAttribute(QWebSettings.CaretBrowsingEnabled, False) - self.selection_state = browsertab.SelectionState.none + self._selection_state = browsertab.SelectionState.none def move_to_next_line(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectNextLine else: act = QWebPage.MoveToNextLine for _ in range(count): self._widget.triggerPageAction(act) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_end() def move_to_prev_line(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectPreviousLine else: act = QWebPage.MoveToPreviousLine for _ in range(count): self._widget.triggerPageAction(act) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_start() def move_to_next_char(self, count=1): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectNextChar - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = QWebPage.MoveToNextChar @@ -242,9 +251,9 @@ class WebKitCaret(browsertab.AbstractCaret): self._widget.triggerPageAction(act) def move_to_prev_char(self, count=1): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectPreviousChar - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = QWebPage.MoveToPreviousChar @@ -252,11 +261,11 @@ class WebKitCaret(browsertab.AbstractCaret): self._widget.triggerPageAction(act) def move_to_end_of_word(self, count=1): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = [QWebPage.SelectNextWord] if utils.is_windows: # pragma: no cover act.append(QWebPage.SelectPreviousChar) - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = [QWebPage.MoveToNextWord] @@ -267,11 +276,11 @@ class WebKitCaret(browsertab.AbstractCaret): self._widget.triggerPageAction(a) def move_to_next_word(self, count=1): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = [QWebPage.SelectNextWord] if not utils.is_windows: # pragma: no branch act.append(QWebPage.SelectNextChar) - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = [QWebPage.MoveToNextWord] @@ -282,9 +291,9 @@ class WebKitCaret(browsertab.AbstractCaret): self._widget.triggerPageAction(a) def move_to_prev_word(self, count=1): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectPreviousWord - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = QWebPage.MoveToPreviousWord @@ -292,25 +301,25 @@ class WebKitCaret(browsertab.AbstractCaret): self._widget.triggerPageAction(act) def move_to_start_of_line(self): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectStartOfLine - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = QWebPage.MoveToStartOfLine self._widget.triggerPageAction(act) def move_to_end_of_line(self): - if self.selection_state is browsertab.SelectionState.normal: + if self._selection_state is browsertab.SelectionState.normal: act = QWebPage.SelectEndOfLine - elif self.selection_state is browsertab.SelectionState.line: + elif self._selection_state is browsertab.SelectionState.line: return else: act = QWebPage.MoveToEndOfLine self._widget.triggerPageAction(act) def move_to_start_of_next_block(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectNextLine, QWebPage.SelectStartOfBlock] else: @@ -319,11 +328,11 @@ class WebKitCaret(browsertab.AbstractCaret): for _ in range(count): for a in act: self._widget.triggerPageAction(a) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_end() def move_to_start_of_prev_block(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectPreviousLine, QWebPage.SelectStartOfBlock] else: @@ -332,11 +341,11 @@ class WebKitCaret(browsertab.AbstractCaret): for _ in range(count): for a in act: self._widget.triggerPageAction(a) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_start() def move_to_end_of_next_block(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectNextLine, QWebPage.SelectEndOfBlock] else: @@ -345,31 +354,31 @@ class WebKitCaret(browsertab.AbstractCaret): for _ in range(count): for a in act: self._widget.triggerPageAction(a) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_end() def move_to_end_of_prev_block(self, count=1): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = [QWebPage.SelectPreviousLine, QWebPage.SelectEndOfBlock] else: act = [QWebPage.MoveToPreviousLine, QWebPage.MoveToEndOfBlock] for _ in range(count): for a in act: self._widget.triggerPageAction(a) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line_to_start() def move_to_start_of_document(self): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectStartOfDocument else: act = QWebPage.MoveToStartOfDocument self._widget.triggerPageAction(act) - if self.selection_state is browsertab.SelectionState.line: + if self._selection_state is browsertab.SelectionState.line: self._select_line() def move_to_end_of_document(self): - if self.selection_state is not browsertab.SelectionState.none: + if self._selection_state is not browsertab.SelectionState.none: act = QWebPage.SelectEndOfDocument else: act = QWebPage.MoveToEndOfDocument @@ -377,16 +386,16 @@ class WebKitCaret(browsertab.AbstractCaret): def toggle_selection(self, line=False): if line: - self.selection_state = browsertab.SelectionState.line + self._selection_state = browsertab.SelectionState.line self._select_line() self.reverse_selection() self._select_line() self.reverse_selection() - elif self.selection_state is not browsertab.SelectionState.normal: - self.selection_state = browsertab.SelectionState.normal + elif self._selection_state is not browsertab.SelectionState.normal: + self._selection_state = browsertab.SelectionState.normal else: - self.selection_state = browsertab.SelectionState.none - self.selection_toggled.emit(self.selection_state) + self._selection_state = browsertab.SelectionState.none + self.selection_toggled.emit(self._selection_state) def drop_selection(self): self._widget.triggerPageAction(QWebPage.MoveToNextChar) From 0770ef11ebeac8072158a639c9297d5c4ef14dfb Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 20:01:08 +0200 Subject: [PATCH 12/17] tests: Make sure entering caret mode is finished Otherwise, the caret JS code could still be initializing while the tests start to run. --- tests/unit/browser/test_caret.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 3aff8cce1..830dda0ff 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -34,7 +34,8 @@ def caret(web_tab, qtbot, mode_manager): with qtbot.wait_signal(web_tab.load_finished, timeout=10000): web_tab.load_url(QUrl('qute://testdata/data/caret.html')) - mode_manager.enter(usertypes.KeyMode.caret) + with qtbot.wait_signal(web_tab.caret.selection_toggled): + mode_manager.enter(usertypes.KeyMode.caret) return web_tab.caret From 7a0cbf54fce491a48a5518c6c7422ab5d998d0a2 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 20:01:51 +0200 Subject: [PATCH 13/17] caret: Fix toggling behavior with QtWebEngine The behavior when pressing `v` in line selection mode was different between QtWebKit and QtWebEngine: With QtWebKit, normal selection mode was entered, while with QtWebEngine, selection mode was left. Do the former with QtWebEngine as well, as that's also what vim does. --- qutebrowser/javascript/caret.js | 2 +- tests/unit/browser/test_caret.py | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/qutebrowser/javascript/caret.js b/qutebrowser/javascript/caret.js index e2063a2d4..2d0bced84 100644 --- a/qutebrowser/javascript/caret.js +++ b/qutebrowser/javascript/caret.js @@ -1452,7 +1452,7 @@ window._qutebrowser.caret = (function() { CaretBrowsing.SelectionState.LINE; CaretBrowsing.selectLine(); CaretBrowsing.finishMove(); - } else if (CaretBrowsing.selectionState === CaretBrowsing.SelectionState.NONE) { + } else if (CaretBrowsing.selectionState !== CaretBrowsing.SelectionState.NORMAL) { CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NORMAL; } else { CaretBrowsing.selectionState = CaretBrowsing.SelectionState.NONE; diff --git a/tests/unit/browser/test_caret.py b/tests/unit/browser/test_caret.py index 830dda0ff..7d1325612 100644 --- a/tests/unit/browser/test_caret.py +++ b/tests/unit/browser/test_caret.py @@ -25,6 +25,7 @@ import pytest from PyQt5.QtCore import QUrl from qutebrowser.utils import usertypes +from qutebrowser.browser import browsertab @pytest.fixture @@ -75,8 +76,10 @@ class Selection: self.check(textwrap.dedent(expected).strip(), strip=strip) def toggle(self, *, line=False): - with self._qtbot.wait_signal(self._caret.selection_toggled): + """Toggle the selection and return the new selection state.""" + with self._qtbot.wait_signal(self._caret.selection_toggled) as blocker: self._caret.toggle_selection(line=line) + return blocker.args[0] @pytest.fixture @@ -84,6 +87,18 @@ def selection(qtbot, caret): return Selection(qtbot, caret) +def test_toggle(caret, selection, qtbot): + """Make sure calling toggleSelection produces the correct callback values. + + This also makes sure that the SelectionState enum in JS lines up with the + Python browsertab.SelectionState enum. + """ + assert selection.toggle() == browsertab.SelectionState.normal + assert selection.toggle(line=True) == browsertab.SelectionState.line + assert selection.toggle() == browsertab.SelectionState.normal + assert selection.toggle() == browsertab.SelectionState.none + + class TestDocument: def test_selecting_entire_document(self, caret, selection): From d57fe79f5f568113f97392c91087702bddf0fec3 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 20:05:38 +0200 Subject: [PATCH 14/17] caret: Use strings instead of ints for enums --- qutebrowser/browser/browsertab.py | 7 +++++-- qutebrowser/browser/webengine/webenginetab.py | 4 ++-- qutebrowser/javascript/caret.js | 10 ++++++---- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index d11ec2fa1..ba758abc7 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -427,9 +427,12 @@ class AbstractZoom(QObject): self._set_factor_internal(self._zoom_factor) -class SelectionState(enum.IntEnum): +class SelectionState(enum.Enum): - """Possible states of selection in Caret mode.""" + """Possible states of selection in caret mode. + + NOTE: Names need to line up with SelectionState in caret.js! + """ none = 1 normal = 2 diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 5b0721c18..33db6e631 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -504,8 +504,8 @@ class WebEngineCaret(browsertab.AbstractCaret): code = javascript.assemble('caret', command, *args) self._tab.run_js_async(code, callback) - def _toggle_sel_translate(self, state_int): - state = browsertab.SelectionState(state_int) + def _toggle_sel_translate(self, state_str): + state = browsertab.SelectionState[state_str] self.selection_toggled.emit(state) diff --git a/qutebrowser/javascript/caret.js b/qutebrowser/javascript/caret.js index 2d0bced84..d7ba88fe6 100644 --- a/qutebrowser/javascript/caret.js +++ b/qutebrowser/javascript/caret.js @@ -706,13 +706,15 @@ window._qutebrowser.caret = (function() { CaretBrowsing.isCaretVisible = false; /** - * selection modes + * Selection modes. + * NOTE: Values need to line up with SelectionState in browsertab.py! + * * @type {enum} */ CaretBrowsing.SelectionState = { - "NONE": 1, - "NORMAL": 2, - "LINE": 3, + "NONE": "none", + "NORMAL": "normal", + "LINE": "line", }; /** From dcab5eb11b8b018bb296475c24e5716a2ac415ca Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 17:12:51 +0200 Subject: [PATCH 15/17] src2asciidoc: Make sure usage lines are generated with fixed width Due to a change in Python 3.8, the output depended on the calling terminal's width. Set a fixed with of 200 (rather than 80) so that we always have the expanded version for the generated documentation. See #5393 and https://github.com/python/cpython/commit/74102c9a5f2327c4fc47feefa072854a53551d1f#diff-837b312b1f3508216ace6adb46492836 --- doc/help/commands.asciidoc | 17 +++++------------ scripts/dev/src2asciidoc.py | 5 +++++ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 565b87ac8..3266c7153 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -285,8 +285,7 @@ Set all settings back to their default. [[config-cycle]] === config-cycle -Syntax: +:config-cycle [*--pattern* 'pattern'] [*--temp*] [*--print*] - 'option' ['values' ['values' ...]]+ +Syntax: +:config-cycle [*--pattern* 'pattern'] [*--temp*] [*--print*] 'option' ['values' ['values' ...]]+ Cycle an option between multiple values. @@ -597,8 +596,7 @@ Show help about a command or setting. [[hint]] === hint -Syntax: +:hint [*--mode* 'mode'] [*--add-history*] [*--rapid*] [*--first*] - ['group'] ['target'] ['args' ['args' ...]]+ +Syntax: +:hint [*--mode* 'mode'] [*--add-history*] [*--rapid*] [*--first*] ['group'] ['target'] ['args' ['args' ...]]+ Start hinting. @@ -855,8 +853,7 @@ Do nothing. [[open]] === open -Syntax: +:open [*--related*] [*--bg*] [*--tab*] [*--window*] [*--secure*] [*--private*] - ['url']+ +Syntax: +:open [*--related*] [*--bg*] [*--tab*] [*--window*] [*--secure*] [*--private*] ['url']+ Open a URL in the current/[count]th tab. @@ -1186,9 +1183,7 @@ Load a session. [[session-save]] === session-save -Syntax: +:session-save [*--current*] [*--quiet*] [*--force*] [*--only-active-window*] - [*--with-private*] - ['name']+ +Syntax: +:session-save [*--current*] [*--quiet*] [*--force*] [*--only-active-window*] [*--with-private*] ['name']+ Save a session. @@ -1252,9 +1247,7 @@ Set a mark at the current scroll position in the current tab. [[spawn]] === spawn -Syntax: +:spawn [*--userscript*] [*--verbose*] [*--output*] [*--output-messages*] - [*--detach*] - 'cmdline'+ +Syntax: +:spawn [*--userscript*] [*--verbose*] [*--output*] [*--output-messages*] [*--detach*] 'cmdline'+ Spawn an external command. diff --git a/scripts/dev/src2asciidoc.py b/scripts/dev/src2asciidoc.py index b82ede3e1..1d3ad8bf4 100755 --- a/scripts/dev/src2asciidoc.py +++ b/scripts/dev/src2asciidoc.py @@ -59,6 +59,11 @@ class UsageFormatter(argparse.HelpFormatter): argparse.HelpFormatter while copying 99% of the code :-/ """ + def __init__(self, prog, indent_increment=2, max_help_position=24, + width=200): + """Override __init__ to set a fixed width as default.""" + super().__init__(prog, indent_increment, max_help_position, width) + def _format_usage(self, usage, actions, groups, _prefix): """Override _format_usage to not add the 'usage:' prefix.""" return super()._format_usage(usage, actions, groups, '') From 3d950c7611e5f45d6fc5373c85f64899d51a8111 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 21:29:24 +0200 Subject: [PATCH 16/17] Update docs --- doc/changelog.asciidoc | 1 + doc/help/commands.asciidoc | 5 +++++ doc/help/settings.asciidoc | 1 + 3 files changed, 7 insertions(+) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index 3d39eec85..510ffeef2 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -35,6 +35,7 @@ Added - New `:debug-keytester` command, which shows a "key tester" widget. Previously, that was only available as a separate application via `python3 -m scripts.keytester`. +- New line selection mode (`:toggle-selection --line`), bound to `Shift-V` in caret mode. Fixed ~~~~~ diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 3266c7153..608329fe8 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -1885,8 +1885,13 @@ This acts like readline's yank. [[toggle-selection]] === toggle-selection +Syntax: +:toggle-selection [*--line*]+ + Toggle caret selection mode. +==== optional arguments +* +*-l*+, +*--line*+: Enables line-selection. + == Debugging commands These commands are mainly intended for debugging. They are hidden if qutebrowser was started without the `--debug`-flag. diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 5182968a6..ca1f41fb9 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -444,6 +444,7 @@ Default: * +pass:[J]+: +pass:[scroll down]+ * +pass:[K]+: +pass:[scroll up]+ * +pass:[L]+: +pass:[scroll right]+ +* +pass:[V]+: +pass:[toggle-selection --line]+ * +pass:[Y]+: +pass:[yank selection -s]+ * +pass:[[]+: +pass:[move-to-start-of-prev-block]+ * +pass:[]]+: +pass:[move-to-start-of-next-block]+ From 57bc2b49c6ff6c67a0a8a6967389f03a12473e9f Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Fri, 22 May 2020 21:29:33 +0200 Subject: [PATCH 17/17] Fix segfault with test_webenginetab and Qt 5.9 --- tests/helpers/fixtures.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/helpers/fixtures.py b/tests/helpers/fixtures.py index 0624ef698..aed243b4b 100644 --- a/tests/helpers/fixtures.py +++ b/tests/helpers/fixtures.py @@ -223,7 +223,8 @@ def webkit_tab(web_tab_setup, qtbot, cookiejar_and_cache, mode_manager, @pytest.fixture def webengine_tab(web_tab_setup, qtbot, redirect_webengine_data, - tabbed_browser_stubs, mode_manager, widget_container): + tabbed_browser_stubs, mode_manager, widget_container, + monkeypatch): tabwidget = tabbed_browser_stubs[0].widget tabwidget.current_index = 0 tabwidget.index_of = 0 @@ -249,6 +250,7 @@ def webengine_tab(web_tab_setup, qtbot, redirect_webengine_data, # tests/unit/browser/test_caret.py). # However, with Qt < 5.12, doing this here will lead to an immediate # segfault... + monkeypatch.undo() # version_check could be patched if qtutils.version_check('5.12'): sip.delete(tab._widget)