bug#23288: 25.0.92; Clicking on links inserts primary X selection

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

Here 'emacs' is an Emacs binary using Gtk+ on GNU/Linux.

echo -n bar | xsel -i && \
  emacs -Q -eval \
  "(insert (propertize \"foo\" 'follow-link (lambda (pos) \"a\")))"

Then click on "foo" in the scratch buffer.  If 'emacs' is Emacs 24.3,
"a" will be inserted.  If 'emacs' is from the emacs-25 branch, "bar"
will be inserted.  I think this is a regression; follow-link should
adhere to the link action no matter what is in the X selection.



In GNU Emacs 25.0.92.2 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2016-04-14 built on localhost
Repository revision: 567ab529f313bc8fe68d25b2ca95f5987cce81a1
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured using:
 'configure --with-modules CFLAGS=-g'

Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-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
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame 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 charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote inotify dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 88168 9879)
 (symbols 48 19855 0)
 (miscs 40 325 173)
 (strings 32 14741 4231)
 (string-bytes 1 437388)
 (vectors 16 12106)
 (vector-slots 8 439789 6169)
 (floats 8 164 78)
 (intervals 56 208 0)
 (buffers 976 12)
 (heap 1024 24962 1011))

-- 
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle

Diese E-Mail ist vertraulich.  Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen
Sie die E-Mail und alle Anhänge.  Vielen Dank.

This e-mail is confidential.  If you are not the right addressee please do not
forward it, please inform the sender, and please erase this e-mail including
any attachments.  Thanks.

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Glenn Morris]

Sounds like http://debbugs.gnu.org/22434

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Nils Berg]

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

I've tracked down the bug to mouse--down-1-maybe-follows-link too, but
don't think read-event is the culprit here.

Rather, mouse--down-1-maybe-follows-link discards the result of
mouse-on-link-p, which contains the event a click is supposed to be
translated into. Instead, it just replaces the mouse event with mouse-2 if
mouse-on-link-p returns non-nil.

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 617 bytes --]

Nils Berg <nilsb@google.com> schrieb am Do., 14. Apr. 2016 um 19:18 Uhr:

> I've tracked down the bug to mouse--down-1-maybe-follows-link too, but
> don't think read-event is the culprit here.
>
> Rather, mouse--down-1-maybe-follows-link discards the result of
> mouse-on-link-p, which contains the event a click is supposed to be
> translated into. Instead, it just replaces the mouse event with mouse-2 if
> mouse-on-link-p returns non-nil.
>

I've attached a patch that should fix this. According to the documentation
of mouse-on-link-p no such translation should happen if the return value is
a string or vector.

[-- Attachment #1.2: Type: text/html, Size: 929 bytes --]

[-- Attachment #2: 0001-Fix-link-handling.patch --]
[-- Type: application/octet-stream, Size: 1410 bytes --]

From 584f288702d9d5c901685e54bfe1b147683d4aab Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 16 Apr 2016 15:33:30 +0200
Subject: [PATCH] Fix link handling.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Sometimes mouse-1 got translated to mouse-2 when it shouldn’t have been,
see Bug#23288.  Instead adhere to the documentation of ‘mouse-on-link-p’
and translate the click only if it’s neither nil nor a string nor a
vector.

* lisp/mouse.el (mouse--down-1-maybe-follows-link): Check for return
type of ‘mouse-on-link-p’.
---
 lisp/mouse.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/mouse.el b/lisp/mouse.el
index fa355ff..626576c 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -98,7 +98,8 @@ mouse--down-1-maybe-follows-link
              (eq (if (eq mouse-1-click-follows-link 'double)
                      'double-down-mouse-1 'down-mouse-1)
                  (car-safe last-input-event))
-             (mouse-on-link-p (event-start last-input-event))
+             (let ((action (mouse-on-link-p (event-start last-input-event))))
+               (not (or (null action) (stringp action) (vectorp action))))
              (or mouse-1-click-in-non-selected-windows
                  (eq (selected-window)
                      (posn-window (event-start last-input-event)))))
-- 
2.7.4

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Nils Berg]

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

I applied the patch, and the paste-on-click behavior is gone.

However, if you try your original example again, you'll find that nothing
happens at all, when we're expecting an "a" to be inserted.
As the documentation of mouse-on-link-p says, a string or vector return
value indicates the event to translate the original mouse-1 click into. In
emacs24, that translation was done in mouse-drag-track:
(let (on-link (and mouse-1-click-follows-link
                       ;; Use start-point before the intangibility
                       ;; treatment, in case we click on a link inside
                       ;; intangible text.
  (mouse-on-link-p start-posn)))
  (if (or (vectorp on-link) (stringp on-link))
 (setq event (aref on-link 0))
(select-window original-window)
(setcar event 'mouse-2)
;; If this mouse click has never been done by the
;; user, it doesn't have the necessary property to be
;; interpreted correctly.
(put 'mouse-2 'event-kind 'mouse-click)))

(abridged from mouse.el:791/901 in Emacs 24.3.1)

I think mouse--down-1-maybe-follows-link should do something similar.

On Sat, Apr 16, 2016 at 3:36 PM, Philipp Stephani <p.stephani2@gmail.com>
wrote:

>
>
> Nils Berg <nilsb@google.com> schrieb am Do., 14. Apr. 2016 um 19:18 Uhr:
>
>> I've tracked down the bug to mouse--down-1-maybe-follows-link too, but
>> don't think read-event is the culprit here.
>>
>> Rather, mouse--down-1-maybe-follows-link discards the result of
>> mouse-on-link-p, which contains the event a click is supposed to be
>> translated into. Instead, it just replaces the mouse event with mouse-2 if
>> mouse-on-link-p returns non-nil.
>>
>
> I've attached a patch that should fix this. According to the documentation
> of mouse-on-link-p no such translation should happen if the return value is
> a string or vector.
>

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

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

Nils Berg <nilsb@google.com> schrieb am Mo., 18. Apr. 2016 um 10:50 Uhr:

> I applied the patch, and the paste-on-click behavior is gone.
>
> However, if you try your original example again, you'll find that nothing
> happens at all, when we're expecting an "a" to be inserted.
> As the documentation of mouse-on-link-p says, a string or vector return
> value indicates the event to translate the original mouse-1 click into. In
> emacs24, that translation was done in mouse-drag-track:
> (let (on-link (and mouse-1-click-follows-link
>                        ;; Use start-point before the intangibility
>                        ;; treatment, in case we click on a link inside
>                        ;; intangible text.
>   (mouse-on-link-p start-posn)))
>   (if (or (vectorp on-link) (stringp on-link))
>  (setq event (aref on-link 0))
> (select-window original-window)
> (setcar event 'mouse-2)
> ;; If this mouse click has never been done by the
> ;; user, it doesn't have the necessary property to be
> ;; interpreted correctly.
> (put 'mouse-2 'event-kind 'mouse-click)))
>
> (abridged from mouse.el:791/901 in Emacs 24.3.1)
>
> I think mouse--down-1-maybe-follows-link should do something similar.
>

Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also be
related.

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1485 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am Mi., 20. Apr. 2016 um
18:53 Uhr:

> Nils Berg <nilsb@google.com> schrieb am Mo., 18. Apr. 2016 um 10:50 Uhr:
>
>> I applied the patch, and the paste-on-click behavior is gone.
>>
>> However, if you try your original example again, you'll find that nothing
>> happens at all, when we're expecting an "a" to be inserted.
>> As the documentation of mouse-on-link-p says, a string or vector return
>> value indicates the event to translate the original mouse-1 click into. In
>> emacs24, that translation was done in mouse-drag-track:
>> (let (on-link (and mouse-1-click-follows-link
>>                        ;; Use start-point before the intangibility
>>                        ;; treatment, in case we click on a link inside
>>                        ;; intangible text.
>>   (mouse-on-link-p start-posn)))
>>   (if (or (vectorp on-link) (stringp on-link))
>>  (setq event (aref on-link 0))
>> (select-window original-window)
>> (setcar event 'mouse-2)
>> ;; If this mouse click has never been done by the
>> ;; user, it doesn't have the necessary property to be
>> ;; interpreted correctly.
>> (put 'mouse-2 'event-kind 'mouse-click)))
>>
>> (abridged from mouse.el:791/901 in Emacs 24.3.1)
>>
>> I think mouse--down-1-maybe-follows-link should do something similar.
>>
>
> Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also be
> related.
>
>
I've attached a new patch that should hopefully correct the behavior.

[-- Attachment #1.2: Type: text/html, Size: 2817 bytes --]

[-- Attachment #2: 0001-Fix-handling-of-mouse-on-link-p.patch --]
[-- Type: application/octet-stream, Size: 7334 bytes --]

From 6b4e9cc6e79ec3e723f1d13457ec8a890de06b8b Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Tue, 10 May 2016 23:23:26 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20handling=20of=20=E2=80=98mouse-on-link-p?=
 =?UTF-8?q?=E2=80=99.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If ‘mouse-on-link-p’ returns a string or vector, the first element
is to be used as new event.  Translation to ‘mouse-2’ should only
happen if the return value is not a string or vector.  See
docstring of ‘mouse-on-link-p’ and Bug#23288.

* lisp/mouse.el (mouse--down-1-maybe-follows-link): Process return
value of ‘mouse-on-link-p’ according to documentation.

* test/automated/mouse-tests.el (bug23288-use-return-value)
(bug23288-translate-to-mouse-2): Tests for Bug#23288.
---
 lisp/mouse.el                 | 66 ++++++++++++++++++++++++-------------------
 test/automated/mouse-tests.el | 48 +++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 29 deletions(-)
 create mode 100644 test/automated/mouse-tests.el

diff --git a/lisp/mouse.el b/lisp/mouse.el
index fa355ff..e010def 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -97,35 +97,43 @@ mouse--down-1-maybe-follows-link
   (when (and mouse-1-click-follows-link
              (eq (if (eq mouse-1-click-follows-link 'double)
                      'double-down-mouse-1 'down-mouse-1)
-                 (car-safe last-input-event))
-             (mouse-on-link-p (event-start last-input-event))
-             (or mouse-1-click-in-non-selected-windows
-                 (eq (selected-window)
-                     (posn-window (event-start last-input-event)))))
-    (let ((timedout
-           (sit-for (if (numberp mouse-1-click-follows-link)
-                     (/ (abs mouse-1-click-follows-link) 1000.0)
-                     0))))
-      (if (if (and (numberp mouse-1-click-follows-link)
-                   (>= mouse-1-click-follows-link 0))
-              timedout (not timedout))
-          nil
-
-        (let ((event (read-key))) ;Use read-key so it works for xterm-mouse-mode!
-          (if (eq (car-safe event) (if (eq mouse-1-click-follows-link 'double)
-                                       'double-mouse-1 'mouse-1))
-              ;; Turn the mouse-1 into a mouse-2 to follow links.
-              (let ((newup (if (eq mouse-1-click-follows-link 'double)
-                                'double-mouse-2 'mouse-2)))
-                ;; If mouse-2 has never been done by the user, it doesn't have
-                ;; the necessary property to be interpreted correctly.
-                (unless (get newup 'event-kind)
-                  (put newup 'event-kind (get (car event) 'event-kind)))
-                (push (cons newup (cdr event)) unread-command-events)
-                ;; Don't change the down event, only the up-event (bug#18212).
-                nil)
-            (push event unread-command-events)
-            nil))))))
+                 (car-safe 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
+                     (eq (selected-window)
+                         (posn-window (event-start last-input-event)))))
+        (let ((timedout
+               (sit-for (if (numberp mouse-1-click-follows-link)
+                            (/ (abs mouse-1-click-follows-link) 1000.0)
+                          0))))
+          (if (if (and (numberp mouse-1-click-follows-link)
+                       (>= mouse-1-click-follows-link 0))
+                  timedout (not timedout))
+              nil
+            ;; Use read-key so it works for xterm-mouse-mode!
+            (let ((event (read-key)))
+              (if (eq (car-safe event)
+                      (if (eq mouse-1-click-follows-link 'double)
+                          'double-mouse-1 'mouse-1))
+                  ;; Turn the mouse-1 into a mouse-2 to follow links,
+                  ;; but only if ‘mouse-on-link-p’ hasn’t returned a
+                  ;; string or vector (see its docstring).
+                  (if (or (stringp action) (vectorp action))
+                      (push (aref action 0) unread-command-events)
+                    (let ((newup (if (eq mouse-1-click-follows-link 'double)
+                                     'double-mouse-2 'mouse-2)))
+                      ;; If mouse-2 has never been done by the user, it
+                      ;; doesn't have the necessary property to be
+                      ;; interpreted correctly.
+                      (unless (get newup 'event-kind)
+                        (put newup 'event-kind (get (car event) 'event-kind)))
+                      (push (cons newup (cdr event)) unread-command-events))
+                    ;; Don't change the down event, only the up-event
+                    ;; (bug#18212).
+                    nil)
+                (push event unread-command-events)
+                nil))))))))
 
 (define-key key-translation-map [down-mouse-1]
   #'mouse--down-1-maybe-follows-link)
diff --git a/test/automated/mouse-tests.el b/test/automated/mouse-tests.el
new file mode 100644
index 0000000..a0cbbeb
--- /dev/null
+++ b/test/automated/mouse-tests.el
@@ -0,0 +1,48 @@
+;;; mouse-tests.el --- unit tests for mouse.el       -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016  Free Software Foundation, Inc.
+
+;; Author: Philipp Stephani <phst@google.com>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Unit tests for lisp/mouse.el.
+
+;;; Code:
+
+(ert-deftest bug23288-use-return-value ()
+  "If ‘mouse-on-link-p’ returns a string, its first character is
+used."
+  (cl-letf ((last-input-event '(down-mouse-1 nil 1))
+            (unread-command-events '((mouse-1 nil 1)))
+            (mouse-1-click-follows-link t)
+            (mouse-1-click-in-non-selected-windows t)
+            ((symbol-function 'mouse-on-link-p) (lambda (_pos) "abc")))
+    (mouse--down-1-maybe-follows-link)
+    (should (equal unread-command-events '(?a)))))
+
+(ert-deftest bug23288-translate-to-mouse-2 ()
+  "If ‘mouse-on-link-p’ doesn’t return a string or vector,
+translate ‘mouse-1’ events into ‘mouse-2’ events."
+  (cl-letf ((last-input-event '(down-mouse-1 nil 1))
+            (unread-command-events '((mouse-1 nil 1)))
+            (mouse-1-click-follows-link t)
+            (mouse-1-click-in-non-selected-windows t)
+            ((symbol-function 'mouse-on-link-p) (lambda (_pos) t)))
+    (mouse--down-1-maybe-follows-link)
+    (should (equal unread-command-events '((mouse-2 nil 1))))))
+
+;;; mouse-tests.el ends here
-- 
2.8.2

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Tue, 10 May 2016 21:25:47 +0000
> Cc: 23288@debbugs.gnu.org
> 
>  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also be related.
> 
> I've attached a new patch that should hopefully correct the behavior. 

Thanks.  If no one objects in a week, please push to the master
branch.

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Nils Berg]

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

Still no dice, sorry.  I just tried the patched version with the original
example:
echo -n bar | xsel -i && \
  emacs -Q -eval \
  "(insert (propertize \"foo\" 'follow-link (lambda (pos) \"a\")))"

Now when I click foo nothing happens in the buffer, but there's an error
message:
Function mouse--down-1-maybe-follows-link returns invalid key sequence

I can't think of a reason why though :(

M-x version: GNU Emacs 25.0.93.1 (x86_64-pc-linux-gnu, GTK+ Version 3.10.8)

On Wed, May 11, 2016 at 10:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 10 May 2016 21:25:47 +0000
> > Cc: 23288@debbugs.gnu.org
> >
> >  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also
> be related.
> >
> > I've attached a new patch that should hopefully correct the behavior.
>
> Thanks.  If no one objects in a week, please push to the master
> branch.
>

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1124 bytes --]

There was a small bug in the patch. I've attached a new version that
hopefully fixes that.

Nils Berg <nilsb@google.com> schrieb am Mi., 11. Mai 2016 um 10:32 Uhr:

> Still no dice, sorry.  I just tried the patched version with the original
> example:
> echo -n bar | xsel -i && \
>   emacs -Q -eval \
>   "(insert (propertize \"foo\" 'follow-link (lambda (pos) \"a\")))"
>
> Now when I click foo nothing happens in the buffer, but there's an error
> message:
> Function mouse--down-1-maybe-follows-link returns invalid key sequence
>
> I can't think of a reason why though :(
>
> M-x version: GNU Emacs 25.0.93.1 (x86_64-pc-linux-gnu, GTK+ Version 3.10.8)
>
> On Wed, May 11, 2016 at 10:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Philipp Stephani <p.stephani2@gmail.com>
>> > Date: Tue, 10 May 2016 21:25:47 +0000
>> > Cc: 23288@debbugs.gnu.org
>> >
>> >  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might
>> also be related.
>> >
>> > I've attached a new patch that should hopefully correct the behavior.
>>
>> Thanks.  If no one objects in a week, please push to the master
>> branch.
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 2124 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-handling-of-mouse-on-link-p.patch --]
[-- Type: text/x-patch; charset=US-ASCII;  name="0001-Fix-handling-of-mouse-on-link-p.patch", Size: 7430 bytes --]

From bda8852c32a2c5ff79bbee5fbe9d61d42274054b Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Tue, 10 May 2016 23:23:26 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20handling=20of=20=E2=80=98mouse-on-link-p?=
 =?UTF-8?q?=E2=80=99.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If ‘mouse-on-link-p’ returns a string or vector, the first element
is to be used as new event.  Translation to ‘mouse-2’ should only
happen if the return value is not a string or vector.  See
docstring of ‘mouse-on-link-p’ and Bug#23288.

* lisp/mouse.el (mouse--down-1-maybe-follows-link): Process return
value of ‘mouse-on-link-p’ according to documentation.

* test/automated/mouse-tests.el (bug23288-use-return-value)
(bug23288-translate-to-mouse-2): Tests for Bug#23288.
---
 lisp/mouse.el                 | 67 ++++++++++++++++++++++++-------------------
 test/automated/mouse-tests.el | 48 +++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 29 deletions(-)
 create mode 100644 test/automated/mouse-tests.el

diff --git a/lisp/mouse.el b/lisp/mouse.el
index fa355ff..ba6b04a 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -97,35 +97,44 @@ mouse--down-1-maybe-follows-link
   (when (and mouse-1-click-follows-link
              (eq (if (eq mouse-1-click-follows-link 'double)
                      'double-down-mouse-1 'down-mouse-1)
-                 (car-safe last-input-event))
-             (mouse-on-link-p (event-start last-input-event))
-             (or mouse-1-click-in-non-selected-windows
-                 (eq (selected-window)
-                     (posn-window (event-start last-input-event)))))
-    (let ((timedout
-           (sit-for (if (numberp mouse-1-click-follows-link)
-                     (/ (abs mouse-1-click-follows-link) 1000.0)
-                     0))))
-      (if (if (and (numberp mouse-1-click-follows-link)
-                   (>= mouse-1-click-follows-link 0))
-              timedout (not timedout))
-          nil
-
-        (let ((event (read-key))) ;Use read-key so it works for xterm-mouse-mode!
-          (if (eq (car-safe event) (if (eq mouse-1-click-follows-link 'double)
-                                       'double-mouse-1 'mouse-1))
-              ;; Turn the mouse-1 into a mouse-2 to follow links.
-              (let ((newup (if (eq mouse-1-click-follows-link 'double)
-                                'double-mouse-2 'mouse-2)))
-                ;; If mouse-2 has never been done by the user, it doesn't have
-                ;; the necessary property to be interpreted correctly.
-                (unless (get newup 'event-kind)
-                  (put newup 'event-kind (get (car event) 'event-kind)))
-                (push (cons newup (cdr event)) unread-command-events)
-                ;; Don't change the down event, only the up-event (bug#18212).
-                nil)
-            (push event unread-command-events)
-            nil))))))
+                 (car-safe 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
+                     (eq (selected-window)
+                         (posn-window (event-start last-input-event)))))
+        (let ((timedout
+               (sit-for (if (numberp mouse-1-click-follows-link)
+                            (/ (abs mouse-1-click-follows-link) 1000.0)
+                          0))))
+          (if (if (and (numberp mouse-1-click-follows-link)
+                       (>= mouse-1-click-follows-link 0))
+                  timedout (not timedout))
+              nil
+            ;; Use read-key so it works for xterm-mouse-mode!
+            (let ((event (read-key)))
+              (if (eq (car-safe event)
+                      (if (eq mouse-1-click-follows-link 'double)
+                          'double-mouse-1 'mouse-1))
+                  (progn
+                    ;; Turn the mouse-1 into a mouse-2 to follow links,
+                    ;; but only if ‘mouse-on-link-p’ hasn’t returned a
+                    ;; string or vector (see its docstring).
+                    (if (or (stringp action) (vectorp action))
+                        (push (aref action 0) unread-command-events)
+                      (let ((newup (if (eq mouse-1-click-follows-link 'double)
+                                       'double-mouse-2 'mouse-2)))
+                        ;; If mouse-2 has never been done by the user, it
+                        ;; doesn't have the necessary property to be
+                        ;; interpreted correctly.
+                        (unless (get newup 'event-kind)
+                          (put newup 'event-kind (get (car event) 'event-kind)))
+                        (push (cons newup (cdr event)) unread-command-events)))
+                    ;; Don't change the down event, only the up-event
+                    ;; (bug#18212).
+                    nil)
+                (push event unread-command-events)
+                nil))))))))
 
 (define-key key-translation-map [down-mouse-1]
   #'mouse--down-1-maybe-follows-link)
diff --git a/test/automated/mouse-tests.el b/test/automated/mouse-tests.el
new file mode 100644
index 0000000..21abf38
--- /dev/null
+++ b/test/automated/mouse-tests.el
@@ -0,0 +1,48 @@
+;;; mouse-tests.el --- unit tests for mouse.el       -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2016  Free Software Foundation, Inc.
+
+;; Author: Philipp Stephani <phst@google.com>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Unit tests for lisp/mouse.el.
+
+;;; Code:
+
+(ert-deftest bug23288-use-return-value ()
+  "If ‘mouse-on-link-p’ returns a string, its first character is
+used."
+  (cl-letf ((last-input-event '(down-mouse-1 nil 1))
+            (unread-command-events '((mouse-1 nil 1)))
+            (mouse-1-click-follows-link t)
+            (mouse-1-click-in-non-selected-windows t)
+            ((symbol-function 'mouse-on-link-p) (lambda (_pos) "abc")))
+    (should-not (mouse--down-1-maybe-follows-link))
+    (should (equal unread-command-events '(?a)))))
+
+(ert-deftest bug23288-translate-to-mouse-2 ()
+  "If ‘mouse-on-link-p’ doesn’t return a string or vector,
+translate ‘mouse-1’ events into ‘mouse-2’ events."
+  (cl-letf ((last-input-event '(down-mouse-1 nil 1))
+            (unread-command-events '((mouse-1 nil 1)))
+            (mouse-1-click-follows-link t)
+            (mouse-1-click-in-non-selected-windows t)
+            ((symbol-function 'mouse-on-link-p) (lambda (_pos) t)))
+    (should-not (mouse--down-1-maybe-follows-link))
+    (should (equal unread-command-events '((mouse-2 nil 1))))))
+
+;;; mouse-tests.el ends here
-- 
2.8.0.rc3.226.g39d4020

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Nils Berg]

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

Perfect, now it works :) Thanks for fixing this!

On Wed, May 11, 2016 at 3:01 PM, Philipp Stephani <p.stephani2@gmail.com>
wrote:

> There was a small bug in the patch. I've attached a new version that
> hopefully fixes that.
>
>
> Nils Berg <nilsb@google.com> schrieb am Mi., 11. Mai 2016 um 10:32 Uhr:
>
>> Still no dice, sorry.  I just tried the patched version with the original
>> example:
>> echo -n bar | xsel -i && \
>>   emacs -Q -eval \
>>   "(insert (propertize \"foo\" 'follow-link (lambda (pos) \"a\")))"
>>
>> Now when I click foo nothing happens in the buffer, but there's an error
>> message:
>> Function mouse--down-1-maybe-follows-link returns invalid key sequence
>>
>> I can't think of a reason why though :(
>>
>> M-x version: GNU Emacs 25.0.93.1 (x86_64-pc-linux-gnu, GTK+ Version
>> 3.10.8)
>>
>> On Wed, May 11, 2016 at 10:14 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>> > From: Philipp Stephani <p.stephani2@gmail.com>
>>> > Date: Tue, 10 May 2016 21:25:47 +0000
>>> > Cc: 23288@debbugs.gnu.org
>>> >
>>> >  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might
>>> also be related.
>>> >
>>> > I've attached a new patch that should hopefully correct the behavior.
>>>
>>> Thanks.  If no one objects in a week, please push to the master
>>> branch.
>>>
>>
>>

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 11. Mai 2016 um 10:14 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 10 May 2016 21:25:47 +0000
> > Cc: 23288@debbugs.gnu.org
> >
> >  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also
> be related.
> >
> > I've attached a new patch that should hopefully correct the behavior.
>
> Thanks.  If no one objects in a week, please push to the master
> branch.
>

Not to emacs-25? The bug is rather annoying, and the fix comparatively
simple.

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

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 11 May 2016 13:32:09 +0000
> Cc: nilsb@google.com, 23288@debbugs.gnu.org
> 
>  Thanks. If no one objects in a week, please push to the master
>  branch.
> 
> Not to emacs-25? The bug is rather annoying, and the fix comparatively simple. 

It didn't look simple to me, but maybe I'm missing something.

CC'ing John, so he could make the decision.

Thanks.

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[John Wiegley]

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

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> It didn't look simple to me, but maybe I'm missing something.

It doesn't look simple enough for 25.1.

Remember, 25.2 can come out as quickly after as we want it to, so please tag
it for that release.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Glenn Morris]

FWIW, this seems to me like a bad bug that should be fixed for 25.1.
Accidentally pasting the selection can be bad (eg if you don't notice it
happened and there was something sensitive there).

(See also merged #22434. Marked as release blocking for 3+ months.)

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Philipp Stephani <p.stephani2@gmail.com>,  John Wiegley <johnw@gnu.org>,  23288@debbugs.gnu.org,  nilsb@google.com
> Date: Wed, 11 May 2016 14:40:54 -0400
> 
> 
> FWIW, this seems to me like a bad bug that should be fixed for 25.1.
> Accidentally pasting the selection can be bad (eg if you don't notice it
> happened and there was something sensitive there).
> 
> (See also merged #22434. Marked as release blocking for 3+ months.)

Sorry, I don't see the list of blocking bugs being treated seriously.
If we did treat it seriously, we wouldn't have had "deep freeze" on
emacs-25, and wouldn't be planning the release in less than a month.

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> (See also merged #22434. Marked as release blocking for 3+ months.)

> Sorry, I don't see the list of blocking bugs being treated seriously. If we
> did treat it seriously, we wouldn't have had "deep freeze" on emacs-25, and
> wouldn't be planning the release in less than a month.

I'm not sure how those two things follow from not treating the blocking list
seriously, Eli. Can you please explain? As far as I understand it, others will
treat it as seriously as we do.

As it stands right now, I think many of the bugs on that list do not deserve
to be there, so it's a matter of priority setting.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Glenn Morris]

Eli Zaretskii wrote:

> Sorry, I don't see the list of blocking bugs being treated seriously.
> If we did treat it seriously, we wouldn't have had "deep freeze" on
> emacs-25, and wouldn't be planning the release in less than a month.

(off-topic)

Indeed. Though I have noticed some people looking at those issues
recently (thanks!).

I'd hope those in charge would be encouraging people to work on those
issues, but I haven't noticed it happening. Previous Emacs releases
weren't time-based, but were made when they were ready.

Also, good luck proof-reading every manual chapter in a month! :)

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Cc: Glenn Morris <rgm@gnu.org>,  p.stephani2@gmail.com,  23288@debbugs.gnu.org,  nilsb@google.com
> Date: Wed, 11 May 2016 22:42:21 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> (See also merged #22434. Marked as release blocking for 3+ months.)
> 
> > Sorry, I don't see the list of blocking bugs being treated seriously. If we
> > did treat it seriously, we wouldn't have had "deep freeze" on emacs-25, and
> > wouldn't be planning the release in less than a month.
> 
> I'm not sure how those two things follow from not treating the blocking list
> seriously, Eli. Can you please explain?

The long list of blocking bugs is simply incompatible with the current
release schedule.  And I don't see any intensive work to resolve those
bugs.

> As it stands right now, I think many of the bugs on that list do not deserve
> to be there, so it's a matter of priority setting.

Bugs that don't deserve being there should be removed from the list.
After that is done, we could again decide whether we can still release
as planned.

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> Bugs that don't deserve being there should be removed from the list. After
> that is done, we could again decide whether we can still release as planned.

Agreed. Then consider the release date flexible until we've determined what
our real workload is.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23288: 25.0.92; Clicking on links inserts primary X selection

[Philipp Stephani]

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

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 11. Mai 2016 um 10:14 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Tue, 10 May 2016 21:25:47 +0000
> > Cc: 23288@debbugs.gnu.org
> >
> >  Agreed. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17887 might also
> be related.
> >
> > I've attached a new patch that should hopefully correct the behavior.
>
> Thanks.  If no one objects in a week, please push to the master
> branch.
>

Pushed to master (after moving  the test file).

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