bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 09 Jul 2015 05:38:29 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 21012@debbugs.gnu.org
> 
> > I think you looked at the wrong `if' branch...?  What I changed was to
> > use `end-of-visual-line', which I hope is more accurate than
> > 
> >   (vertical-motion (cons (/ column (frame-char-width)) 0))
> > 
> > for finding the right point for breaking the line.
> 
> Maybe, it sounds like I need to take a better look.

I did that now.  There are a few subtle issues with this, as described
below.  Please try the patch at the end of this message, and see if it
gives good results.  You didn't provide a reproducible test case for
the screenshots you show, so I used Yamaoka-san's recipe; the patch
below fixes all the issues I saw with that recipe.

First, regarding your suggestion above to use end-of-visual-line: this
is only TRT when shr-width is nil, i.e. shr is filling text to the
full window width.  But shr-vertical-motion should also support the
case when shr-width is some specific number of columns or pixels, in
which case end-of-visual-line will put us at the wrong position.

Now to the issues I found.

First, the value of shr-internal-width was computed incorrectly: it
used window-width, which, as you discovered, is more than the text
area width, and it subtracted 2 columns from that value, whereas it
really needs to subtract only 1 (since column numbers are zero-based).

The 2nd issue was in shr-find-fill-point, where it deals with CJK
characters using the kinsoku feature: (a) it preferred to look for a
breakable point forward even when shr-width is nil, which produces
continuation lines; and (b) it mistakenly allowed to break the line
before a character that is forbidden by kinsoku to be at BOL.

Yet another subtlety is only visible when you disable one of the
fringes (or both): the calculation of shr-internal-width should in
that case subtract one more column, to be reserved for the
continuation glyph.  The patch below fixes that as well, albeit
slightly crudely (we sometimes lose a column); suggestions for how to
do that better are welcome.

Here's a patch that attempts at fixing all but the last of these
issues; if it gives good results, I will install it.  If not, please
show a test case that reproduces whatever problems are left.

Thanks.

diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 0ce77b9..f440abf 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -222,10 +222,29 @@ (defun shr-insert-document (dom)
 				     (if (not shr-use-fonts)
 					 shr-width
 				       (* shr-width (frame-char-width))))
+                                ;; We need to adjust the available
+                                ;; width for when the user disables
+                                ;; the fringes, which will cause the
+                                ;; display engine usurp one column for
+                                ;; the continuation glyph.
 				(if (not shr-use-fonts)
-				    (- (window-width) 2)
-				  (- (window-pixel-width)
-				     (* (frame-fringe-width) 2))))))
+				    (- (window-body-width) 1
+                                       (if (and (null shr-width)
+                                                (or (zerop
+                                                     (fringe-columns 'right))
+                                                    (zerop
+                                                     (fringe-columns 'left))))
+                                           0
+                                         1))
+				  (- (window-body-width nil t)
+                                     (frame-char-width)
+                                     (if (and (null shr-width)
+                                              (or (zerop
+                                                   (fringe-columns 'right))
+                                                  (zerop
+                                                   (fringe-columns 'left))))
+                                         (* (frame-char-width) 2)
+                                       0))))))
     (shr-descend dom)
     (shr-fill-lines start (point))
     (shr-remove-trailing-whitespace start (point))
@@ -439,8 +458,18 @@ (defun shr-fill-text (text)
     (with-temp-buffer
       (let ((shr-indentation 0)
 	    (shr-start nil)
-	    (shr-internal-width (- (window-pixel-width)
-				   (* (frame-fringe-width) 2))))
+	    (shr-internal-width (- (window-body-width nil t)
+                                   (frame-char-width)
+                                   ;; Adjust the window width for when
+                                   ;; the user disables the fringes,
+                                   ;; which causes the display engine
+                                   ;; usurp one coplumn for the
+                                   ;; continuation glyph.
+                                   (if (and (null shr-width)
+                                            (or (zerop (fringe-columns 'right))
+                                                (zerop (fringe-columns 'left))))
+                                       (* (frame-char-width) 2)
+                                     0))))
 	(shr-insert text)
 	(buffer-string)))))
 
@@ -620,7 +649,9 @@ (defun shr-find-fill-point (start)
 	;; There's no breakable point, so we give it up.
 	(let (found)
 	  (goto-char bp)
-	  (unless shr-kinsoku-shorten
+          ;; Don't overflow the window edge, even if
+          ;; shr-kinsoku-shorten is nil.
+	  (unless (or shr-kinsoku-shorten (null shr-width))
 	    (while (setq found (re-search-forward
 				"\\(\\c>\\)\\| \\|\\c<\\|\\c|"
 				(line-end-position) 'move)))
@@ -632,9 +663,12 @@ (defun shr-find-fill-point (start)
        ;; Don't put kinsoku-bol characters at the beginning of a line,
        ;; or kinsoku-eol characters at the end of a line.
        (cond
-	(shr-kinsoku-shorten
+        ;; Don't overflow the window edge, even if shr-kinsoku-shorten
+        ;; is nil.
+	((or shr-kinsoku-shorten (null shr-width))
 	 (while (and (not (memq (preceding-char) (list ?\C-@ ?\n ? )))
-		     (shr-char-kinsoku-eol-p (preceding-char)))
+		     (or (shr-char-kinsoku-eol-p (preceding-char))
+                         (shr-char-kinsoku-bol-p (following-char))))
 	   (backward-char 1))
 	 (when (setq failed (<= (point) start))
 	   ;; There's no breakable point that doesn't violate kinsoku,