Subject: Re: bug#51550: 29.0.50; Customize Group INS buttons sometimes don't have a left box line

From: Mauro Aranda <[email-masked]>
Date: Thu, 16 Jan 2025 18:54:45 -0300

> Eli Zaretskii <[email-masked]> writes:
 >
 > >> From: Po Lu <[email-masked]>
 > >> Cc: [email-masked],  [email-masked], [email-masked]
 > >> Date: Wed, 29 Dec 2021 21:54:38 +0800
 > >>
 > >>   (with-current-buffer (get-buffer-create "*test*")
 > >>     (insert #("foo\nfoo" 0 4 (face widget-field)))
 > >>     (let ((overlay (make-overlay 5 8 nil t nil)))
 > >>       (overlay-put overlay 'before-string (propertize " " 
'invisible t))
 > >>       (overlay-put overlay 'face custom-button)))
 > >>
 > >> After this, I would have expected the second "foo" in *test* to begin
 > >> with a box line, but it doesn't.
 > >
 > > It was a subtle bug in how we handle face's box attribute when the
 > > display engine changes the object on which it iterates.  Compare the
 > > effect of this:
 > >
 > >   (with-current-buffer (get-buffer-create "*test*")
 > >     (load "wid-edit")
 > >     (load "cus-edit")
 > >     (insert #("foo\nfoo" 0 4 (face widget-field)))
 > >     (let ((overlay (make-overlay 5 8 nil t nil)))
 > >       (overlay-put overlay 'before-string "X")
 > >       (overlay-put overlay 'face custom-button)))
 > >
 > > with this:
 > >
 > >   (with-current-buffer (get-buffer-create "*test*")
 > >     (load "wid-edit")
 > >     (load "cus-edit")
 > >     (insert #("foo\nXfoo" 0 4 (face widget-field)))
 > >     (let ((overlay (make-overlay 6 9 nil t nil)))
 > >       (overlay-put overlay 'face custom-button)))
 > >
 > > The change of face between the "X" and the following button should be
 > > displayed the same, but wasn't.
 > >
 > > I installed a fix on master.  Please see if it solves the real-life
 > > use case, and close the bug if it does.
 >
 > Yes, that fixes the problem.  I'll push the corresponding fix to the
 > custom widget code, and I'm closing this bug.  Thanks.

I've reopened this bug, since the fix caused another bug (Bug#73911),
and the fix for that bug (which made sense to me) caused this one to
reappear.

I'll try to summarize things as I understand them.  Please correct me if
I'm wrong:

The change that made visible the bug is:

commit 8b024a6ff10f7907445ea60c4db8355638616ed1
Author: Stefan Kangas <stefan <at> marxist.se>
Date:   Mon Mar 15 00:27:20 2021 +0100

     * lisp/wid-edit.el (widget-field): Add subtle border to face.

  lisp/wid-edit.el | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

The added :box attribute to the widget-field face makes it collide with
the :box attribute of the custom-button face somehow, and it results in
buttons that follow an editable-field widget that happen to end with a
newline character, being displayed without a left box line.

The discussion was about whether it was a bug in the display code or in
the way the faces and overlays get set in the Customize buffer, and
first Eli suggested to add an invisible space before the buttons,
outside any overlay.  I understand this is a workaround, but could be
the way to fix the bug indeed.

Adding the invisible space inside the button overlay seemed to work,
although it led to Bug#73911.  So it's not the right fix, I think.

Now, I think one way to fix this (assuming we're sure we want the
:box attribute for the widget-field face) is not by changing the widgets
(or whatever content) that come after the editable-field widget, since
that's something we can't control in the Widget library. Instead,
the focus should be in the editable-field.  I attach a patch to show
what I'm thinking about, but I need to test it more.

From fa9db896b5921f0217c5512939b31b487d66e6c1 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Thu, 16 Jan 2025 18:45:18 -0300
Subject: [PATCH] Avoid display bugs after editable-field widgets

* lisp/wid-edit.el (widget-specify-button): Don't add an
invisible space before the button to avoid bugs that surface
with the widget-field face.  The Widget library is not
the one that puts a :box attribute for the button face, and we
can't control what other library users put after editable-field
widgets.
(widget-specify-field): Add the invisible space here, after the
newline.  Since we're the ones that added a :box attribute to
the widget-field face, we should take care of protecting
whatever comes after the editable-field widget.  (Bug#51550)
---
 lisp/wid-edit.el | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index ca4e1e184cb..f9314b04020 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -424,7 +424,12 @@ widget-specify-field
 	(overlay-put overlay 'local-map keymap)
 	(overlay-put overlay 'face face)
 	(overlay-put overlay 'follow-link follow-link)
-	(overlay-put overlay 'help-echo help-echo))
+        (overlay-put overlay 'help-echo help-echo)
+        ;; Since the `widget-field' face has a :box attribute, we need add
+        ;; some character with no face after the newline character, to avoid
+        ;; clashing with text that comes after the editable-field and that it
+        ;; also has a face with a :box attribute.  (Bug#51550)
+        (overlay-put overlay 'after-string #(" " 0 1 (invisible t))))
       (setq to (1- to))
       (setq rear-sticky t))
     (let ((overlay (make-overlay from to nil nil rear-sticky)))
@@ -465,8 +470,6 @@ widget-specify-button
     (widget-put widget :button-overlay overlay)
     (when (functionp help-echo)
       (setq help-echo 'widget-mouse-help))
-    (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)
-- 
2.34.1