bug#51556: 29.0.50; Poor contrast of Customize SVG icons with dark backgrounds

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> It is certainly the correct solution for all the sets of scalable icons
>> that I have reviewed.  Which SVG icons do you have in mind?  Could you
>> point me to them?
>
> Try splash.svg, as a trivial example (and forget that it's too large
> for an icon, this is just an example).

That's correct, this approach won't work for multi-color SVG's.  But see
Jim Porter's reply.

>> For the icons I know of, the best solution if you need to change the
>> color of this or that icon, is to either change the active defface to
>> use the correct color, or to introduce a new defface.  This is, not by
>> accident, the chosen solution also for icons on the web.
>
> We have a disconnect here, because I don't follow.  Are you talking
> only about SVG that use only the gray color for its lines?

I'm talking about what Jim Porter calls "Single-color SVGs".  This case
is applicable to all sets of free and scalable icons that I have
reviewed.  This includes the most popular and extensive icon sets, such
as fontawesome, octicons and material icons.

In all these cases, the SVG files may or may not come with a foreground
color, but that is an implementation accident: they are mainly intended
for use in a web browser, where the web browser will override the
foreground color with the foreground color specified by CSS.

In Emacs, we don't let faces override a foreground color specified in an
SVG file (that won't work in general, as is clear from the "splash.svg"
example), so if we want this functionality, we must take care to remove
the foreground color in the SVG file.

> I said "should allow", and you say "need".

Aha.  Thanks for clarifying.  Then your position does not seem to be all
that different from mine.  Perhaps it is only a difference in
priorities, or perhaps there will exist no difference in practice.

>> Furthermore, the patch I have already posted here is sufficient for the
>> purposes of this bug report.  I suggest we install it.
>
> On master?  I don't object.

Thank you, installed as commit 11702a6dd7.