bug#73911: Issue with `widget-specify-button'

[Mauro Aranda <maurooaranda@gmail.com>]

Hi Eli, I'm sorry I was unable to respond to your messages all this
time.  It has been a very difficult year for me.

Eli Zaretskii <eliz@gnu.org> writes:

 > Ping! Ping! Ping!
 >
 >> Cc: da_vid@orange.fr, 73911@debbugs.gnu.org
 >> Date: Sat, 14 Dec 2024 11:35:59 +0200
 >> From: Eli Zaretskii <eliz@gnu.org>
 >>
 >> Ping! Ping!  Mauro, please respond.
 >>
 >> > Cc: da_vid@orange.fr, 73911@debbugs.gnu.org
 >> > Date: Sat, 30 Nov 2024 11:54:11 +0200
 >> > From: Eli Zaretskii <eliz@gnu.org>
 >> >
 >> > Ping! Ping! Mauro, are you there?
 >> >
 >> > > Cc: da_vid@orange.fr, 73911@debbugs.gnu.org
 >> > > Date: Sat, 16 Nov 2024 15:44:39 +0200
 >> > > From: Eli Zaretskii <eliz@gnu.org>
 >> > >
 >> > > Ping! Mauro, could you please respond?
 >> > >
 >> > > > Cc: 73911@debbugs.gnu.org
 >> > > > Date: Sat, 02 Nov 2024 13:39:38 +0200
 >> > > > From: Eli Zaretskii <eliz@gnu.org>
 >> > > >
 >> > > > > Date: Sun, 20 Oct 2024 15:01:47 +0200
 >> > > > > From:  David Ponce via "Bug reports for GNU Emacs,
 >> > > > >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
 >> > > > >
 >> > > > > I found that `widget-specify-button' produces unexpected 
result when
 >> > > > > button prefix string has a space (and maybe others) display
 >> > > > > properties.
 >> > > > >
 >> > > > > To illustrate the issue with emacs -Q, evaluate the below 
code in the
 >> > > > > *scratch* buffer:
 >> > > > >
 >> > > > > (require 'wid-edit)
 >> > > > > (let* ((thin-space (propertize " " 'display '(space :width 
0.5)))
 >> > > > >         (widget-push-button-prefix thin-space)
 >> > > > >         (widget-push-button-suffix thin-space))
 >> > > > >    (widget-create 'push-button
 >> > > > >          :notify #'ignore
 >> > > > >          :button-face '( ;;:background "lightgrey"
 >> > > > >                  :box t)
 >> > > > >          ;;:suppress-face t
 >> > > > >          "OK")
 >> > > > >    (insert "\n"))
 >> > > > >
 >> > > > > I found that the culprit is the invisible space that
 >> > > > > `widget-specify-button' puts at the beginning of the button 
(no idea
 >> > > > > why) using a 'before-string overlay property, at line 467:
 >> > > > >
 >> > > > > (overlay-put overlay 'before-string #(" " 0 1 (invisible t)))
 >> > > > >
 >> > > > > Giving this invisible space the same face property as the 
rest of the
 >> > > > > button fixed the issue for me.  This is what I propose in 
the below
 >> > > > > patch.  With this fix the above test will give the expected 
result.
 >> > > > >
 >> > > > > Thanks
 >> > > > >
 >> > > > > diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
 >> > > > > index 1d47f80b0dd..fd1747d3562 100644
 >> > > > > --- a/lisp/wid-edit.el
 >> > > > > +++ b/lisp/wid-edit.el
 >> > > > > @@ -460,17 +460,20 @@ widget-specify-button
 >> > > > >     "Specify button for WIDGET between FROM and TO."
 >> > > > >     (let ((overlay (make-overlay from to nil t nil))
 >> > > > >       (follow-link (widget-get widget :follow-link))
 >> > > > > -    (help-echo (widget-get widget :help-echo)))
 >> > > > > +    (help-echo (widget-get widget :help-echo))
 >> > > > > +    (face (unless (widget-get widget :suppress-face)
 >> > > > > +        (widget-apply widget :button-face-get))))
 >> > > > >       (widget-put widget :button-overlay overlay)
 >> > > > >       (when (functionp help-echo)
 >> > > > >         (setq help-echo 'widget-mouse-help))
 >> > > > > -    (overlay-put overlay 'before-string #(" " 0 1 
(invisible t)))
 >> > > > > +    (overlay-put overlay 'before-string
 >> > > > > +                 (propertize " " 'invisible t 'face face))
 >> > > > >       (overlay-put overlay 'button widget)
 >> > > > >       (overlay-put overlay 'keymap (widget-get widget :keymap))
 >> > > > >       (overlay-put overlay 'evaporate t)
 >> > > > >       ;; We want to avoid the face with image buttons.
 >> > > > > -    (unless (widget-get widget :suppress-face)
 >> > > > > -      (overlay-put overlay 'face (widget-apply widget 
:button-face-get))
 >> > > > > +    (when face
 >> > > > > +      (overlay-put overlay 'face face)
 >> > > > >         (overlay-put overlay 'mouse-face
 >> > > > >              ;; Make new list structure for the mouse-face value
 >> > > > >              ;; so that different widgets will have
 >> > > >
 >> > > > Thanks.
 >> > > >
 >> > > > Mauro, any comments?
 >> > > >
 >> > > >
 >> > > >

I don't really know the effects that this change could have (whether
good or bad).  The added invisible space comes from Bug#51550, which
involved some discussions about the display engine itself.

I had trouble reproducing the bug itself, since I don't understand what
the unexpected result is.

David, I'm sorry it took me so long to reply, but can you explain a bit
more what's the unexpected result? Thank you.