Subject: bug#13476: 24.3.50; Reverting scroll-bar face customization has no effect

From: Mauro Aranda <[email-masked]>
Date: Mon, 28 Feb 2022 12:25:06 -0300

Eli Zaretskii <[email-masked]> writes:

>> From: Mauro Aranda <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked]
>> Date: Mon, 28 Feb 2022 11:04:13 -0300
>> 
>> Eli Zaretskii <[email-masked]> writes:
>> 
>> >> From: Mauro Aranda <[email-masked]>
>> >> Cc: [email-masked],  [email-masked],  [email-masked]
>> >> Date: Mon, 28 Feb 2022 09:59:15 -0300
>> >> 
>> >> > --- a/lisp/faces.el
>> >> > +++ b/lisp/faces.el
>> >> > @@ -1743,7 +1743,14 @@ The following sources are applied in this order:
>> >> >          (and tail (face-spec-set-2 face frame
>> >> >                                     (list :extend (cadr tail))))))
>> >> >      (setq face-attrs (face-spec-choose (get face 'face-override-spec) frame))
>> >> > -    (face-spec-set-2 face frame face-attrs)))
>> >> > +    (face-spec-set-2 face frame face-attrs)
>> >> > +    (when (and (fboundp 'set-frame-parameter) ; This isn't available
>> >> > +                                              ; during loadup.
>> >> > +               (eq face 'scroll-bar))
>> >> > +      ;; Set the `scroll-bar-foreground' and `scroll-bar-background'
>> >> > +      ;; frame parameters.  (bug#13476)
>> >> > +      (set-frame-parameter frame 'scroll-bar-foreground (face-foreground face))
>> >> > +      (set-frame-parameter frame 'scroll-bar-background (face-background face)))))
>> >
>> > Why do we need this special treatment of the scroll-bar face?
>> 
>> I haven't read the code yet so I can't really answer to your question.  What
>> I know is what I said on
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=13476#8
>> 
>> Customizing and then resetting to the standard scroll-bar face failed
>> because the 'scroll-bar-foreground parameter wasn't updated after
>> resetting the attributes via face-spec-reset-face.
>
> I don't understand why we need to set the frame parameter when the
> customize the face to begin with, I guess.

In an Emacs prior to c307c9648d541338814fe541389ea8c7a1cf50a5 and
configured with: --without-toolkit-scroll-bars

M-x customize-face RET scroll-bar
Customize the foreground color to "green", and set for current session.
Click the State button to "Revert This Session's Customization".
The scroll bar stays green, even when the foreground color says it is
black.

Evaluate the following:
(face-attribute 'scroll-bar :foreground nil 'default) ; ==> "black"
(frame-parameter (selected-frame) 'scroll-bar-foreground) ; ==> "green"

So, AFAIU the reason the scroll bar stayed green even after the attempt
to go back to the standard was that the frame parameter didn't change
after evaluating
(face-spec-reset 'scroll-bar nil 'reset)

Giving the scroll-bar face a non-trivial spec worked because it caused
some code in internal-set-lisp-face-attribute to update the
frame parameter, but it looks like it caused bad side effects when using
some toolkits.