bug#67404: 30.0.50; Issue with :box face attribute

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 30 Nov 2023 07:46:02 -0300
> Cc: 67404@debbugs.gnu.org
> From: Mauro Aranda <maurooaranda@gmail.com>
> 
> On 26/11/23 07:27, Eli Zaretskii wrote:
> 
>  >> Date: Thu, 23 Nov 2023 16:23:15 +0100
>  >> From: David Ponce <da_vid@orange.fr>
>  >>
>  >> Hello,
>  >>
>  >> The below face with specified :box attribute can be defined and further
>  >> used without problem:
>  >>
>  >> (defface test-face
>  >>    '((t
>  >>       (:box (:style released-button :line-width (3 . 0)))
>  >>       ))
>  >>    "Test face.")
>  >>
>  >> (insert (propertize "XXXXX" 'face 'test-face))
>  >>
>  >> However, it is not possible to apply any change to this face with
>  >> `customize-face', for example to change :line-width (3 . 0) to
>  >> :line-width (2 . 0), or to add a :foreground color, etc. This fails
>  >> with the message "set-face-attribute: Invalid face box: :line-width,
>  >> <value>, :style, <value>", and the :box attribute is discarded.
>  >>
>  >> AFAICS, :box attribute with any zero :line-width value works as
>  >> expected: the box line with zero width is not displayed. But zero is
>  >> not considered as a valid value by `set-face-attribute' (in fact, the
>  >> internal function `internal-set-lisp-face-attribute').
>  >>
>  >> For consistency, a zero :line-width value either should be accepted by
>  >> `internal-set-lisp-face-attribute', or never accepted at all.
>  >
>  > Mauro, any comments about these issues?
> 
> Hi Eli,
> 
> Not really, no.  The widget used for line-width just assumes 0 is a
> valid value.  I don't know if that's the case or not, it is not
> explicitly documented in the ELisp manual and I don't really know the
> xfaces.c code.
> 
> For some reason, the code that rejects a value of 0 in
> Finternal_set_lisp_face_attribute doesn't complain (maybe it does not
> get to run?) when using defface, but it does complain when Customize
> tries to set it, by calling face-spec-set.

Thanks for the hints.  This is a (very old) bug in
Finternal_set_lisp_face_attribute.  The value zero for line-width is
not valid, but because of the bug it (and invalid values of other box
attributes) could be accepted, but only if the invalid attribute is
the last attribute of :box.  Try this equivalent definition:

(defface test-face
   '((t
      (:box (:line-width (3 . 0) :style released-button))
      ))
   "Test face.")

which just swaps the :box attributes, and you will see that defface
signals an error.  Customizing the :line-width attribute just happens
to move that attribute to the front of the list, and then you get the
error.

I've now fixed this on the master branch, and now zero width values
will always be rejected.