From df999b05e553af37f90d8dfb758d687448a6a350 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 8 Sep 2026 18:51:02 +0530 Subject: [PATCH] Fix drag gesture state leaking past the button event early returns Follow-ups to the drag_or_normal_select selected text drag support: - Reset the pending drag at the top of handle_button_event again, before the scrollbar, missing screen and mouse position early returns. Moving it into a helper placed it after those returns, so a scrollbar track click (with scrollbar_jump_on_click disabled) no longer cleared an armed URL drag, letting it fire on a later mouse move with no button held. Split the helper in two, as arming the drag needs an up to date mouse position but clearing it does not. - Clear the click queue for all drag kinds, not just text drags. A native drag consumes mouse-up, so the press lingered and turned the next press into a double press, selecting a word instead of starting a new gesture. Reproducible with link drags before this change. - Note that handle_potential_drag can call into Python, which can re-alloc the windows array, so w must not be used after it returns. Also share the drag thumbnail construction between drag_url and drag_selection (and skip the thumbnail rather than dividing by zero on a zero width render), use None instead of -1 for the mock event pixel position so negative positions remain testable, and tighten the off-by-one bound in clear_click_queue. --- kitty/mouse.c | 60 +++++++++++++++++++++-------------- kitty/screen.c | 10 +++--- kitty/window.py | 29 +++++++++-------- kitty_tests/mouse.py | 4 +-- kitty_tests/selection_drag.py | 28 ++++++++++++++++ 5 files changed, 88 insertions(+), 43 deletions(-) diff --git a/kitty/mouse.c b/kitty/mouse.c index e3d19d931..2bdf7d1d6 100644 --- a/kitty/mouse.c +++ b/kitty/mouse.c @@ -656,6 +656,13 @@ distance(double x1, double y1, double x2, double y2) { return sqrt((x1 - x2) * (x1 - x2) + (y1 - y2) * (y1 - y2)); } +static void +clear_click_queue(Window *w, int button) { + if (0 <= button && button < (ssize_t)arraysz(w->click_queues)) w->click_queues[button].length = 0; +} + +// NB: This can call into Python, which can re-alloc the windows array, so w +// must not be used after it returns. Keep it last in its callers. static void handle_potential_drag(Window *w, int button) { if (button != GLFW_MOUSE_BUTTON_LEFT || !w->drag_source.initial_left_press.at || OPT(drag_threshold) <= 0) return; @@ -663,13 +670,14 @@ handle_potential_drag(Window *w, int button) { return; zero_at_ptr(&w->drag_source.initial_left_press); Screen *screen = w->render_data.screen; + // A native drag consumes the release event, including when canceled. Stop + // tracking the local gesture before handing it to the window system, + // otherwise the press lingers and the next one counts as a double click. + clear_click_queue(w, button); if (w->drag_source.potential_drag.type == POTENTIAL_DRAG_SELECTION) { zero_at_ptr(&w->drag_source.potential_drag); - // A native drag consumes the release event, including when canceled. Stop - // tracking the local gesture before handing it to the window system. global_state.active_drag_in_window = 0; global_state.active_drag_button = -1; - w->click_queues[GLFW_MOUSE_BUTTON_LEFT].length = 0; set_mouse_cursor_for_screen(screen); if (screen->callbacks != Py_None) { PyObject *ret = PyObject_CallMethod(screen->callbacks, "drag_selection", NULL); @@ -716,11 +724,6 @@ HANDLER(handle_move_event) { handle_potential_drag(w, button); } -static void -clear_click_queue(Window *w, int button) { - if (0 <= button && button <= (ssize_t)arraysz(w->click_queues)) w->click_queues[button].length = 0; -} - #define N(n) (q->clicks[q->length - n]) static double @@ -877,17 +880,21 @@ dispatch_possible_click(Window *w, int button, int modifiers) { } } +// Must be called for every button event before any early return, otherwise a +// pending drag lingers and fires on a later mouse move with no button held. static void -prepare_mouse_button(Window *w, int button, bool is_release) { +clear_potential_drag(Window *w, int button, bool is_release) { zero_at_ptr(&w->drag_source.potential_drag); - if (button == GLFW_MOUSE_BUTTON_LEFT) { - if (is_release) zero_at_ptr(&w->drag_source.initial_left_press); - else { - w->drag_source.initial_left_press.x = w->mouse_pos.global_x; - w->drag_source.initial_left_press.y = w->mouse_pos.global_y; - w->drag_source.initial_left_press.at = monotonic(); - } - } + if (is_release && button == GLFW_MOUSE_BUTTON_LEFT) zero_at_ptr(&w->drag_source.initial_left_press); +} + +// Needs an up-to-date w->mouse_pos, as the drag threshold is measured from it. +static void +arm_potential_drag(Window *w, int button) { + if (button != GLFW_MOUSE_BUTTON_LEFT) return; + w->drag_source.initial_left_press.x = w->mouse_pos.global_x; + w->drag_source.initial_left_press.y = w->mouse_pos.global_y; + w->drag_source.initial_left_press.at = monotonic(); } HANDLER(handle_button_event) { @@ -897,6 +904,7 @@ HANDLER(handle_button_event) { Tab *t = osw->tabs + osw->active_tab; bool is_release = !osw->mouse_button_pressed[button]; + clear_potential_drag(w, button, is_release); if (handle_scrollbar_mouse(w, button, is_release ? RELEASE : PRESS, modifiers)) return; @@ -907,7 +915,7 @@ HANDLER(handle_button_event) { bool a, b; if (!set_mouse_position(w, &a, &b)) return; - prepare_mouse_button(w, button, is_release); + if (!is_release) arm_potential_drag(w, button); if (button == GLFW_MOUSE_BUTTON_LEFT && !is_release) osw->shader_anim_event_registry |= (1u << SHADER_ANIM_EVENT_POINTER_LEFT_BUTTON_PRESS); id_type wid = w->id; if (!dispatch_mouse_event(w, button, is_release ? -1 : 1, modifiers, screen->modes.mouse_tracking_mode != 0)) { @@ -1729,10 +1737,10 @@ send_mock_mouse_event_to_window(PyObject *self UNUSED, PyObject *args) { PyObject *capsule; int button, modifiers, is_release, clear_clicks, in_left_half_of_cell; unsigned int x, y; - double pixel_x = -1, pixel_y = -1; + PyObject *pixel_x = Py_None, *pixel_y = Py_None; if (!PyArg_ParseTuple( args, - "O!iipIIpp|dd", + "O!iipIIpp|OO", &PyCapsule_Type, &capsule, &button, @@ -1749,8 +1757,13 @@ send_mock_mouse_event_to_window(PyObject *self UNUSED, PyObject *args) { if (!w) return NULL; if (clear_clicks) clear_click_queue(w, button); bool mouse_cell_changed = x != w->mouse_pos.cell_x || y != w->mouse_pos.cell_y || w->mouse_pos.in_left_half_of_cell != in_left_half_of_cell; - w->mouse_pos.global_x = pixel_x < 0 ? 10 * x : pixel_x; - w->mouse_pos.global_y = pixel_y < 0 ? 20 * y : pixel_y; + // None means derive the pixel position from the cell, so that tests that do + // not care about sub-cell positions stay terse. Negative values are legal. + const double px = pixel_x == Py_None ? 10 * x : PyFloat_AsDouble(pixel_x); + const double py = pixel_y == Py_None ? 20 * y : PyFloat_AsDouble(pixel_y); + if (PyErr_Occurred()) return NULL; + w->mouse_pos.global_x = px; + w->mouse_pos.global_y = py; w->mouse_pos.cell_x = x; w->mouse_pos.cell_y = y; w->mouse_pos.in_left_half_of_cell = in_left_half_of_cell; @@ -1767,7 +1780,8 @@ send_mock_mouse_event_to_window(PyObject *self UNUSED, PyObject *args) { if (global_state.active_drag_in_window && is_release && button == global_state.active_drag_button) { end_drag(w); } else { - prepare_mouse_button(w, button, is_release); + clear_potential_drag(w, button, is_release); + if (!is_release) arm_potential_drag(w, button); dispatch_mouse_event(w, button, is_release ? -1 : 1, modifiers, w->render_data.screen->modes.mouse_tracking_mode != NO_TRACKING); if (!is_release) { last_button_pressed = button; diff --git a/kitty/screen.c b/kitty/screen.c index c5b8e9ee4..c8c2c439e 100644 --- a/kitty/screen.c +++ b/kitty/screen.c @@ -4327,11 +4327,11 @@ screen_has_selection(Screen *self) { bool screen_is_cell_selected(Screen *self, index_type x, index_type y) { - if (x >= self->columns || y >= self->lines) return false; + if (x >= self->columns || y >= self->lines || !self->selections.count) return false; const int row = (int)y - self->scrolled_by; - Line *line = checked_range_line(self, row); - if (!line) return false; - const CPUCell cell = line->cpu_cells[x]; + const Line *queried_line = checked_range_line(self, row); + if (!queried_line) return false; + const CPUCell cell = queried_line->cpu_cells[x]; // Every cell of a multi-cell character has the same selection highlight. const int first_row = cell.is_multicell ? row - cell.y : row; const int last_row = cell.is_multicell ? first_row + cell.scale : row + 1; @@ -4342,7 +4342,7 @@ screen_is_cell_selected(Screen *self, index_type x, index_type y) { iteration_data(self->selections.items + i, &idata, self->columns, -self->historybuf->count, 0); const int start = MAX(visible_start, MAX(first_row, idata.y)), end = MIN(visible_end, MIN(last_row, idata.y_limit)); for (int r = start; r < end; r++) { - line = checked_range_line(self, r); + const Line *line = checked_range_line(self, r); if (line) { const XRange xr = xrange_for_iteration_with_multicells(&idata, r, line); if (x >= xr.x && x < xr.x_limit) return true; diff --git a/kitty/window.py b/kitty/window.py index e12625113..16121aa80 100644 --- a/kitty/window.py +++ b/kitty/window.py @@ -326,6 +326,8 @@ DYNAMIC_COLOR_CODES = { 19: DynamicColor.highlight_fg, } DYNAMIC_COLOR_CODES.update({k + 100: v for k, v in DYNAMIC_COLOR_CODES.items()}) +# Maximum number of characters of dragged text shown in the drag thumbnail +MAX_DRAG_PREVIEW_LENGTH = 256 class Watcher: @@ -1417,6 +1419,17 @@ class Window: return False return get_boss().combine(action, window_for_dispatch=self, dispatch_type='MouseEvent') + def drag_thumbnails(self, label: str) -> tuple[tuple[bytes, int, int], ...]: + # Render label as a single line of text, clipped to the width of this window + fg = color_as_int(self.screen.color_profile.default_fg) + bg = color_as_int(self.screen.color_profile.default_bg) + pixels, width = draw_single_line_of_text( + self.os_window_id, f' {label} ', 0xFF000000 | fg, 0xFF000000 | bg, self.geometry.right - self.geometry.left, max_width=True + ) + if width < 1: + return () + return ((pixels, width, len(pixels) // (width * 4)),) + def drag_url(self, url: str, hyperlink_id: int) -> None: if not url: return @@ -1424,15 +1437,9 @@ class Window: from urllib.parse import quote url = 'file://' + quote(os.path.abspath(url)) - fg = color_as_int(self.screen.color_profile.default_fg) - bg = color_as_int(self.screen.color_profile.default_bg) - width = self.geometry.right - self.geometry.left - pixels, width = draw_single_line_of_text(self.os_window_id, f' {url} ', 0xFF000000 | fg, 0xFF000000 | bg, width, max_width=True) - height = len(pixels) // (width * 4) - thumbnails = ((pixels, width, height),) drag_data = {'text/uri-list': (url + '\r\n').encode()} try: - start_drag_with_data(self.os_window_id, drag_data, thumbnails) + start_drag_with_data(self.os_window_id, drag_data, self.drag_thumbnails(url)) except OSError as e: log_error(f'Failed to start URL drag: {e}') @@ -1441,12 +1448,8 @@ class Window: if not text: return # Bound the preview, but always offer the complete selection as plain text. - preview = ' '.join(text[:256].split()) + ('…' if len(text) > 256 else '') - fg = color_as_int(self.screen.color_profile.default_fg) - bg = color_as_int(self.screen.color_profile.default_bg) - width = self.geometry.right - self.geometry.left - pixels, width = draw_single_line_of_text(self.os_window_id, f' {preview} ', 0xFF000000 | fg, 0xFF000000 | bg, width, max_width=True) - thumbnails = ((pixels, width, len(pixels) // (width * 4)),) + preview = ' '.join(text[:MAX_DRAG_PREVIEW_LENGTH].split()) + ('…' if len(text) > MAX_DRAG_PREVIEW_LENGTH else '') + thumbnails = self.drag_thumbnails(preview) data = text.encode('utf-8') drag_data = {'text/plain': data} if not is_macos: diff --git a/kitty_tests/mouse.py b/kitty_tests/mouse.py index ad58236a7..96ea14e11 100644 --- a/kitty_tests/mouse.py +++ b/kitty_tests/mouse.py @@ -25,8 +25,8 @@ def send_mouse_event( x=0.0, y=0, clear_click_queue=False, - pixel_x=-1, - pixel_y=-1, + pixel_x=None, + pixel_y=None, ): ix = int(x) in_left_half_of_cell = x - ix < 0.5 diff --git a/kitty_tests/selection_drag.py b/kitty_tests/selection_drag.py index 66e6f483f..7d026b185 100644 --- a/kitty_tests/selection_drag.py +++ b/kitty_tests/selection_drag.py @@ -11,6 +11,7 @@ from kitty.fast_data_types import ( GLFW_MOD_SHIFT, GLFW_MOUSE_BUTTON_LEFT, GLFW_MOUSE_BUTTON_RIGHT, + MOUSE_SELECTION_DRAG_OR_NORMAL_SELECT, create_mock_window, mock_mouse_selection, set_options, @@ -146,6 +147,32 @@ class TestSelectionDrag(BaseTest): self.ae(urls, ['https://example.com']) self.ae(drags, ['example']) + def test_selection_drag_click_count(self): + s, ev, drags = self.drag_screen() + s.draw('https://example.com') + urls = [] + s.callbacks.drag_url = lambda url, hyperlink_id: urls.append(url) + codes = [] + dispatch = s.callbacks.mouse_selection + + def record(code): + codes.append(code) + dispatch(code) + + s.callbacks.mouse_selection = record + # A native drag consumes mouse-up, so the press must not linger in the + # click queue, otherwise the next press counts as a double press and + # selects a word instead of starting a new gesture. This applies to + # link drags just as much as to text drags. + for i in range(1, 3): + with self.subTest(gesture=i): + ev(x=10) + ev(LEFT, x=10, pixel_x=100) + ev(x=10, pixel_x=106) # exceed the threshold without leaving the cell + self.ae(codes, [MOUSE_SELECTION_DRAG_OR_NORMAL_SELECT] * i) + self.ae(urls, ['https://example.com'] * i) + self.ae(drags, []) + def test_selection_drag_hit_testing(self): for kind in ('forward', 'backward', 'rectangle', 'unicode', 'multicell', 'scrollback', 'alternate'): with self.subTest(kind=kind): @@ -188,6 +215,7 @@ class TestSelectionDrag(BaseTest): geometry=SimpleNamespace(left=0, right=400), os_window_id=1, ) + w.drag_thumbnails = partial(Window.drag_thumbnails, w) with ( patch('kitty.window.draw_single_line_of_text', return_value=(b'\0' * 4, 1)) as draw, patch('kitty.window.start_drag_with_data') as start,