bug#69941: 30.0.50; Faulty fontification of radio button widgets

[Stephen Berman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

On Sun, 21 Apr 2024 21:45:59 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:

> On Thu, 18 Apr 2024 15:38:33 +0200 Stephen Berman <stephen.berman@gmx.net> wrote:
>
>> On Thu, 18 Apr 2024 14:33:57 +0300 Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>>> Date: Thu, 18 Apr 2024 07:18:29 -0300
>>>> Cc: Eli Zaretskii <eliz@gnu.org>, 69941@debbugs.gnu.org,
>>>>  Stefan Monnier <monnier@iro.umontreal.ca>
>>>> From: Mauro Aranda <maurooaranda@gmail.com>
>>>> 
>>>>  > To fix this bug, do you have a preference between this patch for
>>>>  > widget-specify-inactive and the attached patch for
>>>>  > widget-default-create?  Or do you have a better fix?
>>>>  >
>>>>  > Steve Berman
>>>> 
>>>> I don't really have a better fix.  I mean, ideally, we'd find the reason
>>>> why the setting behaves differently for the radio-button-choice widget,
>>>> and only for the first one in a radio widget, as it seems to me. But
>>>> I'll need more time to be able to look into that.
>>>> 
>>>> That said, if Eli is OK with installing a minor hack (with a FIXME,
>>>> please), I don't have problems.  And since it's a hack (and hopefully
>>>> temporary), it's better if we keep it at widget-default-create then.
>>>
>>> My opinion doesn't matter much in this case.  If you two agree on a
>>> solution, feel free to install it, even if it is not 110% clean.
>>
>> I've been using the patch for for widget-specify-inactive in an
>> application I'm developing that exercises radio-button-choice widgets,
>> but I'll switch to using the patch for widget-default-create instead.
>> I've been encountering inconsistent behavior in combination with the use
>> of widget-unselected face that I haven't tracked down the cause of yet.
>> I don't expect using the patch for widget-default-create will improve
>> this issue, but I'll find out.  I also plan to test that patch in
>> combination with widget-unselected face with checklist widgets, which my
>> application currently does not use.  I'll report back here before
>> committing the patch for widget-default-create (or something else,
>> depending on the outcome of further testing).
>
> Just a brief status report: My testing does indeed indicate that the
> fontification problem with radio-button-choice also occurs with
> checklist widgets, though the pattern appears not to be identical; I
> need to do more testing and debugging.

Further testing confirms that checklists are subject to this problem, so
I've added them to the attached patch.  The rest of this post reports
results from and speculations based on my debugging efforts, which
remain somewhat inconclusive.

According to my tests, checklists and radio-button-choice widgets do
indeed display the same problem with the first checkbox or radio-button,
respectively: if it's selected and then the parent widget is
deactivated, then the button/checkbox incorrectly does not have
widget-inactive face.  I think the reason for this is that selecting
inserts "[X]" for the checkbox and "(*)" for the radio-button, and since
the parent widget's :from property has marker insertion type `t', its
position advances to after the insertion (I guess this is because the
starting position of the first checkbox/button coincides with the parent
widget's :from), so the overlay with the widget-inactive face beginning
at :from does not cover the checkbox/button.

But checklists and radio-button-choice widgets differ when a non-initial
checkbox/button is selected.  With checklists, multiple checkboxes can
be selected, and selecting the second checkbox does not advance the
parent widget's :from position, unlike with radio-button-choice widget's
when selecting the second radio-button, as I reported in my OP.  I think
this is because in radio-button-choice widgets only one radio-button can
be selected, so selecting any one triggers the :from marker's advancing.
I could not verify this hypothesis through debugging because I was
unable to find out exactly when this happens.  The marker advance is
done in the C code, I think at adjust_markers_for_insert in insdel.c; I
set a gdb breakpoint there and this triggers when I select a radio
button, but it's too early: a lot happens in wid-edit.el between
selecting a button and the selection becoming visible, and the
breakpoint triggered so often that I gave up.  Is there a way to make a
breakpoint in the C code trigger only when a specific part of
wid-edit.el is evaluated?

Nevertheless, by assigning the :from marker the insertion type nil in
widget-default-create when the widget is either a checklist or
radio-button-choice, does result in the correct fontification of the
first checkbox/radio-button in all tests I've conducted with varying the
selection.  And conceptually it seems to me correct that :from should
not advance with these widgets: selecting a checkbox or button is
operationally quite different from inserting text (e.g. in an
editable-field widget), even though the implementation technically
involves insertion.  So I think the attached patch is at least a viable
stopgap, until a better (or at least less ad hoc) fix is found.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: widget-default-create patch --]
[-- Type: text/x-patch, Size: 1058 bytes --]

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index dc481d4d0a5..9304002ff52 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1757,8 +1757,16 @@ widget-default-create
        (goto-char value-pos)
        (widget-apply widget :value-create)))
    (let ((from (point-min-marker))
-	 (to (point-max-marker)))
-     (set-marker-insertion-type from t)
+	 (to (point-max-marker))
+         ;; Advancing the `:from' marker of a checklist or
+         ;; radio-button-choice widget on selecting a checkbox or a
+         ;; radio-button, which inserts "[X]" or "(*)", can result in
+         ;; misfontifying the first checkbox (bug#69941).  To ensure
+         ;; correct fontification, assign `:from' the marker insertion
+         ;; type `nil', so it does not advance.
+         (from-mit (not (memq (widget-type widget)
+                              '(checklist radio-button-choice)))))
+     (set-marker-insertion-type from from-mit)
      (set-marker-insertion-type to nil)
      (widget-put widget :from from)
      (widget-put widget :to to)))