From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#66655: 29.1; Clicking buttons sometimes doesn't work Date: Mon, 23 Oct 2023 19:00:53 -0400 Message-ID: References: <1d9187b71e7288eaf08ac9a2f0559bdf@gmail.com> <8334y4s0oe.fsf@gnu.org> <83v8axmbsh.fsf@gnu.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16149"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: tomasralph2000@gmail.com, 66655@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Oct 24 01:03:49 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qv3xd-0003y4-0Z for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 24 Oct 2023 01:03:49 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qv3xQ-0001Bu-0S; Mon, 23 Oct 2023 19:03:36 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qv3xO-0001Bi-1U for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2023 19:03:34 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qv3xM-0003vC-H0 for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2023 19:03:33 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qv3xp-00027a-MR for bug-gnu-emacs@gnu.org; Mon, 23 Oct 2023 19:04:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 23 Oct 2023 23:04:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 66655 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: fixed Original-Received: via spool by 66655-submit@debbugs.gnu.org id=B66655.16981022198120 (code B ref 66655); Mon, 23 Oct 2023 23:04:01 +0000 Original-Received: (at 66655) by debbugs.gnu.org; 23 Oct 2023 23:03:39 +0000 Original-Received: from localhost ([127.0.0.1]:52502 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qv3xS-00026t-A3 for submit@debbugs.gnu.org; Mon, 23 Oct 2023 19:03:38 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:3926) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qv3xQ-00026f-1M for 66655@debbugs.gnu.org; Mon, 23 Oct 2023 19:03:37 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 3130C100167; Mon, 23 Oct 2023 19:03:01 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1698102179; bh=YtLtdE3IKkOhtbhRYwjNTxQ6yLI6xjBz11dENFL1WZo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=PvUeJNzK8Ow+z0mTOfwQLcY8hgJYp+53sVVo6jVT5Ast2kfAJShYeJ3/pP5qqyXWo 14eNdGE7OIitr/BTrLTEa2B8ZX8ZEOyDXQjvmZ2uCQA2h64aIx4dt9i5y87Asc6y+G QakQ7a8SEjhzZr0IJeh4jINQO2ZxmQJpLpgyh3qr1A5oRMINEuS0JbWGBjzfpecab/ FrtQknKrYj0v33exHwfXxnr1m4RaoqKsyJP0SxdbPhyacR4yKzeAwnw3RNAI77FkC5 dLWPLOEUq2IShEnOQORBOdcajFjgJpkyNk8Hz9+F8Ti+Id5EIprZUgKsT9UQk7vBag a84NmPkoGFLWQ== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id C8D1C100043; Mon, 23 Oct 2023 19:02:59 -0400 (EDT) Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id BAC561201E3; Mon, 23 Oct 2023 19:02:59 -0400 (EDT) In-Reply-To: <83v8axmbsh.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 23 Oct 2023 21:30:38 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:273069 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii [2023-10-23 21:30:38] wrote: >> From: Stefan Monnier >> Cc: tomasralph2000@gmail.com, 66655@debbugs.gnu.org >> Date: Mon, 23 Oct 2023 12:38:35 -0400 >> >> > Stefan, I'd appreciate your review of the change, as this is a tricky >> > code, where we already had quite a few changes to avoid interpreting >> > an up-event as a drag event. >> >> The change looks OK. But yeah, it does feel like adding yet an >> other hack. The whole: >> >> /* Maybe the mouse has moved a lot, caused scrolling, and >> eventually ended up at the same screen position (but >> not buffer position) in which case it is a drag, not >> a click. */ >> /* FIXME: OTOH if the buffer position has changed >> because of a timer or process filter rather than >> because of mouse movement, it should be considered as >> a click. But mouse-drag-region completely ignores >> this case and it hasn't caused any real problem, so >> it's probably OK to ignore it as well. */ >> && (EQ (Fcar (Fcdr (start_pos)), >> Fcar (Fcdr (position))) /* Same buffer pos */ >> /* Redisplay hscrolled text between down- and >> up-events due to display-line-numbers-mode. */ >> || line_number_mode_hscroll (start_pos, position) >> || !EQ (Fcar (start_pos), >> Fcar (position))))) /* Different window */ >> >> is unsatisfactory. But it's not clear what is the right way to look at >> the problem. As the comment says, we generally want "down+scroll+up" to >> be treated as a drag, but not in the current case. I think the >> difference relies on what caused the scroll: if the scroll was the >> result of a deliberate act by the user (they moved the mouse after >> `down` to cause a scroll), then it's a drag and else it's not? > > Yes, that's the logic here. Technically, it happens because clicking > the mouse emits 2 events: down-mouse-1 followed by another one caused > by releasing the mouse button, and the first event could cause > redisplay (as happens in this case) because it generally moves point > to the location of the click. How 'bout something like the 100% untested patch below? Basically, generate a drag only if all 3 are satisfied: - the mouse has moved (and maybe come back). - the buffer position has changed. - the window has not changed (not sure why we have that, but IIUC that's a constraint we have on drag events). And to check the first we simply "mess up" the remembered start position whenever we emit a mouse-movement. Stefan --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=drag.patch diff --git a/src/keyboard.c b/src/keyboard.c index dc2f78a7c26..43d5b085857 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -5530,10 +5530,7 @@ #define ISO_FUNCTION_KEY_OFFSET 0xfe00 /* A cons recording the original frame-relative x and y coordinates of the down mouse event. */ static Lisp_Object frame_relative_event_pos; - -/* The line-number display width, in columns, at the time of most - recent down mouse event. */ -static int down_mouse_line_number_width; +static bool mouse_has_moved; /* Information about the most recent up-going button event: Which button, what location, and what time. */ @@ -5931,57 +5928,6 @@ coords_in_tab_bar_window (struct frame *f, int x, int y) #endif /* HAVE_WINDOW_SYSTEM */ -static void -save_line_number_display_width (struct input_event *event) -{ - struct window *w; - int pixel_width; - - if (WINDOWP (event->frame_or_window)) - w = XWINDOW (event->frame_or_window); - else if (FRAMEP (event->frame_or_window)) - w = XWINDOW (XFRAME (event->frame_or_window)->selected_window); - else - w = XWINDOW (selected_window); - line_number_display_width (w, &down_mouse_line_number_width, &pixel_width); -} - -/* Return non-zero if the change of position from START_POS to END_POS - is likely to be the effect of horizontal scrolling due to a change - in line-number width produced by redisplay between two mouse - events, like mouse-down followed by mouse-up, at those positions. - This is used to decide whether to converts mouse-down followed by - mouse-up event into a mouse-drag event. */ -static bool -line_number_mode_hscroll (Lisp_Object start_pos, Lisp_Object end_pos) -{ - if (!EQ (Fcar (start_pos), Fcar (end_pos)) /* different window */ - || list_length (start_pos) < 7 /* no COL/ROW info */ - || list_length (end_pos) < 7) - return false; - - Lisp_Object start_col_row = Fnth (make_fixnum (6), start_pos); - Lisp_Object end_col_row = Fnth (make_fixnum (6), end_pos); - Lisp_Object window = Fcar (end_pos); - int col_width, pixel_width; - Lisp_Object start_col, end_col; - struct window *w; - if (!WINDOW_VALID_P (window)) - { - if (WINDOW_LIVE_P (window)) - window = XFRAME (window)->selected_window; - else - window = selected_window; - } - w = XWINDOW (window); - line_number_display_width (w, &col_width, &pixel_width); - start_col = Fcar (start_col_row); - end_col = Fcar (end_col_row); - return EQ (start_col, end_col) - && down_mouse_line_number_width >= 0 - && col_width != down_mouse_line_number_width; -} - /* Given a struct input_event, build the lisp event which represents it. If EVENT is 0, build a mouse movement event from the mouse movement buffer, which should have a movement event in it. @@ -6383,9 +6329,8 @@ make_lispy_event (struct input_event *event) button_down_time = event->timestamp; *start_pos_ptr = Fcopy_alist (position); frame_relative_event_pos = Fcons (event->x, event->y); + mouse_has_moved = false; ignore_mouse_drag_p = false; - /* Squirrel away the line-number width, if any. */ - save_line_number_display_width (event); } /* Now we're releasing a button - check the coordinates to @@ -6415,34 +6360,18 @@ make_lispy_event (struct input_event *event) - XFIXNUM (XCDR (frame_relative_event_pos)); if (! (0 < double_click_fuzz + && !mouse_has_moved && - double_click_fuzz < xdiff && xdiff < double_click_fuzz && - double_click_fuzz < ydiff - && ydiff < double_click_fuzz - /* Maybe the mouse has moved a lot, caused scrolling, and - eventually ended up at the same screen position (but - not buffer position) in which case it is a drag, not - a click. */ - /* FIXME: OTOH if the buffer position has changed - because of a timer or process filter rather than - because of mouse movement, it should be considered as - a click. But mouse-drag-region completely ignores - this case and it hasn't caused any real problem, so - it's probably OK to ignore it as well. */ - && (EQ (Fcar (Fcdr (start_pos)), - Fcar (Fcdr (position))) /* Same buffer pos */ - /* Redisplay hscrolled text between down- and - up-events due to display-line-numbers-mode. */ - || line_number_mode_hscroll (start_pos, position) - || !EQ (Fcar (start_pos), - Fcar (position))))) /* Different window */ - + && ydiff < double_click_fuzz) + && (!EQ (Fcar (Fcdr (start_pos)), + Fcar (Fcdr (position)))) /* Different buffer pos */ + && EQ (Fcar (start_pos), Fcar (position))) /* Same window */ { /* Mouse has moved enough. */ button_down_time = 0; click_or_drag_modifier = drag_modifier; - /* Reset the value for future clicks. */ - down_mouse_line_number_width = -1; } else if (((!EQ (Fcar (start_pos), Fcar (position))) || (!EQ (Fcar (Fcdr (start_pos)), @@ -7084,6 +7013,7 @@ make_lispy_movement (struct frame *frame, Lisp_Object bar_window, enum scroll_ba { Lisp_Object position; position = make_lispy_position (frame, x, y, t); + mouse_has_moved = true; return list2 (Qmouse_movement, position); } } --=-=-=--