mirror of
https://github.com/kovidgoyal/kitty.git
synced 2026-10-20 12:19:36 +00:00
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.
This commit is contained in:
parent
d90869a8c6
commit
df999b05e5
5 changed files with 88 additions and 43 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue