Subject: 30.0.50; Fontification of radio-button widget labels

From: Stephen Berman <[email-masked]>
Date: Fri, 22 Mar 2024 16:04:15 +0100

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?

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..21848849ba5 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2652,9 +2652,7 @@ widget-radio-add-item
 		(setq child (if chosen
 				(widget-create-child-value
 				 widget type value)
-			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+			      (widget-create-child widget type))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2704,8 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(when match (widget-value-set current value))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2733,11 +2727,9 @@ widget-radio-action
       (dolist (current (widget-get widget :children))
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
-		 (widget-value-set button t)
-		 (widget-apply current :activate))
+		 (widget-value-set button t))
 		((widget-value button)
-		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (widget-value-set button nil)))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 172da3db1e0..616286a817d 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2591,6 +2591,16 @@ 'radio-button
   :off "( )"
   :off-glyph "radio")

+(defcustom widget-radio-face-from-state nil
+  "How to fontify the label of a radio button.
+If non-nil, use `widget-inactive' face for the label if the
+radio-button-choice widget is inactive and default face if it is active.
+If nil, use the default face for the label only if the associated radio
+button is pressed, otherwise use `widget-inactive' face."
+  :type 'boolean
+  :group 'widget-faces
+  :version "30.1")
+
 (defun widget-radio-button-notify (widget _child &optional event)
   ;; Tell daddy.
   (widget-apply (widget-get widget :parent) :action widget event))
@@ -2653,8 +2663,9 @@ widget-radio-add-item
 				(widget-create-child-value
 				 widget type value)
 			      (widget-create-child widget type)))
-		(unless chosen
-		  (widget-apply child :deactivate)))
+		(unless widget-radio-face-from-state
+                  (unless chosen
+		    (widget-apply child :deactivate))))
 	       (t
 		(error "Unknown escape `%c'" escape)))))
      ;; Update properties.
@@ -2706,12 +2717,14 @@ widget-radio-value-set
 	     (match (and (not found)
 			 (widget-apply current :match value))))
 	(widget-value-set button match)
-	(if match
-	    (progn
-	      (widget-value-set current value)
-	      (widget-apply current :activate))
-	  (widget-apply current :deactivate))
-	(setq found (or found match))))))
+	(if widget-radio-face-from-state
+            (when match (widget-value-set current value))
+	  (if match
+              (progn
+                (widget-value-set current value)
+                (widget-apply current :activate))
+            (widget-apply current :deactivate)))
+        (setq found (or found match))))))

 (defun widget-radio-validate (widget)
   ;; Valid if we have made a valid choice.
@@ -2734,10 +2747,12 @@ widget-radio-action
 	(let* ((button (widget-get current :button)))
 	  (cond ((eq child button)
 		 (widget-value-set button t)
-		 (widget-apply current :activate))
+                 (unless widget-radio-face-from-state
+		   (widget-apply current :activate)))
 		((widget-value button)
 		 (widget-value-set button nil)
-		 (widget-apply current :deactivate)))))))
+		 (unless widget-radio-face-from-state
+                   (widget-apply current :deactivate))))))))
   ;; Pass notification to parent.
   (widget-apply widget :notify child event))

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