bug#34506: 27.0.50: push-button bug with basic text-property button

bug#34506: 27.0.50: push-button bug with basic text-property button

[Bob Weiner]

From the Elisp manual (https://www.gnu.org/software/emacs/manual/html_node/elisp/Manipulating-Buttons.html), we have these accurate explanations:

  — Function: button-at pos
  Return the button at position pos in the current buffer, or nil. If the button at pos is a text property button, the return value is a marker pointing to pos.

  — Function: button-activate button &optional use-mouse-action
  Call button's action property (i.e., invoke the function that is the value of that property, passing it the single argument button). If use-mouse-action is non-nil, try to invoke the button's mouse-action property instead of action; if the button has no mouse-action property, use action as normal.

-----

With point on a "Choose" button in a customize-group buffer, point is on
a text-property button and (button-at (point)) returns a marker object
rather than a button whose action is a marker object.  Thus, if one
calls (push-button) at that location, it sends this marker object as the
button argument to 'button-activate' which then triggers an error when
it tries to funcall the button's action which is nil in this case.
Shouldn't there be additional logic that checks if the button itself is
a marker and then uses the button as the action in that case?

Related to this:  (button-type (button-at (point))) returns nil which seems
to contradict the fact that button-at returns non-nil.

Am I missing things here or does button-activate need additional code?

Thanks,

Bob


-------



In GNU Emacs 27.0.50 (build 13, x86_64-apple-darwin16.7.0, NS appkit-1504.83 Version 10.12.6 (Build 16G1036))
 of 2017-12-17 built on bka-iMac.local
Repository revision: 36375d35aa06e84865cce678559ddfa8f79a9775
Windowing system distributor 'Apple', version 10.3.1561
Recent messages:
[Sat 04:46:35 PM] 
[Sat 04:46:35 PM] 
[Sat 04:46:35 PM] Result: nil
[Sat 04:46:37 PM] 
[Sat 04:46:37 PM] 
[Sat 04:46:37 PM] Result: nil
[Sat 04:46:38 PM] 
[Sat 04:46:38 PM] nil
[Sat 04:47:16 PM] 
Back to top level

Configured using:
 'configure --with-ns --with-imagemagick --without-pop --with-mailutils
 CC=clang 'CFLAGS=-O3 -g''

Configured features:
JPEG NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS LCMS2

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

Major mode: Custom

Minor modes in effect:
  recentf-mode: t
  treemacs-follow-mode: t
  treemacs-filewatch-mode: t
  treemacs-git-mode: deferred
  treemacs-fringe-indicator-mode: t
  async-bytecomp-package-mode: t
  diff-auto-refine-mode: t
  desktop-save-mode: t
  winner-mode: t
  which-key-mode: t
  which-function-mode: t
  persistent-scratch-autosave-mode: t
  global-edit-server-edit-mode: t
  delete-selection-mode: t
  auto-compile-on-load-mode: t
  auto-compile-on-save-mode: t
  column-number-indicator-zero-based: t
  dynamic-completion-mode: t
  eros-mode: t
  shell-dirtrack-mode: t
  show-paren-mode: t
  global-company-mode: t
  company-mode: t
  ace-window-display-mode: t
  major-mode-icons-mode: t
  minibuffer-depth-indicate-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

bug#34506: 27.0.50: push-button bug with basic text-property button

[Eli Zaretskii]

> From: Bob Weiner <rsw@gnu.org>
> Date: Sat, 16 Feb 2019 17:08:47 -0500
> 
> With point on a "Choose" button in a customize-group buffer, point is on
> a text-property button and (button-at (point)) returns a marker object
> rather than a button whose action is a marker object.  Thus, if one
> calls (push-button) at that location, it sends this marker object as the
> button argument to 'button-activate' which then triggers an error when
> it tries to funcall the button's action which is nil in this case.
> Shouldn't there be additional logic that checks if the button itself is
> a marker and then uses the button as the action in that case?
> 
> Related to this:  (button-type (button-at (point))) returns nil which seems
> to contradict the fact that button-at returns non-nil.
> 
> Am I missing things here or does button-activate need additional code?

button-activate and push-button already include that additional code,
but you are trying to invoke them on a kind of "button" that they
don't know how to handle.  The problem is that "button" is overloaded
here: buttons created by Customize are not of the kind supported by
functions from button.el, you need to invoke functions described in
widget.info instead.  The "text-property buttons" mentioned in the
documentation of button-at etc. are those created by make-text-button
and insert-text-button, not those created by Customize.

The ELisp manual hints on this at the beginning the parent node,
"Buttons".  Maybe that's not clear enough; patches to make that more
clear are welcome.  Other than that, I don't see a bug here.

Thanks.

bug#34506: 27.0.50: push-button bug with basic text-property button

[Robert Weiner]

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

On Sun, Feb 17, 2019 at 10:24 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Bob Weiner <rsw@gnu.org>
> > Date: Sat, 16 Feb 2019 17:08:47 -0500
> >
> > With point on a "Choose" button in a customize-group buffer, point is on
> > a text-property button and (button-at (point)) returns a marker object
> > rather than a button whose action is a marker object.  Thus, if one
> > calls (push-button) at that location, it sends this marker object as the
> > button argument to 'button-activate' which then triggers an error when
> > it tries to funcall the button's action which is nil in this case.
> > Shouldn't there be additional logic that checks if the button itself is
> > a marker and then uses the button as the action in that case?
> >
> > Related to this:  (button-type (button-at (point))) returns nil which
> seems
> > to contradict the fact that button-at returns non-nil.
> >
> > Am I missing things here or does button-activate need additional code?
>
> button-activate and push-button already include that additional code,
> but you are trying to invoke them on a kind of "button" that they
> don't know how to handle.  The problem is that "button" is overloaded
> here: buttons created by Customize are not of the kind supported by
> functions from button.el, you need to invoke functions described in
> widget.info instead.  The "text-property buttons" mentioned in the
> documentation of button-at etc. are those created by make-text-button
> and insert-text-button, not those created by Customize.
>

I really don't fully understand what you are saying here.
There is a section in widget.info which describes use of the (push-button)
function which is defined in button.el, so although I
understand you are saying there are two different types
of buttons, it seems like they are connected.  Maybe
the custom widgets use text-property buttons.

If you could, for each of the two types of buttons,
just show code that finds the button at point and then
activates it, so I can see the difference clearly.  That
would be most helpful.

And what about (button-type (button-at (point))) returning
nil when button-at returns non-nil.  Both of these functions
operate on push-buttons as the button.el code reflects, right?
If so, then that should be a bug.  If not, then it could use
some explanation.

Thanks,

Bob

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

bug#34506: 27.0.50: push-button bug with basic text-property button

[Eli Zaretskii]

> From: Robert Weiner <rsw@gnu.org>
> Date: Sun, 17 Feb 2019 18:46:09 -0500
> Cc: 34506@debbugs.gnu.org
> 
>  button-activate and push-button already include that additional code,
>  but you are trying to invoke them on a kind of "button" that they
>  don't know how to handle.  The problem is that "button" is overloaded
>  here: buttons created by Customize are not of the kind supported by
>  functions from button.el, you need to invoke functions described in
>  widget.info instead.  The "text-property buttons" mentioned in the
>  documentation of button-at etc. are those created by make-text-button
>  and insert-text-button, not those created by Customize.
> 
> I really don't fully understand what you are saying here.

I'm saying that a "button" created by Customize is radically different
from "buttons" that button.el functions can handle.  They are
different objects, and it's unfortunate that both kinds are named the
same.

> There is a section in widget.info which describes use of the (push-button)
> function which is defined in button.el

No, push-button described in widget.info is a type of widget, not a
function.  Again, the same name used for two very different things.

> Maybe the custom widgets use text-property buttons.

Custom widget button do indeed use text properties, but those
properties are entirely different from the properties used by
text-buttons created by button.el.  You can see that yourself by using
the describe-text-properties command with point on each type of
button.

> If you could, for each of the two types of buttons,
> just show code that finds the button at point and then
> activates it, so I can see the difference clearly.  That
> would be most helpful.

If you type "C-h c RET" on a button in a Customize buffer, you will
see that RET invokes Custom-newline there -- this is the way to "push"
the button widgets that Custom uses.  By contrast, if you do the same
on a button created by button.el, like a hyperlink in the *Help*
buffer, you will see that RET invokes the command push-button there, a
different command.  These are the two ways of pushing these two
different kinds of buttons.

> And what about (button-type (button-at (point))) returning
> nil when button-at returns non-nil.  Both of these functions
> operate on push-buttons as the button.el code reflects, right?
> If so, then that should be a bug.  If not, then it could use
> some explanation.

button-type requires a button as an argument, whereas button-at is
documented to return a marker for text-buttons.  So you cannot safely
invoke button-type if the button at point might be of the text-button
type.  IOW, the text-button is not an object, it is a place in the
buffer where the button starts.

bug#34506: 27.0.50: push-button bug with basic text-property button

[Robert Weiner]

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

Thanks Eli, that is much clearer.  I can work with that explanation.

Your last paragraph indicates that the button API by itself could use some
improvement.
How does one obtain a button to send to button-type if not button-at?

Also, a confusing part of the widget documentation is this:

-------

5.4 The ‘push-button’ Widget
============================

Syntax:

     TYPE ::= (push-button [KEYWORD ARGUMENT]...  [ VALUE ])

-------

The above syntax description of course looks like a lisp-function call of
push-button.
At the very least, changing this thing's name to widget-button would
prevent such
confusion.

-- Bob

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

bug#34506: 27.0.50: push-button bug with basic text-property button

[Eli Zaretskii]

> From: Robert Weiner <rsw@gnu.org>
> Date: Mon, 18 Feb 2019 11:56:49 -0500
> Cc: 34506@debbugs.gnu.org
> 
> Your last paragraph indicates that the button API by itself could use some improvement.

Possibly.

> How does one obtain a button to send to button-type if not button-at?

Looks like you need to use text-properties-at, and then look at the
result: the type is stored in the 'category' property there.

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

Robert Weiner <rsw@gnu.org> writes:

> Thanks Eli, that is much clearer.  I can work with that explanation.
>
> Your last paragraph indicates that the button API by itself could use
> some improvement.  How does one obtain a button to send to button-type
> if not button-at?

You can use button-at if you want to be agnostic of the button (not
widget) type.  If you're only dealing with text (not overlay) buttons,
you can avoid creating markers by passing a buffer position like (point)
directly to button-type.

If you are dealing with widgets, on the other hand, you should use
widget-at and widget-type in place of button-at and button-type,
respectively.  The widget and button APIs are mutually incompatible.

-- 
Basil

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Weiner <rsw@gnu.org>
>> Date: Sun, 17 Feb 2019 18:46:09 -0500
>> Cc: 34506@debbugs.gnu.org
>> 
>> And what about (button-type (button-at (point))) returning
>> nil when button-at returns non-nil.  Both of these functions
>> operate on push-buttons as the button.el code reflects, right?
>> If so, then that should be a bug.  If not, then it could use
>> some explanation.
>
> button-type requires a button as an argument, whereas button-at is
> documented to return a marker for text-buttons.  So you cannot safely
> invoke button-type if the button at point might be of the text-button
> type.

Buffer positions, markers, and overlays all qualify as "buttons", so
button-type works with both text- and overlay-buttons (but not widgets).

So I'm guessing what you meant is "you cannot safely invoke button-type
if the button at point might be a widget rather than a button".

-- 
Basil

bug#34506: 27.0.50: push-button bug with basic text-property button

[Robert Weiner]

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

On Mon, Feb 18, 2019 at 3:51 PM Basil L. Contovounesios <contovob@tcd.ie>
wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Robert Weiner <rsw@gnu.org>
> >> Date: Sun, 17 Feb 2019 18:46:09 -0500
> >> Cc: 34506@debbugs.gnu.org
> >>
> >> And what about (button-type (button-at (point))) returning
> >> nil when button-at returns non-nil.  Both of these functions
> >> operate on push-buttons as the button.el code reflects, right?
> >> If so, then that should be a bug.  If not, then it could use
> >> some explanation.
> >
> > button-type requires a button as an argument, whereas button-at is
> > documented to return a marker for text-buttons.  So you cannot safely
> > invoke button-type if the button at point might be of the text-button
> > type.
>
> Buffer positions, markers, and overlays all qualify as "buttons", so
> button-type works with both text- and overlay-buttons (but not widgets).
>

But as I think I noted in my first message, my recollection
is that button-type returned nil when given a marker value
returned from button-at.  Since widgets use text-properties,
button-at on a widget can return a non-nil value, so to say that
widgets and buttons are unrelated ignores the programming API.
Maybe the solution is to add a more opaque programming abstraction
atop each type so that they don't expose their underlying implementations
and cause programming errors.

>
> So I'm guessing what you meant is "you cannot safely invoke button-type
> if the button at point might be a widget rather than a button".
>

Yes, again noting that widget documentation specifically mentions
push-buttons
(push-button is the function used to activate buttons).  This negates the
idea
that the two constructs are wholly independent of each other.

Regards,

Bob

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

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

Robert Weiner <rsw@gnu.org> writes:

> On Mon, Feb 18, 2019 at 3:51 PM Basil L. Contovounesios <contovob@tcd.ie> wrote:
>
>  Eli Zaretskii <eliz@gnu.org> writes:
>
>  >> From: Robert Weiner <rsw@gnu.org>
>  >> Date: Sun, 17 Feb 2019 18:46:09 -0500
>  >> Cc: 34506@debbugs.gnu.org
>  >> 
>  >> And what about (button-type (button-at (point))) returning
>  >> nil when button-at returns non-nil.  Both of these functions
>  >> operate on push-buttons as the button.el code reflects, right?
>  >> If so, then that should be a bug.  If not, then it could use
>  >> some explanation.
>  >
>  > button-type requires a button as an argument, whereas button-at is
>  > documented to return a marker for text-buttons.  So you cannot safely
>  > invoke button-type if the button at point might be of the text-button
>  > type.
>
>  Buffer positions, markers, and overlays all qualify as "buttons", so
>  button-type works with both text- and overlay-buttons (but not widgets).
>
> But as I think I noted in my first message, my recollection
> is that button-type returned nil when given a marker value
> returned from button-at.

I think what's confusing you is that button-at returns a marker even
when there is no button at point.  If either of the following two
expressions evaluates to nil, then there is no button at point:

  (button-type (button-at (point)))
  (button-type (point))

If there is something that *looks* like a button at point, yet these
expressions evaluate to nil, then you're probably looking at a widget
instead.

>  Since widgets use text-properties,

AFAICT Customize widgets use overlays, not text properties.  I concluded
this by comparing the results of (text-properties-at (point)) and
(overlays-at (point)) with point at a Customize button.

> button-at on a widget can return a non-nil value, so to say that
> widgets and buttons are unrelated ignores the programming API.

Again, that button-at returns a marker for a widget is a coincidence,
not part of either library's API.  Last time I read/skimmed the relevant
manuals I was not given the impression that these libraries were
related, but suggestions for clarification of their text is always
welcome.

> Maybe the solution is to add a more opaque programming abstraction
> atop each type so that they don't expose their underlying
> implementations and cause programming errors.

Can you please elaborate?  I don't see how either library's
implementation is exposed beyond what is documented in its respective
manual.

>  So I'm guessing what you meant is "you cannot safely invoke button-type
>  if the button at point might be a widget rather than a button".
>
> Yes, again noting that widget documentation specifically mentions push-buttons
> (push-button is the function used to activate buttons).

push-button is indeed a button.el function used to activate buttons, but
this has nothing to do with the push-button widget type, which is
documented under '(widget) Introduction' and '(widget) push-button'.

> This negates the idea that the two constructs are wholly independent
> of each other.

I hope I have managed to convince you otherwise.

-- 
Basil

bug#34506: 27.0.50: push-button bug with basic text-property button

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: <rswgnu@gmail.com>,  <34506@debbugs.gnu.org>
> Date: Mon, 18 Feb 2019 20:51:12 +0000
> 
> So I'm guessing what you meant is "you cannot safely invoke button-type
> if the button at point might be a widget rather than a button".

No, I'm trying to explain why button-type returns nil in the case of
text-buttons.

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: <rswgnu@gmail.com>,  <34506@debbugs.gnu.org>
>> Date: Mon, 18 Feb 2019 20:51:12 +0000
>> 
>> So I'm guessing what you meant is "you cannot safely invoke button-type
>> if the button at point might be a widget rather than a button".
>
> No, I'm trying to explain why button-type returns nil in the case of
> text-buttons.

I'm sorry if I've misunderstood something obvious, but button-type
should never return nil for any kind of button.el button, regardless of
whether it's a text or overlay button.

As stated in '(elisp) Button Types', "every button has a button type".
For example, the following should return 'button:

  (with-temp-buffer
    (let* ((tstart  (prog1 (point) (insert-text-button "text")))
           (ostart  (prog1 (point) (insert-button "overlay")))
           (tbutton (button-at tstart))
           (obutton (button-at ostart))
           (ttype   (button-type tbutton))
           (otype   (button-type obutton)))
      (cl-assert (and tbutton obutton ttype otype))
      (cl-assert (eq ttype otype))
      (cl-assert (eq ttype (button-type tstart)))
      (cl-assert (null (button-type ostart)))
      ttype))

[ By the way, would something like this be welcome as a start for
  test/lisp/button-tests.el? ]

The only cases (within reason) in which button-type may return nil are:

0. button-type is mistakenly given a buffer position POS instead of an
   overlay when the button at POS is an overlay (not text) button, as
   shown in the example above.  Using (button-at POS) avoids this
   problem.

1. button-type is given the value of a widget's (not button's) 'button
   overlay property.  This is not that far-fetched since both libraries
   rely on (get-char-property POS 'button), but I don't see why anyone
   would mix widgets and buttons in the same buffer and run the risk of
   such gotchas.  I believe this contributed in part to Robert's
   confusion.

Am I missing something?

-- 
Basil

bug#34506: 27.0.50: push-button bug with basic text-property button

[Robert Weiner]

> On Feb 19, 2019, at 10:26 AM, Basil L. Contovounesios <contovob@tcd.ie> wrote:
> 
> I don't see why anyone
>   would mix widgets and buttons in the same buffer and run the risk of
>   such gotchas.  I believe this contributed in part to Robert's
>   confusion.

If buttons and widgets are wholly incompatible and separate then button-at should return nil when on a widget even if both buttons and widgets use the same underlying mechanism.  This is what abstractions are for.  The implementation should be hidden.

The abstraction I really want is if there is a clickable item under point, activate it in the most appropriate way, e.g. if selected with a mouse event, use it’s mouse action, etc.  Maybe an activate-at function that handles different types of objects with a priority order if there are multiple matches. 

Bob

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

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

Robert Weiner <rswgnu@gmail.com> writes:

>> On Feb 19, 2019, at 10:26 AM, Basil L. Contovounesios <contovob@tcd.ie> wrote:
>> 
>> I don't see why anyone
>>   would mix widgets and buttons in the same buffer and run the risk of
>>   such gotchas.  I believe this contributed in part to Robert's
>>   confusion.
>
> If buttons and widgets are wholly incompatible and separate then button-at
> should return nil when on a widget even if both buttons and widgets use the same
> underlying mechanism.

How's the following?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: button.diff --]
[-- Type: text/x-diff, Size: 1348 bytes --]

diff --git a/lisp/button.el b/lisp/button.el
index c46f3d9a52..921e84dfa6 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -382,10 +382,12 @@ button-at
 If the button at POS is a text property button, the return value
 is a marker pointing to POS."
   (let ((button (get-char-property pos 'button)))
-    (if (or (overlayp button) (null button))
-	button
-      ;; Must be a text-property button; return a marker pointing to it.
-      (copy-marker pos t))))
+    (and button (get-char-property pos 'category)
+         (if (overlayp button)
+             button
+           ;; Must be a text-property button;
+           ;; return a marker pointing to it.
+           (copy-marker pos t)))))
 
 (defun next-button (pos &optional count-current)
   "Return the next button after position POS in the current buffer.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 52c0b5b74d..b9f98cdc4c 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1163,8 +1163,9 @@ widget-field-list
 
 (defun widget-at (&optional pos)
   "The button or field at POS (default, point)."
-  (or (get-char-property (or pos (point)) 'button)
-      (widget-field-at pos)))
+  (let ((widget (or (get-char-property (or pos (point)) 'button)
+                    (widget-field-at pos))))
+    (and (widgetp widget) widget)))
 
 ;;;###autoload
 (defun widget-setup ()

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]


-- 
Basil

bug#34506: 27.0.50: push-button bug with basic text-property button

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Eli Zaretskii <eliz@gnu.org>, 34506@debbugs.gnu.org
> Date: Mon, 25 Feb 2019 02:40:26 +0000
> 
> > If buttons and widgets are wholly incompatible and separate then button-at
> > should return nil when on a widget even if both buttons and widgets use the same
> > underlying mechanism.
> 
> How's the following?

LGTM, but can we please have a simple test for this, so that this
never again regresses?

Thanks.

bug#34506: 27.0.50: push-button bug with basic text-property button

[Basil L. Contovounesios]

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

tags 34506 fixed
close 34506
quit


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Distinguish-buttons-from-widgets-bug-34506.patch --]
[-- Type: text/x-diff, Size: 5477 bytes --]

From 08235af38c92e95d8ec9d268916d8910ea50ab2d Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 7 Apr 2019 03:36:47 +0100
Subject: [PATCH] Distinguish buttons from widgets (bug#34506)

* lisp/button.el (button-at):
* lisp/wid-edit.el (widget-at): Avoid returning a false positive
when looking for a button and finding a widget, or vice versa.
* test/lisp/button-tests.el:
* test/lisp/wid-edit-tests.el: New files.
---
 lisp/button.el              | 10 ++++++----
 lisp/wid-edit.el            |  5 +++--
 test/lisp/button-tests.el   | 40 +++++++++++++++++++++++++++++++++++++
 test/lisp/wid-edit-tests.el | 39 ++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 6 deletions(-)
 create mode 100644 test/lisp/button-tests.el
 create mode 100644 test/lisp/wid-edit-tests.el

diff --git a/lisp/button.el b/lisp/button.el
index c46f3d9a52..921e84dfa6 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -382,10 +382,12 @@ button-at
 If the button at POS is a text property button, the return value
 is a marker pointing to POS."
   (let ((button (get-char-property pos 'button)))
-    (if (or (overlayp button) (null button))
-	button
-      ;; Must be a text-property button; return a marker pointing to it.
-      (copy-marker pos t))))
+    (and button (get-char-property pos 'category)
+         (if (overlayp button)
+             button
+           ;; Must be a text-property button;
+           ;; return a marker pointing to it.
+           (copy-marker pos t)))))
 
 (defun next-button (pos &optional count-current)
   "Return the next button after position POS in the current buffer.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 52c0b5b74d..b9f98cdc4c 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1163,8 +1163,9 @@ widget-field-list
 
 (defun widget-at (&optional pos)
   "The button or field at POS (default, point)."
-  (or (get-char-property (or pos (point)) 'button)
-      (widget-field-at pos)))
+  (let ((widget (or (get-char-property (or pos (point)) 'button)
+                    (widget-field-at pos))))
+    (and (widgetp widget) widget)))
 
 ;;;###autoload
 (defun widget-setup ()
diff --git a/test/lisp/button-tests.el b/test/lisp/button-tests.el
new file mode 100644
index 0000000000..d54a992ab8
--- /dev/null
+++ b/test/lisp/button-tests.el
@@ -0,0 +1,40 @@
+;;; button-tests.el --- tests for button.el -*- lexical-binding: t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs 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.
+
+;; GNU Emacs 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 GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest button-at ()
+  "Test `button-at' behavior."
+  (with-temp-buffer
+    (should-not (button-at (point)))
+    (let ((button (insert-text-button "text button"))
+          (marker (button-at (1- (point)))))
+      (should (markerp marker))
+      (should (= (button-end button) (button-end marker) (point))))
+    (let ((button  (insert-button "overlay button"))
+          (overlay (button-at (1- (point)))))
+      (should (overlayp overlay))
+      (should (eq button overlay)))
+    ;; Buttons and widgets are incompatible (bug#34506).
+    (widget-create 'link "link widget")
+    (should-not (button-at (1- (point))))))
+
+;;; button-tests.el ends here
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
new file mode 100644
index 0000000000..a4350e715e
--- /dev/null
+++ b/test/lisp/wid-edit-tests.el
@@ -0,0 +1,39 @@
+;;; wid-edit-tests.el --- tests for wid-edit.el -*- lexical-binding: t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs 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.
+
+;; GNU Emacs 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 GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'wid-edit)
+
+(ert-deftest widget-at ()
+  "Test `widget-at' behavior."
+  (with-temp-buffer
+    (should-not (widget-at))
+    (let ((marco (widget-create 'link "link widget"))
+          (polo  (widget-at (1- (point)))))
+      (should (widgetp polo))
+      (should (eq marco polo)))
+    ;; Buttons and widgets are incompatible (bug#34506).
+    (insert-text-button "text button")
+    (should-not (widget-at (1- (point))))
+    (insert-button "overlay button")
+    (should-not (widget-at (1- (point))))))
+
+;;; wid-edit-tests.el ends here
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 659 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 34506@debbugs.gnu.org
>> Date: Mon, 25 Feb 2019 02:40:26 +0000
>> 
>> > If buttons and widgets are wholly incompatible and separate then button-at
>> > should return nil when on a widget even if both buttons and widgets use the same
>> > underlying mechanism.
>> 
>> How's the following?
>
> LGTM, but can we please have a simple test for this, so that this
> never again regresses?

Of course.  I've pushed the attached patch to master, and am thus
closing this bug report.  Let me know if I missed anything.

Thanks,

-- 
Basil