bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Eyal Soha]

[-- Attachment #1: Type: text/plain, Size: 6644 bytes --]

Setting the variable mouse-1-click-follows-link to 'double was broken by
the commit 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9 .

To test this, customize the mouse-1-click-follows-link variable to
`double.  Then try to double click on a link, such at the word "back"
that appears in many of the info help pages.  Double click will not
work.  Building on the version directly before that, double click does
work.



In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5,
cairo version 1.16.0)
 of 2021-03-03 built on mx
Repository revision: 43b40bc880f66cb3f48318ba3a480a76b149b815
Repository branch: feature/native-comp
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Debian GNU/Linux 10 (buster)

Configured using:
 'configure --with-native-compilation --prefix=/home/esoha/.local
 --with-x-toolkit=gtk3'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: C/*l

Minor modes in effect:
  gpm-mouse-mode: t
  hideshowvis-minor-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  xterm-mouse-mode: t
  global-color-identifiers-mode: t
  color-identifiers-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  company-quickhelp-mode: t
  company-quickhelp-local-mode: t
  global-company-mode: t
  company-mode: t
  recentf-mode: t
  savehist-mode: t
  save-place-mode: t
  show-paren-mode: t
  beacon-mode: t
  spaceline-info-mode: t
  spaceline-helm-mode: t
  counsel-mode: t
  ivy-mode: t
  delete-selection-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  global-auto-revert-mode: t
  shell-dirtrack-mode: t
  which-key-mode: t
  global-whitespace-mode: t
  ws-butler-mode: t
  goto-address-mode: t
  global-goto-address-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t
  hs-minor-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug sendmail mc-edit-lines
mc-hide-unmatched-lines-mode mc-mark-more mc-cycle-cursors
multiple-cursors-core t-mouse strokes msb avoid artist picture reporter
rect cus-edit pp cl-print eieio-opt speedbar ezimage dframe shortdoc
help-fns radix-tree ffap misearch multi-isearch vc-git cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
hideshow hideshowvis init diff-hl log-view vc-dir ewoc vc vc-dispatcher
scons-mode xt-mouse llvm-mode tablegen-mode
sanityinc-tomorrow-night-theme color-theme-sanityinc-tomorrow image-file
image-converter cus-start cus-load color-identifiers-mode python
tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat parse-time iso8601 ls-lisp flycheck find-func ace-jump-mode
cl company-quickhelp pos-tip company-oddmuse company-keywords
company-etags etags fileloop generator company-gtags
company-dabbrev-code company-dabbrev company-files company-clang
company-capf company-cmake company-semantic company-template
company-bbdb company rainbow-delimiters recentf tree-widget wid-edit
savehist saveplace paren beacon spaceline-config spaceline-segments s
spaceline powerline powerline-separators powerline-themes counsel xdg
xref project compile swiper ivy delsel ivy-faces ivy-overlay colir color
region-bindings-mode edmacro kmacro magit-submodule magit-obsolete
magit-popup magit-blame magit-stash magit-reflog magit-bisect magit-push
magit-pull magit-fetch magit-clone magit-remote magit-commit
magit-sequence magit-notes magit-worktree magit-tag magit-merge
magit-branch magit-reset magit-files magit-refs magit-status magit
magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff
smerge-mode diff diff-mode git-commit log-edit message rmc puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log magit-core
magit-autorevert autorevert filenotify magit-margin magit-transient
magit-process with-editor shell pcomplete comint ring server ansi-color
magit-mode transient comp comp-cstr warnings rx format-spec magit-git
magit-section magit-utils crm which-key advice disp-table whitespace
ws-butler goto-addr thingatpt diminish time-date auto-package-update
dash cl-extra help-mode use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core finder-inf info package easymenu browse-url
url url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face pcase macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process nativecomp
emacs)

Memory information:
((conses 16 682782 115976)
 (symbols 48 36240 2)
 (strings 32 156281 6872)
 (string-bytes 1 5629612)
 (vectors 16 69564)
 (vector-slots 8 1952275 168183)
 (floats 8 365 557)
 (intervals 56 14064 6666)
 (buffers 992 22))

[-- Attachment #2: Type: text/html, Size: 7244 bytes --]

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Eyal Soha]

On Wed, Mar 10, 2021 at 2:02 PM Eyal Soha <eyalsoha@gmail.com> wrote:
>
>
> Setting the variable mouse-1-click-follows-link to 'double was broken by
> the commit 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9 .
>
> To test this, customize the mouse-1-click-follows-link variable to
> `double.  Then try to double click on a link, such at the word "back"
> that appears in many of the info help pages.  Double click will not
> work.  Building on the version directly before that, double click does
> work.
>
>
>
> In GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
>  of 2021-03-03 built on mx
> Repository revision: 43b40bc880f66cb3f48318ba3a480a76b149b815
> Repository branch: feature/native-comp
> Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
> System Description: Debian GNU/Linux 10 (buster)
>
> Configured using:
>  'configure --with-native-compilation --prefix=/home/esoha/.local
>  --with-x-toolkit=gtk3'
>
> Configured features:
> CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
> LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
> SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB
>
> Important settings:
>   value of $LC_ALL: en_US.UTF-8
>   value of $LANG: en_US.UTF-8
>   locale-coding-system: utf-8-unix
>
> Major mode: C/*l
>
> Minor modes in effect:
>   gpm-mouse-mode: t
>   hideshowvis-minor-mode: t
>   global-diff-hl-mode: t
>   diff-hl-mode: t
>   xterm-mouse-mode: t
>   global-color-identifiers-mode: t
>   color-identifiers-mode: t
>   global-flycheck-mode: t
>   flycheck-mode: t
>   company-quickhelp-mode: t
>   company-quickhelp-local-mode: t
>   global-company-mode: t
>   company-mode: t
>   recentf-mode: t
>   savehist-mode: t
>   save-place-mode: t
>   show-paren-mode: t
>   beacon-mode: t
>   spaceline-info-mode: t
>   spaceline-helm-mode: t
>   counsel-mode: t
>   ivy-mode: t
>   delete-selection-mode: t
>   global-git-commit-mode: t
>   magit-auto-revert-mode: t
>   global-auto-revert-mode: t
>   shell-dirtrack-mode: t
>   which-key-mode: t
>   global-whitespace-mode: t
>   ws-butler-mode: t
>   goto-address-mode: t
>   global-goto-address-mode: t
>   override-global-mode: t
>   tooltip-mode: t
>   global-eldoc-mode: t
>   electric-indent-mode: t
>   mouse-wheel-mode: t
>   file-name-shadow-mode: t
>   global-font-lock-mode: t
>   font-lock-mode: t
>   blink-cursor-mode: t
>   auto-composition-mode: t
>   auto-encryption-mode: t
>   auto-compression-mode: t
>   column-number-mode: t
>   line-number-mode: t
>   transient-mark-mode: t
>   abbrev-mode: t
>   hs-minor-mode: t
>
> Load-path shadows:
> None found.
>
> Features:
> (shadow sort mail-extr emacsbug sendmail mc-edit-lines
> mc-hide-unmatched-lines-mode mc-mark-more mc-cycle-cursors
> multiple-cursors-core t-mouse strokes msb avoid artist picture reporter
> rect cus-edit pp cl-print eieio-opt speedbar ezimage dframe shortdoc
> help-fns radix-tree ffap misearch multi-isearch vc-git cc-mode cc-fonts
> cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
> hideshow hideshowvis init diff-hl log-view vc-dir ewoc vc vc-dispatcher
> scons-mode xt-mouse llvm-mode tablegen-mode
> sanityinc-tomorrow-night-theme color-theme-sanityinc-tomorrow image-file
> image-converter cus-start cus-load color-identifiers-mode python
> tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
> tramp-compat parse-time iso8601 ls-lisp flycheck find-func ace-jump-mode
> cl company-quickhelp pos-tip company-oddmuse company-keywords
> company-etags etags fileloop generator company-gtags
> company-dabbrev-code company-dabbrev company-files company-clang
> company-capf company-cmake company-semantic company-template
> company-bbdb company rainbow-delimiters recentf tree-widget wid-edit
> savehist saveplace paren beacon spaceline-config spaceline-segments s
> spaceline powerline powerline-separators powerline-themes counsel xdg
> xref project compile swiper ivy delsel ivy-faces ivy-overlay colir color
> region-bindings-mode edmacro kmacro magit-submodule magit-obsolete
> magit-popup magit-blame magit-stash magit-reflog magit-bisect magit-push
> magit-pull magit-fetch magit-clone magit-remote magit-commit
> magit-sequence magit-notes magit-worktree magit-tag magit-merge
> magit-branch magit-reset magit-files magit-refs magit-status magit
> magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff
> smerge-mode diff diff-mode git-commit log-edit message rmc puny dired
> dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config gnus-util
> rmail rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
> mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
> mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log magit-core
> magit-autorevert autorevert filenotify magit-margin magit-transient
> magit-process with-editor shell pcomplete comint ring server ansi-color
> magit-mode transient comp comp-cstr warnings rx format-spec magit-git
> magit-section magit-utils crm which-key advice disp-table whitespace
> ws-butler goto-addr thingatpt diminish time-date auto-package-update
> dash cl-extra help-mode use-package use-package-ensure
> use-package-delight use-package-diminish use-package-bind-key bind-key
> easy-mmode use-package-core finder-inf info package easymenu browse-url
> url url-proxy url-privacy url-expand url-methods url-history url-cookie
> url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
> eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
> url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
> iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
> lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
> dnd fontset image regexp-opt fringe tabulated-list replace newcomment
> text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
> rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
> syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
> cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
> korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
> european ethiopic indian cyrillic chinese composite charscript charprop
> case-table epa-hook jka-cmpr-hook help simple abbrev obarray
> cl-preloaded nadvice button loaddefs faces cus-face pcase macroexp files
> window text-properties overlay sha1 md5 base64 format env code-pages
> mule custom widget hashtable-print-readable backquote threads dbusbind
> inotify dynamic-setting system-font-setting font-render-setting cairo
> move-toolbar gtk x-toolkit x multi-tty make-network-process nativecomp
> emacs)
>
> Memory information:
> ((conses 16 682782 115976)
>  (symbols 48 36240 2)
>  (strings 32 156281 6872)
>  (string-bytes 1 5629612)
>  (vectors 16 69564)
>  (vector-slots 8 1952275 168183)
>  (floats 8 365 557)
>  (intervals 56 14064 6666)
>  (buffers 992 22))

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Stefan Monnier]

> Setting the variable mouse-1-click-follows-link to 'double was broken by
> the commit 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9 .

Indeed!  And since this code can be sometimes tricky to debug and tune,
your report got me scared that I was going to have to rethink the way it
works, but ... it was a silly mistake: I called `event-convert-list`
with a list whose last element was not always a "basic" event,
so we ended up comparing `down-double-mouse-1` and `double-down-mouse-1`
and deciding these are different events.

The patch below should fix it.
I think this patch should go to `emacs-27`.  Eli any objection?


        Stefan


diff --git a/lisp/mouse.el b/lisp/mouse.el
index 8732fb8086..f4979e37b0 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -119,7 +119,9 @@ mouse--click-1-maybe-follows-link
 			      (time-since (cdr mouse--last-down))
                               (/ (abs mouse-1-click-follows-link) 1000.0))))))
        (eq (car mouse--last-down)
-           (event-convert-list (list 'down (car-safe last-input-event))))
+           (event-convert-list
+            `(down ,@(event-modifiers last-input-event)
+                   ,(event-basic-type last-input-event))))
        (let* ((action (mouse-on-link-p (event-start last-input-event))))
          (when (and action
                     (or mouse-1-click-in-non-selected-windows

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 11 Mar 2021 09:55:24 -0500
> Cc: 47060@debbugs.gnu.org
> 
> > Setting the variable mouse-1-click-follows-link to 'double was broken by
> > the commit 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9 .
> 
> Indeed!  And since this code can be sometimes tricky to debug and tune,
> your report got me scared that I was going to have to rethink the way it
> works, but ... it was a silly mistake: I called `event-convert-list`
> with a list whose last element was not always a "basic" event,
> so we ended up comparing `down-double-mouse-1` and `double-down-mouse-1`
> and deciding these are different events.
> 
> The patch below should fix it.
> I think this patch should go to `emacs-27`.  Eli any objection?

No, please go ahead, and thanks.

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Stefan Monnier]

>> The patch below should fix it.
>> I think this patch should go to `emacs-27`.  Eli any objection?
> No, please go ahead, and thanks.

Thanks, pushed,


        Stefan

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Eyal Soha]

I tested it using the same procedure as before on feature/native-comp
branch and it works.  Thanks!

Eyal


On Thu, Mar 11, 2021 at 10:14 AM Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> >> The patch below should fix it.
> >> I think this patch should go to `emacs-27`.  Eli any objection?
> > No, please go ahead, and thanks.
>
> Thanks, pushed,
>
>
>         Stefan
>

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Jimmy Yuen Ho Wong]

This patch now results in this error whenever I left click on the NS 
port. The mouse is now completely broken.

Debugger entered--Lisp error: (error "Two bases given in one event")
   event-convert-list((down click mouse-1))
   mouse--click-1-maybe-follows-link(nil)

Jimmy

On 12/03/2021 4:10 AM, Eyal Soha wrote:
> I tested it using the same procedure as before on feature/native-comp
> branch and it works.  Thanks!
>
> Eyal
>
>
> On Thu, Mar 11, 2021 at 10:14 AM Stefan Monnier
> <monnier@iro.umontreal.ca> wrote:
>>>> The patch below should fix it.
>>>> I think this patch should go to `emacs-27`.  Eli any objection?
>>> No, please go ahead, and thanks.
>> Thanks, pushed,
>>
>>
>>          Stefan
>>
>
>

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Lars Ingebrigtsen]

Jimmy Yuen Ho Wong <wyuenho@gmail.com> writes:

> This patch now results in this error whenever I left click on the NS
> port. The mouse is now completely broken.
>
> Debugger entered--Lisp error: (error "Two bases given in one event")
>   event-convert-list((down click mouse-1))
>   mouse--click-1-maybe-follows-link(nil)

You dropped Stefan M from the CCs, so he might not see your message.
I've now added him back.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Stefan Monnier]

>> This patch now results in this error whenever I left click on the NS
>> port. The mouse is now completely broken.
>>
>> Debugger entered--Lisp error: (error "Two bases given in one event")
>>   event-convert-list((down click mouse-1))
>>   mouse--click-1-maybe-follows-link(nil)
>
> You dropped Stefan M from the CCs, so he might not see your message.
> I've now added him back.

Oh, my!  Indeed, I forgot that I tested that with my local Emacs which
has included the patch below for some years.

Hmm... Eli, what do you say, should I revert the patch on `emacs-27`
(seeing how it's obviously not as safe as I thought), or should I add
the patch below to `emacs-27`?

The problem is that currently `event-convert-list` disagrees with
`event-modifiers` about which set of symbols can be used as modifiers:
`event-modifiers` will sometimes include `click` as a modifier, whereas
`event-convert-list` doesn't accept it.


        Stefan


diff --git a/src/keyboard.c b/src/keyboard.c
index e3fc6adf81..512fa279b3 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6679,6 +6679,7 @@ #define MULTI_LETTER_MOD(BIT, NAME, LEN)		\
     case 'c':
       MULTI_LETTER_MOD (ctrl_modifier, "ctrl", 4);
       MULTI_LETTER_MOD (ctrl_modifier, "control", 7);
+      MULTI_LETTER_MOD (click_modifier, "click", 5);
       break;
 
     case 'H':

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Fri, 12 Mar 2021 12:38:42 -0500
> Cc: 47060@debbugs.gnu.org, Jimmy Yuen Ho Wong <wyuenho@gmail.com>
> 
> Oh, my!  Indeed, I forgot that I tested that with my local Emacs which
> has included the patch below for some years.
> 
> Hmm... Eli, what do you say, should I revert the patch on `emacs-27`
> (seeing how it's obviously not as safe as I thought), or should I add
> the patch below to `emacs-27`?
> 
> The problem is that currently `event-convert-list` disagrees with
> `event-modifiers` about which set of symbols can be used as modifiers:
> `event-modifiers` will sometimes include `click` as a modifier, whereas
> `event-convert-list` doesn't accept it.

Too bad.  I think we must revert the change you made.

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Stefan Monnier]

> Too bad.  I think we must revert the change you made.

OK,


        Stefan

bug#47060: mouse-1-click-follows-link double broken by 3d5e31eceb9dc1fb62b2b27bcab549df3bd04ce9

[Glenn Morris]

Jimmy Yuen Ho Wong wrote:

> This patch now results in this error whenever I left click on the NS
> port. The mouse is now completely broken.
>
> Debugger entered--Lisp error: (error "Two bases given in one event")
>   event-convert-list((down click mouse-1))
>   mouse--click-1-maybe-follows-link(nil)

This was also flagged by the test suite.
I wish people would at least run foo-tests.el before pushing foo.el
changes (especially to the release branch).