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: Wed, 25 Oct 2023 17:22:41 -0400 Message-ID: References: <1d9187b71e7288eaf08ac9a2f0559bdf@gmail.com> <83v8axmbsh.fsf@gnu.org> <83y1fskyjj.fsf@gnu.org> <83il6wktxt.fsf@gnu.org> <83h6mgkst3.fsf@gnu.org> <83edhkkrzd.fsf@gnu.org> <83bkcokoxu.fsf@gnu.org> <83pm12kj44.fsf@gnu.org> <83msw6it17.fsf@gnu.org> <83h6meiraw.fsf@gnu.org> <83fs1yimho.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="33522"; 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 Wed Oct 25 23:26:17 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 1qvlOI-0008MO-S5 for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 25 Oct 2023 23:26:15 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qvlNe-0007Rw-UQ; Wed, 25 Oct 2023 17:25:34 -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 1qvlNc-0007NA-BY for bug-gnu-emacs@gnu.org; Wed, 25 Oct 2023 17:25:32 -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 1qvlNb-0006pX-Rc for bug-gnu-emacs@gnu.org; Wed, 25 Oct 2023 17:25:32 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qvlO5-0004GG-TQ for bug-gnu-emacs@gnu.org; Wed, 25 Oct 2023 17:26: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: Wed, 25 Oct 2023 21:26: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.169826913016333 (code B ref 66655); Wed, 25 Oct 2023 21:26:01 +0000 Original-Received: (at 66655) by debbugs.gnu.org; 25 Oct 2023 21:25:30 +0000 Original-Received: from localhost ([127.0.0.1]:60108 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qvlNa-0004FK-3C for submit@debbugs.gnu.org; Wed, 25 Oct 2023 17:25:30 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:60556) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qvlNW-0004F5-Cg for 66655@debbugs.gnu.org; Wed, 25 Oct 2023 17:25:28 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 9DB8010013E; Wed, 25 Oct 2023 17:24:50 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1698269089; bh=++ilHWoMEwdWQeWhuAfK9cMvk+lt8fFdbNEbf8O/Om0=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=nnWlbLtJu5DXMAmQ0vxfyELcEvEWt1KFNChRDxEXDxthoJwQdfLlEFqTLIlmLng01 T3zvWA6c7vrp+SizO5DVjoCSbjumlLEQGlAeZDsymfDR3zhpavciOyc8mcDJxTse0Y XUa12H7Sic9E2e2WrB6iLW59P5/0TPlMKw4AV2uR6GqGH25PgzJZ+5962bk/QHLVD+ 3CdDN+i60hhGkGxFsfd9FppHJWZodgudNQJViuTU0t1VEDO/YXSqrExDBCizUyoOcM 8sDHZ5zfFUny9OCX8HzxoPdMiWDBa3Bj79kkLoVIrBlY0viiP7MGeNuu/MoYBjs/Jx 2TIT+xTPh80fw== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 9277010006B; Wed, 25 Oct 2023 17:24:49 -0400 (EDT) Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 7E66F1203C5; Wed, 25 Oct 2023 17:24:49 -0400 (EDT) In-Reply-To: <83fs1yimho.fsf@gnu.org> (Eli Zaretskii's message of "Wed, 25 Oct 2023 21:29:55 +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:273215 Archived-At: --=-=-= Content-Type: text/plain >> > But then we are back at the problem which the buffer-position check >> > tries to address: >> > >> > /* 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. */ >> > >> > IOW, just testing the screen coordinates is not enough. >> >> In my "in short is approximately" I used `mouse_has_moved` but that >> was an oversimplification: in the new code `mouse_has_moved` doesn't >> revert to "false" when the mouse returns to the original position, >> contrary to what happen in the current code. > > How exactly does that happen? Because as soon as `note_mouse_highlight` receives information that the mouse is outside of the fuzz, the var is set to `false` [ And it's only set to true when we get the mouse-down event ]. >> The comment above talks about buffer positions (i.e. the Fcar+Fcdr >> part of the positions), whereas this `EQ` tests the windows, and the >> only relevant comment I see is >> >> /* Different window */ >> >> which reminds the reader that it's comparing windows but doesn't say why. >> Did I miss something? > > Yes: we can be at the same buffer position, but a different window. Right, but what's the problem with that? IIUC the current code can generate a drag between windows if the mouse has moved to a new screen position, but we never generate a drag between windows if the mouse ends at the same screen position as it started. I see now that this was done (in commit 6e2d3bce087d30a535b1f01715d7820576ffe390) to handle the case where a mouse click causes some window-shuffle, so the up event ends up pointing into another window. IOW, a similar problem to the display-line-number one you just fixed. I think my code is immune to this problem since with it we only ever generate a drag event if the mouse actually moved. Which leads to a further simplification of the code, see patch below. Stefan --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=drag.patch diff --git a/src/keyboard.c b/src/keyboard.c index dc2f78a7c26..f9777eee120 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -5530,10 +5530,9 @@ #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; +/* False iff the mouse has stayed within `double_click_fuzz` of + `frame_relative_event_pos`. */ +static bool mouse_has_moved; /* Information about the most recent up-going button event: Which button, what location, and what time. */ @@ -5931,55 +5930,19 @@ 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) +/* */ +void +check_mouse_click_fuzz (EMACS_INT x, EMACS_INT y) { - 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; + EMACS_INT xdiff = x - XFIXNUM (XCAR (frame_relative_event_pos)); + EMACS_INT ydiff = y - XFIXNUM (XCDR (frame_relative_event_pos)); - 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; + if (0 < double_click_fuzz + && - double_click_fuzz < xdiff + && xdiff < double_click_fuzz + && - double_click_fuzz < ydiff + && ydiff < double_click_fuzz) + mouse_has_moved = true; } /* Given a struct input_event, build the lisp event which represents @@ -6383,9 +6346,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 @@ -6407,78 +6369,16 @@ make_lispy_event (struct input_event *event) ignore_mouse_drag_p = false; else { - intmax_t xdiff = double_click_fuzz, ydiff = double_click_fuzz; - - xdiff = XFIXNUM (event->x) - - XFIXNUM (XCAR (frame_relative_event_pos)); - ydiff = XFIXNUM (event->y) - - XFIXNUM (XCDR (frame_relative_event_pos)); - - if (! (0 < double_click_fuzz - && - 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 */ - + /* Check if this position is within the fuzz. */ + check_mouse_click_fuzz (XFIXNUM (event->x), XFIXNUM (event->y)); + if (mouse_has_moved + && (!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)), - Fcar (Fcdr (position))))) - /* Was the down event in a window body? */ - && FIXNUMP (Fcar (Fcdr (start_pos))) - && WINDOW_LIVE_P (Fcar (start_pos)) - && !NILP (Ffboundp (Qwindow_edges))) - /* If the window (etc.) at the mouse position has - changed between the down event and the up event, - we assume there's been a redisplay between the - two events, and we pretend the mouse is still in - the old window to prevent a spurious drag event - being generated. */ - { - Lisp_Object edges - = call4 (Qwindow_edges, Fcar (start_pos), Qt, Qnil, Qt); - int new_x = XFIXNUM (Fcar (frame_relative_event_pos)); - int new_y = XFIXNUM (Fcdr (frame_relative_event_pos)); - - /* If the up-event is outside the down-event's - window, use coordinates that are within it. */ - if (new_x < XFIXNUM (Fcar (edges))) - new_x = XFIXNUM (Fcar (edges)); - else if (new_x >= XFIXNUM (Fcar (Fcdr (Fcdr (edges))))) - new_x = XFIXNUM (Fcar (Fcdr (Fcdr (edges)))) - 1; - if (new_y < XFIXNUM (Fcar (Fcdr (edges)))) - new_y = XFIXNUM (Fcar (Fcdr (edges))); - else if (new_y - >= XFIXNUM (Fcar (Fcdr (Fcdr (Fcdr (edges)))))) - new_y = XFIXNUM (Fcar (Fcdr (Fcdr (Fcdr (edges))))) - 1; - - position = make_lispy_position - (XFRAME (event->frame_or_window), - make_fixnum (new_x), make_fixnum (new_y), - event->timestamp); } } @@ -7084,6 +6984,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); } } --=-=-=--