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

[Eli Zaretskii <eliz@gnu.org>]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Wed, 08 Jul 2015 20:10:35 +0200
> 
> in eww, using shr-use-fonts -> t, for some lines, the last character is
> only partially visible.  Not a big deal, but distracting.

Is the partially-visible character always the last character on that
line, before the hard newline?  If so, I think I know where's the flaw
in the logic.  See below.

> --8<---------------cut here---------------start------------->8---
> (defun shr-insert-document (dom)
>   ...
>   (setq shr-content-cache nil)
>   (let ((start (point))
> 	(shr-start nil)
>         ...
> 	(shr-internal-width (or (and shr-width..3..)
> 				(if (not shr-use-fonts)
> 				    (- (window-width) 2)
> 				  (- (window-pixel-width) ; <---- here
> 				     (* (frame-fringe-width) 2))))))
>     (shr-descend dom)
>     (shr-fill-lines start (point))
>     (shr-remove-trailing-whitespace start (point))
>     (when shr-warning..1..)))
> --8<---------------cut here---------------end--------------->8---
> 
> AFAICT
> 
>   (- (window-pixel-width) (* (frame-fringe-width) 2))
> 
> is not the available width for text, it is a larger value including
> scroll bars etc.

Do you understand why the value of frame-fringe-width is multiplied by
2?

> When I change it to
> 
>   (window-body-width nil t)
> 
> this improves things.

I think that using window-body-width is indeed better here.  It will,
for example, account for display margins.

> --8<---------------cut here---------------start------------->8---
> (defun shr-vertical-motion (column)
>   (if (not shr-use-fonts)
>       (move-to-column column)
>     (unless (eolp)
>       (forward-char 1))
>     (vertical-motion (cons (/ column (frame-char-width)) 0)) ; <-- here
>     (unless (eolp)
>       (forward-char 1))))
> --8<---------------cut here---------------end--------------->8---
> 
> This function is used, among other places, to decide where to break
> lines in `shr-fill-line'.
> 
> Probably (/ column (frame-char-width)) can be too large if you are
> unlucky.

Sorry, I don't follow.  Can you elaborate on when this could happen?

> For testing I tried with this version:
> 
> --8<---------------cut here---------------start------------->8---
> (defun shr-vertical-motion (column)
>   (if (not shr-use-fonts)
>       (move-to-column column)
>     (unless (eolp)
>       (forward-char 1))
>     (end-of-visual-line)))
> --8<---------------cut here---------------end--------------->8---
> 
> This seems to fix this issue (together with the first change),

I don't see how this could be right, unless you only tested it with
text that is rendered using a single font.  move-to-column goes to the
Nth character starting from the left edge (forget tabs and
double-width CJK characters for a moment), so it will not DTRT when a
screen line displays characters of different size (as in with
different-size fonts).

The original code works in pixels (vertical-motion interprets the
column number as the number of pixels equivalent to that number of
frame's canonical characters), so it is not prone to this problem.

I believe the problem is that the code determines whether the line
should be wrapped based on this test:

    (shr-vertical-motion shr-internal-width)
    (when (looking-at " $")  <<<<<<<<<<<<<<<<<<<<<<<<<
      (delete-region (point) (line-end-position)))

IOW, it assumes that if the character at the goal pixel coordinate
immediately precedes the newline, the line doesn't need to be wrapped.
But that will fail if that last character is unusually wide.