bug#49433: 28.0.50; Faulty list of faces in shr-tag-h1

bug#49433: 28.0.50; Faulty list of faces in shr-tag-h1

[Protesilaos Stavrou]

In 'shr.el' we find these:

    (defun shr-tag-h1 (dom)
      (shr-heading dom (if shr-use-fonts
                   '(variable-pitch (:height 1.3 :weight bold))
                 'bold)))

    (defun shr-tag-h2 (dom)
      (shr-heading dom 'bold))

The intent is to make h1 headings use 'variable-pitch', however this
happens regardless, as can be noticed in other headings, such as h2 for
rendered documents.  As such, 'shr-tag-h1' creates duplication of the
'variable-pitch' entry.

Steps to reproduce on emacs -Q:

+ M-x eww RET https://www.gnu.org/software/emacs/

+ Go to the h1 heading that reads "GNU Emacs".

+ M-x describe-char

+ In the resulting Help buffer, notice this:

    (variable-pitch (variable-pitch (:height 1.3 :weight bold)))

+ The button that references that anonymous face does not provide a Help
  buffer with accurate information as to what attributes are in effect.
  Instead, it only lists the attributes of 'variable-pitch'.

+ Compare that to the information we get from the next heading, "An
  extensible, customizable, free/libre text editor — and more.", which
  accurately shows both of the faces that are in effect.

To fix the issue, we need to pass a face directly, without combining it
with 'variable-pitch'.  Just like 'shr-tag-h2'.  In a separate report, I
provide a patch that does that and adds new faces for all heading
elements (using 'defface' instead of hardcoding values).

Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com

bug#49433: 28.0.50; [PATCH] Add faces for shr heading elements (bug#49433)

[Protesilaos Stavrou]

[-- Attachment #1: Type: text/plain, Size: 375 bytes --]

In bug#49433 I noted how shr-tag-h1 creates a faulty anonymous face that
users cannot inspect properly.  The attached patch fixes this issue and
also provides named faces for all shr heading levels from 1-6.

The attributes of those new faces are the same as before for
backward-compatibility in terms of visuals.

Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-faces-for-shr-heading-elements-bug-49433.patch --]
[-- Type: text/x-patch, Size: 2751 bytes --]

From c8b70c1a020e2352379368fd9c2a2647bf88c4d8 Mon Sep 17 00:00:00 2001
Message-Id: <c8b70c1a020e2352379368fd9c2a2647bf88c4d8.1625557498.git.info@protesilaos.com>
From: Protesilaos Stavrou <info@protesilaos.com>
Date: Tue, 6 Jul 2021 10:44:46 +0300
Subject: [PATCH] Add faces for shr heading elements (bug#49433)

* etc/NEWS: Document new faces.
* lisp/net/shr.el (shr-h1, shr-h2, shr-h3, shr-h4, shr-h5, shr-h6):
Define new faces.
(shr-tag-h1): Remove inclusion of 'variable-pitch' face.  Fix
bug#49433 by applying a new face directly.
(shr-tag-h2, shr-tag-h3, shr-tag-h4, shr-tag-h5, shr-tag-h6): Apply
new faces.
---
 etc/NEWS        |  4 ++++
 lisp/net/shr.el | 41 +++++++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c3eaf5fcbb..b1ab6941d0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1620,6 +1620,10 @@ t, which preserves the original behavior.
 If set non-nil, showing an unseen message will set the Rmail buffer's
 modified flag.
 
+---
+*** New faces for heading elements.
+Those are 'shr-h1', 'shr-h2', 'shr-h3', 'shr-h4', 'shr-h5', 'shr-h6'.
+
 ** Apropos
 
 *** New commands 'apropos-next-symbol' and 'apropos-previous-symbol'.
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 873f0457e3..85d81b6bbc 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -183,6 +183,33 @@ (defface shr-abbreviation
   "Face for <abbr> elements."
   :version "27.1")
 
+(defface shr-h1
+  '((t :height 1.3 :weight bold))
+  "Face for <h1> elements."
+  :version "28.1")
+
+(defface shr-h2
+  '((t :weight bold))
+  "Face for <h2> elements."
+  :version "28.1")
+
+(defface shr-h3
+  '((t :slant italic))
+  "Face for <h3> elements."
+  :version "28.1")
+
+(defface shr-h4 nil
+  "Face for <h4> elements."
+  :version "28.1")
+
+(defface shr-h5 nil
+  "Face for <h5> elements."
+  :version "28.1")
+
+(defface shr-h6 nil
+  "Face for <h6> elements."
+  :version "28.1")
+
 (defcustom shr-inhibit-images nil
   "If non-nil, inhibit loading images."
   :version "28.1"
@@ -1939,24 +1966,22 @@ (defun shr-tag-span (dom)
   (shr-generic dom))
 
 (defun shr-tag-h1 (dom)
-  (shr-heading dom (if shr-use-fonts
-		       '(variable-pitch (:height 1.3 :weight bold))
-		     'bold)))
+  (shr-heading dom 'shr-h1))
 
 (defun shr-tag-h2 (dom)
-  (shr-heading dom 'bold))
+  (shr-heading dom 'shr-h2))
 
 (defun shr-tag-h3 (dom)
-  (shr-heading dom 'italic))
+  (shr-heading dom 'shr-h3))
 
 (defun shr-tag-h4 (dom)
-  (shr-heading dom))
+  (shr-heading dom 'shr-h4))
 
 (defun shr-tag-h5 (dom)
-  (shr-heading dom))
+  (shr-heading dom 'shr-h5))
 
 (defun shr-tag-h6 (dom)
-  (shr-heading dom))
+  (shr-heading dom 'shr-h6))
 
 (defun shr-tag-hr (_dom)
   (shr-ensure-newline)
-- 
2.32.0

bug#49433: 28.0.50; Faulty list of faces in shr-tag-h1

[Lars Ingebrigtsen]

Protesilaos Stavrou <info@protesilaos.com> writes:

> In bug#49433 I noted how shr-tag-h1 creates a faulty anonymous face that
> users cannot inspect properly.  The attached patch fixes this issue and
> also provides named faces for all shr heading levels from 1-6.
>
> The attributes of those new faces are the same as before for
> backward-compatibility in terms of visuals.

Thanks; applied to Emacs 28.

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