Subject: bug#61521: "default" is now the first item returned from (font-faces), breaking various code.

From: Kai Ma <[email-masked]>
Date: Sat, 18 Feb 2023 06:17:25 +0800

Gregory Heytings <[email-masked]> writes:

>>> Thanks.  It seems the change you describe is not a recent one: the
>>> first element of the list returned by 'face-list' is 'default' in
>>> Emacs 27, 28, 29 and 30.  (This is caused by e3b8ddd500, since
>>> which frame faces are stored in a hash table instead of an alist.)
>>
>> Right.  So I wonder whether we should remove the nreverse call in
>> face-set-after-frame-default.  WDYT?
>>
>
> There are three occurrences of '(nreverse (face-list))': one in
> facemenu-complete-face-list, which seems to date from the 1990s, one
> in x-create-frame-with-faces, which was added by e3b8ddd500 "to handle
> subtle semantic change to how frame faces propagate, which otherwise
> breaks frame creation with reverse video enabled (bug#41200)", and the
> third one in 'face-set-after-frame-default'.
>
> The comment there is definitely outdated and should be removed, but
> given that '(nreverse (face-list))' is placed in a dolist whose body
> starts with '(face-spec-recalc face frame)', like in
> x-create-frame-with-faces, I'm not sure the nreverse can be removed
> without introducing a subtle bug. It is probably safer to leave the
> code unchanged.

My config becomes broken after pulling this change: the child frame of
vertico-posframe does not appear under certain themes, and signals
errors like:

  Error in post-command-hook (vertico--exhibit): (error "Invalid face" consult-separator)
  Error in post-command-hook (vertico--exhibit): (error "Invalid face" hl-todo)

I'm not sure if this is a downstream issue, but this problem can be
solved by either reverting this commit or removing the nreverse in
x-create-frame-with-faces:

diff --git a/lisp/faces.el b/lisp/faces.el
index 4933b495a6c..e91107e98cc 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2226,7 +2226,7 @@ x-create-frame-with-faces
     (unwind-protect
 	(progn
 	  (x-setup-function-keys frame)
-	  (dolist (face (nreverse (face-list)))
+	  (dolist (face (face-list)))
 	    (face-spec-recalc face frame))
 	  (x-handle-reverse-video frame parameters)
 	  (frame-set-background-mode frame t)

This piece of code (w/ nreverse) was written as part of the hash table
rewrite, and at that time (face-list) did not sort its results.  I don't
know why nreverse is significant here though.