bug#67269: 30.0.50; italic face fallback does not assume italic properties

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

When the default font doesn't supply an Italic variant, the italic face
is assigned an underline propery in its place.  But when the italic face
is merged with another face whose font does supply such a variant, the
underline is applied all the same while the slant that would ideally be
also is disregarded.

Hence if the default font is set to Droid Sans Mono (with no Italic
variant) while shr-text is set to Times New Roman, italic text rendered
by shr:

  (shr-text italic)

is underlined rather than slanted.  Moreover, no underline or slant is
manifested if there is an Italic variant to the default font but not the
face being merged with italic, depriving such italic text of any visual
distinctiveness.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Eli Zaretskii]

tags 67269 notabug
thanks

> Date: Sun, 19 Nov 2023 14:01:04 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> When the default font doesn't supply an Italic variant, the italic face
> is assigned an underline propery in its place.  But when the italic face
> is merged with another face whose font does supply such a variant, the
> underline is applied all the same while the slant that would ideally be
> also is disregarded.
> 
> Hence if the default font is set to Droid Sans Mono (with no Italic
> variant) while shr-text is set to Times New Roman, italic text rendered
> by shr:
> 
>   (shr-text italic)
> 
> is underlined rather than slanted.  Moreover, no underline or slant is
> manifested if there is an Italic variant to the default font but not the
> face being merged with italic, depriving such italic text of any visual
> distinctiveness.

I see no bug here.  You are talking about faces defined with defface,
which are always based on the default font.  Given that fact, there's
not much we can do here, except perhaps keep the :slant attribute
together with :underline (although that's questionable, and I'm not
sure we should do that).  Removing the underline when a non-default
font does have the italic variant is not TRT, since it will cause
parts of the text appear with an underline and other parts without it.

Basically, choosing a default font that doesn't have the italic
variant is simply a bad idea, so what you describe is a cockpit error,
not an Emacs bug.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Basically, choosing a default font that doesn't have the italic
> variant is simply a bad idea, so what you describe is a cockpit error,
> not an Emacs bug.

OK, but half the fixed pitch fonts on the face of this earth have no
Italic variant.  Noto Sans Mono, for example, is the default fixed pitch
font on countless free systems, and there is no Italic variant of it.

So we have to take some action to resolve this problem.  What about
modifying eww and such to use a different face from italic that always
sets the :slant property?

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 67269@debbugs.gnu.org
> Date: Sun, 19 Nov 2023 15:05:24 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Basically, choosing a default font that doesn't have the italic
> > variant is simply a bad idea, so what you describe is a cockpit error,
> > not an Emacs bug.
> 
> OK, but half the fixed pitch fonts on the face of this earth have no
> Italic variant.  Noto Sans Mono, for example, is the default fixed pitch
> font on countless free systems, and there is no Italic variant of it.

Yes, choosing the default font for Emacs is not a trivial task.  Bold
and Italic are just one aspect of that; the other is support for
enough non-ASCII characters to avoid changing fonts too frequently in
multilingual text.  Which is why Emacs comes with hard-coded defaults
for the standard fonts.  People who customize their fontsets should
understand the issues and choose the default font wisely.

> What about modifying eww and such to use a different face from
> italic that always sets the :slant property?

Please show a patch, to make sure we are talking about the same thing.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Yes, choosing the default font for Emacs is not a trivial task.  Bold
> and Italic are just one aspect of that; the other is support for
> enough non-ASCII characters to avoid changing fonts too frequently in
> multilingual text.  Which is why Emacs comes with hard-coded defaults
> for the standard fonts.  People who customize their fontsets should
> understand the issues and choose the default font wisely.

On the systems I mentioned, the default font is either Droid Sans Mono
or Noto Sans Mono.  Emacs's fontset selects that font, since they are
tied to the "Monospace" font name it specifies.

> Please show a patch, to make sure we are talking about the same thing.

I don't have a patch at the ready, but see shr-tag-i in shr.el.
Thanks.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 67269@debbugs.gnu.org
> Date: Sun, 19 Nov 2023 20:37:34 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Yes, choosing the default font for Emacs is not a trivial task.  Bold
> > and Italic are just one aspect of that; the other is support for
> > enough non-ASCII characters to avoid changing fonts too frequently in
> > multilingual text.  Which is why Emacs comes with hard-coded defaults
> > for the standard fonts.  People who customize their fontsets should
> > understand the issues and choose the default font wisely.
> 
> On the systems I mentioned, the default font is either Droid Sans Mono
> or Noto Sans Mono.  Emacs's fontset selects that font, since they are
> tied to the "Monospace" font name it specifies.

Then maybe we should change the default fontset's definition on
Android (if those are the systems you have in mind).  We already do
that for w32, see w32-standard-fontset-spec as opposed to
standard-fontset-spec.

> > Please show a patch, to make sure we are talking about the same thing.
> 
> I don't have a patch at the ready, but see shr-tag-i in shr.el.

I don't understand how shr-tag-i is relevant.  I'm probably missing
something.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Then maybe we should change the default fontset's definition on
> Android (if those are the systems you have in mind).  We already do
> that for w32, see w32-standard-fontset-spec as opposed to
> standard-fontset-spec.

The systems I have in mind aren't only Android: Noto Sand Mono is the
default (or only) Monospace font on many GNU/Linux systems as well, and
I've also witnessed DejaVu Sans Mono being installed without an Italic
variant.

> I don't understand how shr-tag-i is relevant.  I'm probably missing
> something.

It combines a variable pitch face with italic, and so loses when the
italic face doesn't support slant properties, even if the variable pitch
one does.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 67269@debbugs.gnu.org
> Date: Mon, 20 Nov 2023 08:22:35 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Then maybe we should change the default fontset's definition on
> > Android (if those are the systems you have in mind).  We already do
> > that for w32, see w32-standard-fontset-spec as opposed to
> > standard-fontset-spec.
> 
> The systems I have in mind aren't only Android: Noto Sand Mono is the
> default (or only) Monospace font on many GNU/Linux systems as well, and
> I've also witnessed DejaVu Sans Mono being installed without an Italic
> variant.

If so, then it is strange that we haven't heard about this issue
before.

Maybe we should do something like the below, and leave the rest to end
users and downstream distros.

> > I don't understand how shr-tag-i is relevant.  I'm probably missing
> > something.
> 
> It combines a variable pitch face with italic, and so loses when the
> italic face doesn't support slant properties, even if the variable pitch
> one does.

Yes, you already said that in your original message.  I thought
shr-tag-i provides some additional insight.

diff --git a/lisp/faces.el b/lisp/faces.el
index 7eacc40..8eaabbf 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2440,7 +2440,10 @@ italic
   '((((supports :slant italic))
      :slant italic)
     (((supports :underline t))
-     :underline t)
+     ;; Include italic, even if it isn't supported by the default
+     ;; font, because this face could be merged with another face
+     ;; which uses font that does have an italic variant.
+     :underline t :slant italic)
     (t
      ;; Default to italic, even if it doesn't appear to be supported,
      ;; because in some cases the display engine will do its own
@@ -2457,7 +2460,9 @@ bold-italic
 (defface underline
   '((((supports :underline t))
      :underline t)
-    (((supports :weight bold))
+    ;; Include underline, for when this face is merged with another
+    ;; whose font does support underline.
+    (((supports :weight bold :underline t))
      :weight bold)
     (t :underline t))
   "Basic underlined face."

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> If so, then it is strange that we haven't heard about this issue
> before.
>
> Maybe we should do something like the below, and leave the rest to end
> users and downstream distros.
>
>> > I don't understand how shr-tag-i is relevant.  I'm probably missing
>> > something.
>> 
>> It combines a variable pitch face with italic, and so loses when the
>> italic face doesn't support slant properties, even if the variable pitch
>> one does.
>
> Yes, you already said that in your original message.  I thought
> shr-tag-i provides some additional insight.

Ah, I'm sorry for the misunderstanding then.

> diff --git a/lisp/faces.el b/lisp/faces.el
> index 7eacc40..8eaabbf 100644
> --- a/lisp/faces.el
> +++ b/lisp/faces.el
> @@ -2440,7 +2440,10 @@ italic
>    '((((supports :slant italic))
>       :slant italic)
>      (((supports :underline t))
> -     :underline t)
> +     ;; Include italic, even if it isn't supported by the default
> +     ;; font, because this face could be merged with another face
> +     ;; which uses font that does have an italic variant.
> +     :underline t :slant italic)
>      (t
>       ;; Default to italic, even if it doesn't appear to be supported,
>       ;; because in some cases the display engine will do its own
> @@ -2457,7 +2460,9 @@ bold-italic
>  (defface underline
>    '((((supports :underline t))
>       :underline t)
> -    (((supports :weight bold))
> +    ;; Include underline, for when this face is merged with another
> +    ;; whose font does support underline.
> +    (((supports :weight bold :underline t))
>       :weight bold)
>      (t :underline t))
>    "Basic underlined face."

That's alright by me, thanks.

bug#67269: 30.0.50; italic face fallback does not assume italic properties

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 67269@debbugs.gnu.org
> Date: Mon, 20 Nov 2023 20:00:36 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If so, then it is strange that we haven't heard about this issue
> > before.
> >
> > Maybe we should do something like the below, and leave the rest to end
> > users and downstream distros.
> >
> >> > I don't understand how shr-tag-i is relevant.  I'm probably missing
> >> > something.
> >> 
> >> It combines a variable pitch face with italic, and so loses when the
> >> italic face doesn't support slant properties, even if the variable pitch
> >> one does.
> >
> > Yes, you already said that in your original message.  I thought
> > shr-tag-i provides some additional insight.
> 
> Ah, I'm sorry for the misunderstanding then.
> 
> > diff --git a/lisp/faces.el b/lisp/faces.el
> > index 7eacc40..8eaabbf 100644
> > --- a/lisp/faces.el
> > +++ b/lisp/faces.el
> > @@ -2440,7 +2440,10 @@ italic
> >    '((((supports :slant italic))
> >       :slant italic)
> >      (((supports :underline t))
> > -     :underline t)
> > +     ;; Include italic, even if it isn't supported by the default
> > +     ;; font, because this face could be merged with another face
> > +     ;; which uses font that does have an italic variant.
> > +     :underline t :slant italic)
> >      (t
> >       ;; Default to italic, even if it doesn't appear to be supported,
> >       ;; because in some cases the display engine will do its own
> > @@ -2457,7 +2460,9 @@ bold-italic
> >  (defface underline
> >    '((((supports :underline t))
> >       :underline t)
> > -    (((supports :weight bold))
> > +    ;; Include underline, for when this face is merged with another
> > +    ;; whose font does support underline.
> > +    (((supports :weight bold :underline t))
> >       :weight bold)
> >      (t :underline t))
> >    "Basic underlined face."
> 
> That's alright by me, thanks.

Thanks, installed on master, and closing the bug.