bug#53636: 29.0.50; face-remapping broken on master

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Sorry, I don't follow -- examples of what that had hundreds of faces?
>
> People reported they have hundreds of faces defined in their Emacs
> sessions.  See this message, for example:
>
>   https://lists.gnu.org/archive/html/emacs-devel/2021-05/msg01031.html
>
> It reports to have 600 to 800 faces defined (with 129 in "emacs -Q").

But people aren't remapping all those faces, so the number of faces in
total doesn't seem to matter that much here.  (And looping over 800
faces to pick out dependencies is probably not very expensive.)

> You'd need to loop through all those hundreds of faces for each change
> in face-remapping-alist.  Imagine what this will do to "C-x C-=" and
> its ilk.

It would have no impact on those ilks, because the looping will be done
in `face-remap-add-relative' and nowhere else?

>> If it's not, then extending defface to keep track of this
>> information would be simple enough, too.
>
> Still a major complication, since inheritance is recursive.
>
> All that just to have one face that mode-line-* and header-line
> etc. can inherit from?  Doesn't this look like a tail wagging the dog?

Well, fixing bugs is nice anyway.  :-)

So here's a reproducer for the first problem: These two forms, both from
"emacs -Q", have different effects on the new frame:

(progn
  (face-remap-add-relative 'mode-line 'link-visited)
  (make-frame))

vs

(progn
  (face-remap-add-relative 'mode-line 'link-visited)
  (switch-to-buffer "*Messages*")
  (make-frame))

In the first case, the new frame will have mode-line remapped to
link-visited in all windows, while in the latter it won't.  So it looks
like a pretty simple bug -- `make-frame' (when computing the faces for
the new frame) is using the buffer-local value of
`face-remapping-alist' instead of the global one.

But after poking at the code for a couple minutes, I'm not sure where
the computation for faces is done for new faces.  Hm...  is it
`face-spec-recalc'?  Hm...  but that doesn't access
`face-remapping-alist'...   Is this done at a lower level?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no