GNU bug report logs - #69942
30.0.50; Fontification of radio-button widget labels

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Stephen Berman <stephen.berman <at> gmx.net>
Subject: bug#69942: closed (Re: bug#69942: 30.0.50; Fontification of
 radio-button widget labels)
Date: Wed, 26 Jun 2024 06:57:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#69942: 30.0.50; Fontification of radio-button widget labels

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 69942 <at> debbugs.gnu.org.

-- 
69942: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69942
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Stephen Berman <stephen.berman <at> gmx.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69942-done <at> debbugs.gnu.org, maurooaranda <at> gmail.com
Subject: Re: bug#69942: 30.0.50; Fontification of radio-button widget labels
Date: Wed, 26 Jun 2024 08:56:16 +0200

Patch pushed to emacs-30 in commit 8d354925ddb, closing the bug.

Steve Berman

On Sat, 08 Jun 2024 14:48:21 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:

> Ping! Ping! Ping!  Mauro, any objections to installing the current
> version of the patch?
>
>> From: Stephen Berman <stephen.berman <at> gmx.net>
>> Cc: maurooaranda <at> gmail.com,  69942 <at> debbugs.gnu.org
>> Date: Sat, 25 May 2024 11:29:28 +0200
>> 
>> On Sat, 25 May 2024 10:41:40 +0300 Eli Zaretskii <eliz <at> gnu.org> wrote:
>> 
>> > Ping! Ping!  Can we please make some progress here?
>> 
>> I haven't noticed any problems with the current version of the patch, so
>> if it were up to me, I'd install it.  (The caveat about my ignorance of
>> widget-inline-p remains, however.)
>> 
>> Steve Berman
>> 
>> >> Cc: 69942 <at> debbugs.gnu.org, maurooaranda <at> gmail.com
>> >> Date: Thu, 09 May 2024 10:22:35 +0300
>> >> From: Eli Zaretskii <eliz <at> gnu.org>
>> >> 
>> >> Ping!  Any further comments, or is there agreement to install the
>> >> proposed patch?
>> >> 
>> >> > From: Stephen Berman <stephen.berman <at> gmx.net>
>> >> > Cc: Eli Zaretskii <eliz <at> gnu.org>,  69942 <at> debbugs.gnu.org
>> >> > Date: Fri, 26 Apr 2024 14:47:26 +0200
>> >> > 
>> >> > On Sun, 21 Apr 2024 21:45:21 +0200 Stephen Berman
>> >> > <stephen.berman <at> gmx.net> wrote:
>> >> > 
>> >> > > On Thu, 18 Apr 2024 15:37:58 +0200 Stephen Berman
>> >> > > <stephen.berman <at> gmx.net> wrote:
>> >> > >
>> >> > >> On Thu, 18 Apr 2024 07:07:43 -0300 Mauro Aranda
>> >> > >> <maurooaranda <at> gmail.com> wrote:
>> >> > >>
>> >> > >>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >> > >>>
>> >> > >>>>> Date: Mon, 8 Apr 2024 07:58:44 -0300
>> >> > >>>>> Cc: 69942 <at> debbugs.gnu.org
>> >> > >>>>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>> >> > >>>>>
>> >> > >>>>> On 6/4/24 06:02, Eli Zaretskii wrote:
>> >> > >>>>>  >> From: Stephen Berman <stephen.berman <at> gmx.net>
>> >> > >>>>>  >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 69942 <at> debbugs.gnu.org
>> >> > >>>>>  >> Date: Mon, 01 Apr 2024 17:21:27 +0200
>> >> > >>>>>  >>
>> >> > >>>>>  >> On Mon, 25 Mar 2024 01:40:36 +0100 Stephen Berman
>> >> > >>>>> <stephen.berman <at> gmx.net> wrote:
>> >> > >>>>>  >>
>> >> > >>>>>  >>> Ok, I've gotten further with implementing disinguishing by faces
>> >> > >>>>>  >>> selected (chosen) and unselected radio buttons in
>> >> > >>>>> radio-button-choice
>> >> > >>>>>  >>> widgets and check boxes in checklist widgets, see the
>> >> > >>>>> attached patch.
>> >> > >>>>>  >>> Initial tests seem ok, but it definitely needs more testing.
>> >> > >>>>>  >>
>> >> > >>>>>  >> Any comments on this patch for using a widget-unselected
>> >> > >>>>> face?  I have
>> >> > >>>>>  >> been detained from further testing this past week, but can
>> >> > >>>>> now resume.
>> >> > >>>>>  >
>> >> > >>>>>  > Mauro, any further comments?
>> >> > >>>>>
>> >> > >>>>> Hi Eli and Stephen,
>> >> > >>>>>
>> >> > >>>>> Please forgive me, for the past 2 weeks I haven't been able to do any
>> >> > >>>>> computer stuff.  If it's OK, please give me until the weekend so I
>> >> > >>>>> can catch up with this and the other 2 bug reports by Stephen.
>> >> > >>>>
>> >> > >>>> Mauro, were you able to find time to look into this and the other 2
>> >> > >>>> bugs?
>> >> > >>>
>> >> > >>> I have, just now.  The patch looks good to me.  It'll be great if
>> >> > >>> Stephen can add some documentation to the manual, so it stays updated.
>> >> > >>> If not, I can do that in a few days.
>> >> > >>
>> >> > >> Sure, I'll add documentation and post it here for approval before
>> >> > >> pushing the changes.
>> >> > >
>> >> > > I've encountered some problems with the patch.  One is that it breaks
>> >> > > the display of all face attributes in the customize-face buffer.  I've
>> >> > > determined the part of the patch that triggers this, though I haven't
>> >> > > yet figured out just why this bit of code breaks the display.  Also, it
>> >> > > appears that the widget-unselected face does not completely replace
>> >> > > widget-inactive where it's intended to do so, but I need to do more
>> >> > > testing and debugging here to find out why.  Until I've fixed these
>> >> > > issues the patch is not suitable for installing, so I'm also holding off
>> >> > > with the accompanying documentation.  (But in preparation for the
>> >> > > documentation I looked more closely at the Widget manual and found
>> >> > > several typos and other issues, for which I opened bug#70502.)
>> >> > 
>> >> > The breakage in displaying all face attributes in the customize-face
>> >> > buffer was caused by the invocation of `(widget-specify-selected child)'
>> >> > in widget-checklist-add-item in the cond-clause satisfying
>> >> > `(widget-inline-p type t)'.  I still don't understand why it has this
>> >> > effect, and I have to admit that I don't understand what an inline
>> >> > widget is.  But simply omitting the invocation of
>> >> > `(widget-specify-selected child)' at this point in the patch does avoid
>> >> > the customize-face breakage and I have not noticed any problems due to
>> >> > this omission.
>> >> > 
>> >> > As for the problematic interaction between the widget-unselected and
>> >> > widget-inactive faces, this seems to have been due to my having copied
>> >> > most of the definition of widget-specify-unselected from that of
>> >> > widget-specify-inactive, and specifically, copying the overlay priority.
>> >> > In the attached patch, widget-specify-unselected now uses a lower
>> >> > overlay priority than the one used in widget-specify-inactive, and in my
>> >> > tests this yields the desired results: the labels of active checkboxes
>> >> > and radio-buttons have widget-unselected face but when these widgets are
>> >> > deactivated, the labels have widget-inactive face.
>> >> > 
>> >> > Another change I've made in the attached patch is to have the default
>> >> > value of widget-unselected face inherit from widget-inactive instead of
>> >> > inheriting from shadow face, like widget-inactive does by default.  This
>> >> > way, if a user customizes widget-inactive, that will also apply by
>> >> > default to widget-unselected, thus retaining the current default widget
>> >> > UI where the labels of checkboxes and radio-buttons have widget-inactive
>> >> > face.  (Thus, the "desired results" in the preceding paragraph are only
>> >> > visible when widget-unselected face is customized to differ from
>> >> > widget-unselected face.)
>> >> > 
>> >> > Finally, regarding documentation of widget-unselected face in the Widget
>> >> > manual, I think it would be helpful for the documentation to mention the
>> >> > use case that motivated introducing it (following Mauro's suggestion to
>> >> > use a face instead of a defcustom), namely, to visually distinguish the
>> >> > labels of unselected and inactive widgets.  Here is what I suggest:
>> >> > 
>> >> > @deffn Face widget-unselected
>> >> > Face used for unselected widgets.  This face is also used on the text
>> >> > labels of radio-button and checkbox widgets.
>> >> > 
>> >> > The default value inherits @code{widget-inactive} face.  If you want to
>> >> > visually distinguish the labels of unselected active radio-button or
>> >> > checkbox widgets from the labels of unselected inactive widgets,
>> >> > customize this face to a non-default value.
>> >> > @end deffn
>> >> > 
>> >> > Since the recent widget.texi changes that include documenting
>> >> > widget-inactive face have not yet been merged to master, I haven't
>> >> > included the propopsed documentaton of widget-unselected in the attached
>> >> > patch, but if approved, will of course add it after the merge.
>> >> > 
>> >> > Steve Berman
>> >> 
>> >> 
>> >> 
>> >> 
>> 

[Message part 3 (message/rfc822, inline)]

From: Stephen Berman <stephen.berman <at> gmx.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Fontification of radio-button widget labels
Date: Fri, 22 Mar 2024 16:04:15 +0100

[Message part 4 (text/plain, inline)]

In bug#69941 I reported a faulty fontification of radio-button widgets
and noted in passing that the labels associated with the radio buttons
also have unexpected faces, namely, the widget-inactive face regardless
of whether the associated radio buttons are inactive or active (except
for the label of a radio button that has been pressed, which has the
default face).  While the faulty fontification discussed in bug#69941
appears to be a real bug, the widget-inactive face assigned to
radio-button labels is apparently by design -- it was present in the
initial commit of the widget library.  But this seems to me to have been
a UX mistake, since it effectively ignores the semantics implied by the
name widget-inactive.  I think a less surprising UI would be for the
labels to be fontified according to the widget's activation state:
default face when the widget is active and widget-inactive face when
it's inactive.  The attached patches provide two possible
implementations of this UI.

The first patch makes the change unconditionally, treating the current
fontification as a UI/UX bug.  But it may be argued that this aspect of
the widget UI should not be unconditionally changed, since it was
apparently a deliberate design choice and there have been (AFAIK) no
complaints about the semantic discrepancy till now.  The lack of
complaint could be because the widget-inactive face inherits the shadow
face, so it is not sharply different from the default face.  But if one
uses a very different face (as I did for illustrative purposes in
bug#69941), the inconsistency is very obvious and (IMO) jarring.
Nevertheless, to allow keeping the current fontification, the second
patch conditionalizes the change from the current fontification by means
of a user option (with the default being the current fontification).

Is either of these changes acceptable?

[widget-radio-nocust.diff (text/x-patch, attachment)]

[widget-radio-cust.diff (text/x-patch, attachment)]

[Message part 7 (text/plain, inline)]

In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.18.0) of 2024-03-22 built on strobelfs2
Repository revision: c1530a2e4973005633ebe00d447f1f3aa1200301
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101009
System Description: Linux From Scratch r12.0-112

Configured using:
 'configure -C --with-xwidgets 'CFLAGS=-Og -g3'
 PKG_CONFIG_PATH=/opt/qt5/lib/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM XWIDGETS GTK3 ZLIB

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.