bug#29047: 26.0.90; shr hang (regression from 25.3)

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Mike Kupfer]

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

Displaying the attached email with MH-E (using "emacs -Q") or Gnus leads
to a hang in shr (CPU is pegged, I waited for 5 minutes before giving
up).  The HTML looks pretty gnarly, but Emacs 25.3 displays the message
just fine, with maybe a pause of a couple seconds.

In GNU Emacs 26.0.90 (build 2, x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars)
 of 2017-10-27 built on alto
Repository revision: 50f711e7fab149d11da8c5caa61ee0b5e6c89bf1
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.9 (jessie)

Recent messages:
Auto-saving...done
Scanning +inbox...done
Quit
Mark set
Mark saved where search started
Mark set
No more undeleted messages
Processing deletes and refiles for +inbox...done
Auto-saving...done
Making completion list...

Configured using:
 'configure --prefix=/usr/new'

Configured features:
XPM JPEG TIFF GIF PNG SOUND NOTIFY GNUTLS LIBXML2 FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11

Important settings:
  value of $LC_TIME: C
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: MH-Folder

Minor modes in effect:
  diff-auto-refine-mode: t
  hl-line-mode: t
  shell-dirtrack-mode: t
  delete-selection-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug mh-funcs gnus-draft mh-thread nnir dired-aux diff-mode
gnus-mh mh-identity mh-letter mh-comp gnus-html cus-theme eieio-custom
wid-browse tree-widget help-fns radix-tree cus-edit cus-start cus-load
flow-fill grep compile dired-x mh-alias crm cl-print debug mh-mime
mh-gnus diary-lib diary-loaddefs cal-move info pp shr-color sort
mail-extr gnus-async gnus-bcklg misearch multi-isearch qp gnus-kill
cursor-sensor mhtml-mode css-mode smie color eww url-queue shr svg
browse-url js json map sgml-mode dom flyspell ispell gnus-ml disp-table
mm-archive url-http url-gw url-cache url-auth url-handlers nnrss xml
mm-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util url-parse url-vars nndoc nndraft nnmh
network-stream nsm starttls gnus-agent gnus-srvr gnus-score score-mode
nnvirtual gnus-msg nntp gnus-cache mh-show goto-addr thingatpt gnus-cite
gnus-art mm-uu mml2015 mm-view mml-smime smime dig mailcap gnus-sum
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source tls
gnutls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-win gnus-range
gnus nnheader wid-edit mh-inc hl-line mh-tool-bar mh-seq mh-xface
mh-utils mdk-mail smtpmail auth-source eieio eieio-core cl-macs
eieio-loaddefs sendmail message rmc puny seq byte-opt gv bytecomp
byte-compile cconv rfc822 mml mml-sec password-cache epa derived epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 gmm-utils
mailheader mh-folder which-func imenu mh-scan mh-e mh-compat mailabbrev
mh-buffers mh-loaddefs vc-hg cl-extra help-mode org-rmail org-mhe
org-irc org-info org-gnus gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils org-docview doc-view jka-compr
image-mode dired dired-loaddefs org-bibtex bibtex org-bbdb org-w3m
org-element cl-seq avl-tree generator org org-macro org-footnote
org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp
ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp ob-comint
ob-core ob-eval org-compat org-macs org-loaddefs format-spec find-func
cal-menu calendar cal-loaddefs elec-pair server noutline outline
easy-mmode cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs shell pcomplete comint ansi-color ring xcscope
easymenu advice delsel vc cl-loaddefs cl-lib vc-dispatcher timeclock
mdk-hacks time-date mule-util tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote inotify dynamic-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 618925 173854)
 (symbols 48 47553 20)
 (miscs 40 894 1526)
 (strings 32 131114 29559)
 (string-bytes 1 6012527)
 (vectors 16 55934)
 (vector-slots 8 1841037 152694)
 (floats 8 559 704)
 (intervals 56 102368 2278)
 (buffers 992 123)
 (heap 1024 79393 50480))

[-- Attachment #2: message that causes the hang --]
[-- Type: message/rfc822, Size: 66315 bytes --]

[-- Attachment #2.1.1: Type: text/html, Size: 63242 bytes --]

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Eli Zaretskii]

> From: Mike Kupfer <mkupfer@alum.berkeley.edu>
> Date: Sat, 28 Oct 2017 16:58:20 -0700
> 
> Displaying the attached email with MH-E (using "emacs -Q") or Gnus leads
> to a hang in shr (CPU is pegged, I waited for 5 minutes before giving
> up).  The HTML looks pretty gnarly, but Emacs 25.3 displays the message
> just fine, with maybe a pause of a couple seconds.

(Sending such a message as an inline HTML attachment to people most of
whom can be assumed to use Emacs to read email, which will use shr, is
not very friendly, okay?)

Thanks, this bug was introduced by 9eb028f, and the problematic
property is 'display' with value '(space :align-to (NNN))'.  It
causes shr-fill-line to infloop when this property is put on a
newline.

I can prevent the loop with the patch below.  I'm not sure this is the
right fix, though.  Comments?

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index c90d71d..067a3b9 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -704,8 +704,11 @@ shr-fill-line
 	    (gap-start (point)))
 	(insert "\n")
 	(shr-indent)
-	(when props
-	  (add-text-properties gap-start (point) props)))
+	(while props
+	  (let ((type (pop props))
+		(value (pop props)))
+	    (unless (eq type 'display)
+	      (put-text-property gap-start (point) type value)))))
       (setq start (point))
       (shr-vertical-motion shr-internal-width)
       (when (looking-at " $")

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Mike Kupfer]

Eli Zaretskii wrote:

> (Sending such a message as an inline HTML attachment to people most of
> whom can be assumed to use Emacs to read email, which will use shr, is
> not very friendly, okay?)

Yes, sorry, after sending it I realized I should have switched it to
non-inline.  Ah, I see I can make non-inline the default by tweaking
mml-content-disposition-alist.  Anyway, my apologies.

mike

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Katsumi Yamaoka]

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

On Sun, 29 Oct 2017 16:07:34 +0200, Eli Zaretskii wrote:
> Thanks, this bug was introduced by 9eb028f, and the problematic
> property is 'display' with value '(space :align-to (NNN))'.  It
> causes shr-fill-line to infloop when this property is put on a
> newline.

Sorry, I did the 9eb028f change for fixing bug#24034:
<https://lists.gnu.org/archive/html/bug-gnu-emacs/2016-07/msg01042.html>

> I can prevent the loop with the patch below.  I'm not sure this is the
> right fix, though.  Comments?

That's ok, but the properties necessary to copy there are only
`face' and `image-displayer'.  How about this one?

* lisp/net/shr.el (shr-fill-line): Copy only face and image-displayer
properties to gaps (bug#29047).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 821 bytes --]

--- shr.el~	2017-10-29 22:05:08.139411500 +0000
+++ shr.el	2017-10-30 06:19:48.874402200 +0000
@@ -700,12 +700,16 @@
       ;; Success; continue.
       (when (= (preceding-char) ?\s)
 	(delete-char -1))
-      (let ((props (text-properties-at (point)))
+      (let ((props `(face ,(get-text-property (point) 'face)
+			  ;; Don't break the image-displayer property
+			  ;; as it will cause `gnus-article-show-images'
+			  ;; to show the two or more same images.
+			  image-displayer
+			  ,(get-text-property (point) 'image-displayer)))
 	    (gap-start (point)))
 	(insert "\n")
 	(shr-indent)
-	(when props
-	  (add-text-properties gap-start (point) props)))
+	(add-text-properties gap-start (point) props))
       (setq start (point))
       (shr-vertical-motion shr-internal-width)
       (when (looking-at " $")

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Eli Zaretskii]

> Date: Mon, 30 Oct 2017 15:21:17 +0900
> From: Katsumi Yamaoka <yamaoka@jpl.org>
> Cc: Mike Kupfer <mkupfer@alum.berkeley.edu>,
>  Lars Ingebrigtsen <larsi@gnus.org>, 29047@debbugs.gnu.org
> 
> Sorry, I did the 9eb028f change for fixing bug#24034:
> <https://lists.gnu.org/archive/html/bug-gnu-emacs/2016-07/msg01042.html>
> 
> > I can prevent the loop with the patch below.  I'm not sure this is the
> > right fix, though.  Comments?
> 
> That's ok, but the properties necessary to copy there are only
> `face' and `image-displayer'.  How about this one?
> 
> * lisp/net/shr.el (shr-fill-line): Copy only face and image-displayer
> properties to gaps (bug#29047).

Works for me, thanks.  Please push to the emacs-26 branch.

bug#29047: 26.0.90; shr hang (regression from 25.3)

[Katsumi Yamaoka]

On Mon, 30 Oct 2017 20:54:57 +0200, Eli Zaretskii wrote:
>> Date: Mon, 30 Oct 2017 15:21:17 +0900
>> From: Katsumi Yamaoka <yamaoka@jpl.org>
[...]
>> That's ok, but the properties necessary to copy there are only
>> `face' and `image-displayer'.  How about this one?
[...]
> Works for me, thanks.  Please push to the emacs-26 branch.

Thanks.  Although, strictly speaking, not only `image-displayer'
but also `image-url' is essential to display an image,
'gnus-article-show-images' sees it in only the start position of
the `image-displayer' property, not gaps.  So, I've installed
the patch I posted last as is in the emacs-26 branch.

Regards,