bug#23057: 25.0.92; shr wrongly adds two newlines to div element

bug#23057: 25.0.92; shr wrongly adds two newlines to div element

[Stephen Berman]

As a consequence of commit 9781dc4da35934839bf848b576829786962655b4
Author: Lars Ingebrigtsen <larsi@gnus.org>
Date:   Mon Feb 29 18:06:36 2016 +1100

    Make <div> in <li> not insert extra newlines
    
    * lisp/net/shr.el (shr-tag-div): Make <div> in <li> not insert
    extra newlines (bug#19587).

div elements that do not contain other block elements now get two
newlines added to them.  To reproduce, select the following bit of HTML
and apply shr-render-region to it:

<ul>
  <li>
    <div>
      <p >This is the first paragraph of a list item.
    </div>
    <p >This is the second paragraph of a list item.
  </li>
  <li>
    <div>This is the first paragraph of a list item.</div>
    <div>This is the second paragraph of a list item.</div>
  </li>
</ul>

This shows up e.g. in Gnus postings displayed as HTML (see
http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-03/msg00817.html)
and in eww (look e.g. at this page in eww:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-25&id=9781dc4da35934839bf848b576829786962655b4).
The following patch fixes this display problem for me, but I'm not
familiar enough either with shr or with all the details of HTML block
elements to be sure this is a sufficient or even correct fix.


diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index e943132..d9dcda3 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -812,6 +812,9 @@ shr-ensure-paragraph
 						    (line-end-position))
 		       (line-end-position)))))
 	(delete-region (match-beginning 0) (match-end 0)))
+       ((eq (dom-tag dom) 'div)
+	;; <div> contains no block element; do nothing.
+	)
        (t
 	(insert "\n\n"))))))


In GNU Emacs 25.0.92.4 (x86_64-suse-linux-gnu, GTK+ Version 3.14.15)
 of 2016-03-18 built on rosalinde
Repository revision: ed909c049e845a22a7beb626ac98f139388005fa
Windowing system distributor 'The X.Org Foundation', version 11.0.11601000
System Description:	openSUSE 13.2 (Harlequin) (x86_64)

Configured using:
 'configure --with-xwidgets 'CFLAGS=-Og -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GCONF GSETTINGS NOTIFY
GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 XWIDGETS

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

bug#23057: 25.0.92; shr wrongly adds two newlines to div element

[Stephen Berman]

On Fri, 18 Mar 2016 22:10:37 +0100 Stephen Berman <stephen.berman@gmx.net> wrote:

> The following patch fixes this display problem for me, but I'm not
> familiar enough either with shr or with all the details of HTML block
> elements to be sure this is a sufficient or even correct fix.
>
> diff --git a/lisp/net/shr.el b/lisp/net/shr.el
> index e943132..d9dcda3 100644
> --- a/lisp/net/shr.el
> +++ b/lisp/net/shr.el
> @@ -812,6 +812,9 @@ shr-ensure-paragraph
>  						    (line-end-position))
>  		       (line-end-position)))))
>  	(delete-region (match-beginning 0) (match-end 0)))
> +       ((eq (dom-tag dom) 'div)
> +	;; <div> contains no block element; do nothing.
> +	)
>         (t
>  	(insert "\n\n"))))))

Oops, I overlooked that `dom' is unbound here; it's dynamically bound
when called from shr-tag-div, but that may be a problem elsewhere.
Here's a cleaner alternative, but it's probably too ad hoc for a real
fix:

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index e943132..6350dfb 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -786,7 +786,7 @@ shr-ensure-newline
   (unless (zerop (current-column))
     (insert "\n")))
 
-(defun shr-ensure-paragraph ()
+(defun shr-ensure-paragraph (&optional dom)
   (unless (bobp)
     (let ((prefix (get-text-property (line-beginning-position)
 				     'shr-prefix-length)))
@@ -812,6 +812,10 @@ shr-ensure-paragraph
 						    (line-end-position))
 		       (line-end-position)))))
 	(delete-region (match-beginning 0) (match-end 0)))
+       ((and dom
+             (eq (dom-tag dom) 'div))
+	;; <div> contains no block element; do nothing.
+	)
        (t
 	(insert "\n\n"))))))
 
@@ -1206,7 +1210,7 @@ shr-tag-p
   (shr-ensure-paragraph))
 
 (defun shr-tag-div (dom)
-  (shr-ensure-paragraph)
+  (shr-ensure-paragraph dom)
   (shr-generic dom)
   (shr-ensure-newline))
 

bug#23057: 25.0.92; shr wrongly adds two newlines to div element

[Michael Heerdegen]

Hi,

Lars, could you please have a look if it is ok to install this?  This
fix should go to emacs25 (soon).


Thanks,

Michael.


> > The following patch fixes this display problem for me, but I'm not
> > familiar enough either with shr or with all the details of HTML block
> > elements to be sure this is a sufficient or even correct fix.
> >
> > diff --git a/lisp/net/shr.el b/lisp/net/shr.el
> > index e943132..d9dcda3 100644
> > --- a/lisp/net/shr.el
> > +++ b/lisp/net/shr.el
> > @@ -812,6 +812,9 @@ shr-ensure-paragraph
> >  						    (line-end-position))
> >  		       (line-end-position)))))
> >  	(delete-region (match-beginning 0) (match-end 0)))
> > +       ((eq (dom-tag dom) 'div)
> > +	;; <div> contains no block element; do nothing.
> > +	)
> >         (t
> >  	(insert "\n\n"))))))
>
> Oops, I overlooked that `dom' is unbound here; it's dynamically bound
> when called from shr-tag-div, but that may be a problem elsewhere.
> Here's a cleaner alternative, but it's probably too ad hoc for a real
> fix:
>
> diff --git a/lisp/net/shr.el b/lisp/net/shr.el
> index e943132..6350dfb 100644
> --- a/lisp/net/shr.el
> +++ b/lisp/net/shr.el
> @@ -786,7 +786,7 @@ shr-ensure-newline
>    (unless (zerop (current-column))
>      (insert "\n")))
>  
> -(defun shr-ensure-paragraph ()
> +(defun shr-ensure-paragraph (&optional dom)
>    (unless (bobp)
>      (let ((prefix (get-text-property (line-beginning-position)
>  				     'shr-prefix-length)))
> @@ -812,6 +812,10 @@ shr-ensure-paragraph
>  						    (line-end-position))
>  		       (line-end-position)))))
>  	(delete-region (match-beginning 0) (match-end 0)))
> +       ((and dom
> +             (eq (dom-tag dom) 'div))
> +	;; <div> contains no block element; do nothing.
> +	)
>         (t
>  	(insert "\n\n"))))))
>  
> @@ -1206,7 +1210,7 @@ shr-tag-p
>    (shr-ensure-paragraph))
>  
>  (defun shr-tag-div (dom)
> -  (shr-ensure-paragraph)
> +  (shr-ensure-paragraph dom)
>    (shr-generic dom)
>    (shr-ensure-newline))
>  

bug#23057: 25.0.92; shr wrongly adds two newlines to div element

[Lars Magne Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> Oops, I overlooked that `dom' is unbound here; it's dynamically bound
> when called from shr-tag-div, but that may be a problem elsewhere.
> Here's a cleaner alternative, but it's probably too ad hoc for a real
> fix:

Yes, it doesn't really seem like the way to do this.

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