bug#47705: [PATCH] EWW: Customize display of images

bug#47705: [PATCH] EWW: Customize display of images

[Ralph Schleicher]

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

EWW already supports displaying web pages with ALT texts instead of
images but this feature is not visible to the user.  Additionally,
images or ALT texts are always displayed on a line by itself.  This
may waste a lot of vertical space, e.g. when displaying the Common
Lisp HyperSpec.  The attached patch fixes these issues.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-EWW-Customize-display-of-images.patch --]
[-- Type: text/x-diff, Size: 4995 bytes --]

From 8105b1db4328f212608998bf71187ba196e2205b Mon Sep 17 00:00:00 2001
From: Ralph Schleicher <rs@ralph-schleicher.de>
Date: Sat, 10 Apr 2021 23:08:27 +0200
Subject: [PATCH] EWW: Customize display of images

* lisp/net/eww.el (eww-toggle-images): New function.
(eww-mode-map): Add key binding and menu entry.
* lisp/net/shr.el (shr-inhibit-images): Make it customizable.
(shr-image-newline): New customization variable.
(shr-tag-img, shr-insert): Use shr-image-newline.
* doc/misc/eww.texi (Basics): Document eww-toggle-images.
Fix index entries for shr-use-fonts and shr-use-colors.
(Advanced): Document shr-inhibit-images and shr-image-newline.
---
 doc/misc/eww.texi | 22 +++++++++++++++++++---
 lisp/net/eww.el   | 10 ++++++++++
 lisp/net/shr.el   | 15 +++++++++++----
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/doc/misc/eww.texi b/doc/misc/eww.texi
index 6e82a97030..0ec3b4bf6e 100644
--- a/doc/misc/eww.texi
+++ b/doc/misc/eww.texi
@@ -124,17 +124,25 @@ Basics
 only display this part.  This usually gets rid of menus and the like.
 
 @findex eww-toggle-fonts
-@findex shr-use-fonts
+@vindex shr-use-fonts
 @kindex F
   The @kbd{F} command (@code{eww-toggle-fonts}) toggles whether to use
 variable-pitch fonts or not.  This sets the @code{shr-use-fonts} variable.
 
 @findex eww-toggle-colors
-@findex shr-use-colors
-@kindex F
+@vindex shr-use-colors
+@kindex M-C
   The @kbd{M-C} command (@code{eww-toggle-colors}) toggles whether to use
 HTML-specified colors or not.  This sets the @code{shr-use-colors} variable.
 
+@findex eww-toggle-images
+@vindex shr-inhibit-images
+@kindex M-I
+@cindex Image Display
+  The @kbd{M-I} command (@code{eww-toggle-images}, capital letter i)
+toggles whether to display images or not.  This sets the
+@code{shr-inhibit-images} variable.
+
 @findex eww-download
 @vindex eww-download-directory
 @kindex d
@@ -305,6 +313,14 @@ Advanced
 support required) then larger images are scaled down.  You can block
 specific images completely by customizing @code{shr-blocked-images}.
 
+@vindex shr-inhibit-images
+@vindex shr-image-newline
+  You can control image display by customizing
+@code{shr-inhibit-images}.  If this variable is nil, always display
+the ALT text of images.  Images or ALT texts are displayed on a
+separate line by default.  You can preserve the normal text flow by
+setting the variable @code{shr-image-newline} to nil.
+
 @vindex shr-color-visible-distance-min
 @vindex shr-color-visible-luminance-min
 @cindex Contrast
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 32fe857e65..eec3ec7ba8 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -987,6 +987,7 @@ eww-mode-map
     (define-key map "F" 'eww-toggle-fonts)
     (define-key map "D" 'eww-toggle-paragraph-direction)
     (define-key map [(meta C)] 'eww-toggle-colors)
+    (define-key map [(meta I)] 'eww-toggle-images)
 
     (define-key map "b" 'eww-add-bookmark)
     (define-key map "B" 'eww-list-bookmarks)
@@ -1015,6 +1016,7 @@ eww-mode-map
 	["List cookies" url-cookie-list t]
 	["Toggle fonts" eww-toggle-fonts t]
 	["Toggle colors" eww-toggle-colors t]
+	["Toggle images" eww-toggle-images t]
         ["Character Encoding" eww-set-character-encoding]
         ["Toggle Paragraph Direction" eww-toggle-paragraph-direction]))
     map))
@@ -1893,6 +1895,14 @@ eww-toggle-colors
 	     "off"))
   (eww-reload))
 
+(defun eww-toggle-images ()
+  "Toggle whether or not to display images."
+  (interactive nil eww-mode)
+  (setq shr-inhibit-images (not shr-inhibit-images))
+  (eww-reload)
+  (message "Images are now %s"
+           (if shr-inhibit-images "off" "on")))
+
 ;;; Bookmarks code
 
 (defvar eww-bookmarks nil)
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index c122a19e90..4332a1ef15 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -183,8 +183,13 @@ shr-abbreviation
   "Face for <abbr> elements."
   :version "27.1")
 
-(defvar shr-inhibit-images nil
-  "If non-nil, inhibit loading images.")
+(defcustom shr-inhibit-images nil
+  "If non-nil, inhibit loading images."
+  :type 'boolean)
+
+(defcustom shr-image-newline t
+  "If non-nil, display images on a separate line."
+  :type 'boolean)
 
 (defvar shr-external-rendering-functions nil
   "Alist of tag/function pairs used to alter how shr renders certain tags.
@@ -664,7 +669,8 @@ shr--translate-insertion-chars
 
 (defun shr-insert (text)
   (when (and (not (bolp))
-	     (get-text-property (1- (point)) 'image-url))
+	     (get-text-property (1- (point)) 'image-url)
+             shr-image-newline)
     (insert "\n"))
   (cond
    ((eq shr-folding-mode 'none)
@@ -1654,7 +1660,8 @@ shr-tag-img
 	    (and dom
 		 (or (> (length (dom-attr dom 'src)) 0)
                      (> (length (dom-attr dom 'srcset)) 0))))
-    (when (> (current-column) 0)
+    (when (and (> (current-column) 0)
+               shr-image-newline)
       (insert "\n"))
     (let ((alt (dom-attr dom 'alt))
           (width (shr-string-number (dom-attr dom 'width)))
-- 
2.20.1


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]

-- 
Ralph

bug#47705: [PATCH] EWW: Customize display of images

[Lars Ingebrigtsen]

Ralph Schleicher <rs@ralph-schleicher.de> writes:

> EWW already supports displaying web pages with ALT texts instead of
> images but this feature is not visible to the user.

Thanks; applied to Emacs 28.

> Additionally, images or ALT texts are always displayed on a line by
> itself.  This may waste a lot of vertical space, e.g. when displaying
> the Common Lisp HyperSpec.  The attached patch fixes these issues.

But not this bit.  The image display in shr should be improved, though.
The problem is that mixing large inline in text often makes the text
less easy to read, and this is particularly a problem in tables, where
shr chooses to not include images at all.

shr should instead include images in-line if they are "small" -- i.e.,
if they've got a height that's somewhat comparable to the line height,
but not otherwise.

This sounds like trivial thing to do, but it's not -- we don't know the
size of the images until we load them (usually), and shr doesn't to
re-flowing at all.

shr could do inline images if the images have known width/heights (and
the images are "small"), though.  Patches welcome.

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

bug#47705: [PATCH] EWW: Customize display of images

[Ralph Schleicher]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> Additionally, images or ALT texts are always displayed on a line by
>> itself.  This may waste a lot of vertical space, e.g. when displaying
>> the Common Lisp HyperSpec.  The attached patch fixes these issues.
>
> But not this bit.
[...]
> shr should instead include images in-line if they are "small" -- i.e.,
> if they've got a height that's somewhat comparable to the line height,
> but not otherwise.
>
> This sounds like trivial thing to do, but it's not -- we don't know the
> size of the images until we load them (usually), and shr doesn't to
> re-flowing at all.
>
> shr could do inline images if the images have known width/heights (and
> the images are "small"), though.  Patches welcome.

I see your point.  Would it be acceptable if, as a first step, inlining
of images is combined with shr-inhibit-images non-nil?  Then we know
that the ALT texts fit into a line.

-- 
Ralph

bug#47705: [PATCH] EWW: Customize display of images

[Lars Ingebrigtsen]

Ralph Schleicher <rs@ralph-schleicher.de> writes:

> I see your point.  Would it be acceptable if, as a first step, inlining
> of images is combined with shr-inhibit-images non-nil?  Then we know
> that the ALT texts fit into a line.

I'm not sure I understand what you mean -- the size of the alt texts has
no relation to the size of the images.

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

bug#47705: [PATCH] EWW: Customize display of images

[Ralph Schleicher]

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

Hi Lars,

I have thought about it again and maybe we should approach the topic
from another side.  Images are inline elements and so, first of all,
there is no reason why the renderer should insert line breaks at all.
The decision whether an image shall be displayed as an inline element
or as a block element is actually made by the creator of the web page.
The renderer should only follow the instructions.  That means, if an
image is intermixed with text, or if multiple images are placed on a
single line, we should assume that there must be a reason and SHR
shouldn't try to be too smart.

After all, the user should be able to take the final decision.  Thus,
I've added an eww-toggle-inline-images command.  I checked the result
with, e.g. www.gnu.org, www.gnu.org/software/emacs, docs.gtk.org/gtk4,
and www.lispworks.com/documentation/HyperSpec/Front -- it looks good.

Maybe you reconsider adding the patch.  Thank you.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-eww-toggle-inline-images.patch --]
[-- Type: text/x-diff, Size: 3800 bytes --]

diff --git a/doc/misc/eww.texi b/doc/misc/eww.texi
index cc546a92d6..9500624859 100644
--- a/doc/misc/eww.texi
+++ b/doc/misc/eww.texi
@@ -143,6 +143,13 @@ Basics
 toggles whether to display images or not.  This also sets the
 @code{shr-inhibit-images} variable.
 
+@findex eww-toggle-inline-images
+@vindex shr-inline-images
+@cindex Image Display
+  The command @code{eww-toggle-inline-images} toggles whether to
+display images as inline elements or not.  This also sets the
+@code{shr-inline-images} variable.
+
 @findex eww-download
 @vindex eww-download-directory
 @kindex d
@@ -314,9 +321,12 @@ Advanced
 specific images completely by customizing @code{shr-blocked-images}.
 
 @vindex shr-inhibit-images
+@vindex shr-inline-images
   You can control image display by customizing
 @code{shr-inhibit-images}.  If this variable is @code{nil}, display
-the ``ALT'' text of images instead.
+the ``ALT'' text of images instead.  By definition, images are inline
+elements.  However, if the variable @code{shr-inline-images} is
+@code{nil}, images are displayed on a separate line.
 
 @vindex shr-color-visible-distance-min
 @vindex shr-color-visible-luminance-min
diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index eec3ec7ba8..e6046bf145 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -1017,6 +1017,7 @@ eww-mode-map
 	["Toggle fonts" eww-toggle-fonts t]
 	["Toggle colors" eww-toggle-colors t]
 	["Toggle images" eww-toggle-images t]
+	["Toggle inline images" eww-toggle-inline-images t]
         ["Character Encoding" eww-set-character-encoding]
         ["Toggle Paragraph Direction" eww-toggle-paragraph-direction]))
     map))
@@ -1903,6 +1904,14 @@ eww-toggle-images
   (message "Images are now %s"
            (if shr-inhibit-images "off" "on")))
 
+(defun eww-toggle-inline-images ()
+  "Toggle whether or not to display images as inline elements."
+  (interactive nil eww-mode)
+  (setq shr-inline-images (not shr-inline-images))
+  (eww-reload)
+  (message "Inline images are now %s"
+           (if shr-inline-images "on" "off")))
+
 ;;; Bookmarks code
 
 (defvar eww-bookmarks nil)
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index cbdeb65ba8..673c4787a3 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -188,6 +188,11 @@ shr-inhibit-images
   :version "28.1"
   :type 'boolean)
 
+(defcustom shr-inline-images t
+  "If non-nil, render images as inline elements."
+  :version "28.1"
+  :type 'boolean)
+
 (defvar shr-external-rendering-functions nil
   "Alist of tag/function pairs used to alter how shr renders certain tags.
 For instance, eww uses this to alter rendering of title, forms
@@ -672,7 +677,9 @@ shr--translate-insertion-chars
 
 (defun shr-insert (text)
   (when (and (not (bolp))
-	     (get-text-property (1- (point)) 'image-url))
+	     (get-text-property (1- (point)) 'image-url)
+             (not shr-inhibit-images)
+             (not shr-inline-images))
     (insert "\n"))
   (cond
    ((eq shr-folding-mode 'none)
@@ -1147,7 +1154,8 @@ shr-put-image
 	  ;; When inserting big-ish pictures, put them at the
 	  ;; beginning of the line.
 	  (when (and (> (current-column) 0)
-		     (> (car (image-size image t)) 400))
+		     (> (car (image-size image t)) 400)
+                     (not shr-inline-images))
 	    (insert "\n"))
 	  (if (eq size 'original)
 	      (insert-sliced-image image (or alt "*") nil 20 1)
@@ -1662,7 +1670,9 @@ shr-tag-img
 	    (and dom
 		 (or (> (length (dom-attr dom 'src)) 0)
                      (> (length (dom-attr dom 'srcset)) 0))))
-    (when (> (current-column) 0)
+    (when (and (> (current-column) 0)
+               (not shr-inhibit-images)
+               (not shr-inline-images))
       (insert "\n"))
     (let ((alt (dom-attr dom 'alt))
           (width (shr-string-number (dom-attr dom 'width)))

[-- Attachment #3: Type: text/plain, Size: 12 bytes --]


-- 
Ralph

bug#47705: [PATCH] EWW: Customize display of images

[Jose A. Ortega Ruiz]

On Tue, Apr 13 2021, Ralph Schleicher wrote:

> Hi Lars,
>
> I have thought about it again and maybe we should approach the topic
> from another side.  Images are inline elements and so, first of all,
> there is no reason why the renderer should insert line breaks at all.
> The decision whether an image shall be displayed as an inline element
> or as a block element is actually made by the creator of the web page.
> The renderer should only follow the instructions.  That means, if an
> image is intermixed with text, or if multiple images are placed on a
> single line, we should assume that there must be a reason and SHR
> shouldn't try to be too smart.
>
> After all, the user should be able to take the final decision.  Thus,
> I've added an eww-toggle-inline-images command.  I checked the result
> with, e.g. www.gnu.org, www.gnu.org/software/emacs, docs.gtk.org/gtk4,
> and www.lispworks.com/documentation/HyperSpec/Front -- it looks good.
>
> Maybe you reconsider adding the patch.  Thank you.

I just wanted to echo my approval of the thoughts above, based on my
experience reading pages with embedded mathematical formulae... those
are very common in math and physics blogs, and consist of many little
inline images, and the current eww ways make them look worse than, say,
emacs-w3m.  For a random example of what i mean, just take any page from
Terry Tao's blog: https://terrytao.wordpress.com/.

Cheers,
jao
-- 
In this age, the mere example of nonconformity, the mere refusal to bend
the knee to custom, is itself a service.
 -John Stuart Mill, philosopher and economist (1806-1873)

bug#47705: [PATCH] EWW: Customize display of images

[Lars Ingebrigtsen]

Ralph Schleicher <rs@ralph-schleicher.de> writes:

> The decision whether an image shall be displayed as an inline element
> or as a block element is actually made by the creator of the web page.
> The renderer should only follow the instructions.

You're framing this as if it's a moral imperative, and I reject the
implication.  Anybody is free to render data in however way they
prefer -- it's an essential freedom.

As a practical matter, I've already explained why the patch isn't
appropriate for shr.  shr's handling of images should be improved, but
your proposed patch would make more things unreadable than it would
help, so it's not going to be applied.

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

bug#47705: [PATCH] EWW: Customize display of images

[Ralph Schleicher]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> You're framing this as if it's a moral imperative, and I reject the
> implication.  Anybody is free to render data in however way they
> prefer -- it's an essential freedom.

Fine, I accept your decision.

-- 
Ralph