bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

[OGAWA Hirofumi]

Hi,

The following is the reproduce code of issue,

(let ((buf (get-buffer-create "test-case")))
  (display-buffer buf)
  (with-current-buffer buf
    (erase-buffer)
    (insert "1\n")
    (shr-insert-document
     '(html nil (body nil (a ((href . "http://example.org")) "example"))))
    (insert "\n")
    (insert "2\n")))

and expected result is

	1
	example
	2

But actual result is

	example
	2
	1

Like above example, `shr-insert-document' modify the point
of "test-case" buffer. (display-buffer is important to
reproduce. If commented out display-buffer line, the issue
disappear.)

With some debugging I noticed, the usage of `with-temp-buffer' of
`shr-string-pixel-width' modify the point. Simplified version of
`shr-string-pixel-width' to reproduce the issue is the following,

    (with-temp-buffer
      (save-window-excursion
	(set-window-buffer nil (current-buffer))))

In this conbination, `with-temp-buffer' seems to fail to restore
the point.

So the patch seems to save and restore the point correctly.

Thanks.

---

 lisp/net/shr.el |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff -puN lisp/net/shr.el~shr-fix-point-modification lisp/net/shr.el
--- emacs/lisp/net/shr.el~shr-fix-point-modification	2017-12-16 07:32:47.777230050 +0900
+++ emacs-hirofumi/lisp/net/shr.el	2017-12-16 07:35:48.083847035 +0900
@@ -591,9 +591,10 @@ size, and full-buffer size."
 (defun shr-string-pixel-width (string)
   (if (not shr-use-fonts)
       (length string)
-    (with-temp-buffer
-      (insert string)
-      (shr-pixel-column))))
+    (save-excursion
+      (with-temp-buffer
+        (insert string)
+        (shr-pixel-column)))))
 
 (defsubst shr--translate-insertion-chars ()
   ;; Remove soft hyphens.
_



In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.24)
 of 2017-12-12 built on devron
Repository revision: 786907238bcb86ab9e0e2e9ebcc91c52a6eb024c
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
System Description:	Debian GNU/Linux testing (buster)

Configured using:
 'configure --libexecdir=/usr/local/lib --with-x --with-x-toolkit=gtk3
 --without-xim --with-xpm --with-jpeg --with-tiff --with-gif --with-png
 --with-rsvg --with-dbus --with-wide-int --with-modules'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY ACL
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES JSON LCMS2

Important settings:
  value of $LANG: ja_JP.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

[Eli Zaretskii]

> From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Date: Sat, 16 Dec 2017 21:47:07 +0900
> 
> (let ((buf (get-buffer-create "test-case")))
>   (display-buffer buf)
>   (with-current-buffer buf
>     (erase-buffer)
>     (insert "1\n")
>     (shr-insert-document
>      '(html nil (body nil (a ((href . "http://example.org")) "example"))))
>     (insert "\n")
>     (insert "2\n")))
> 
> and expected result is
> 
> 	1
> 	example
> 	2
> 
> But actual result is
> 
> 	example
> 	2
> 	1
> 
> Like above example, `shr-insert-document' modify the point
> of "test-case" buffer. (display-buffer is important to
> reproduce. If commented out display-buffer line, the issue
> disappear.)
> 
> With some debugging I noticed, the usage of `with-temp-buffer' of
> `shr-string-pixel-width' modify the point. Simplified version of
> `shr-string-pixel-width' to reproduce the issue is the following,
> 
>     (with-temp-buffer
>       (save-window-excursion
> 	(set-window-buffer nil (current-buffer))))
> 
> In this conbination, `with-temp-buffer' seems to fail to restore
> the point.
> 
> So the patch seems to save and restore the point correctly.

Thanks.

I think save-excursion is too heavy in this case, so I installed a
change to only preserve point.

Please try the recent emacs-26 branch, it should now be fixed there.

bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

[OGAWA Hirofumi]

Eli Zaretskii <eliz@gnu.org> writes:

> I think save-excursion is too heavy in this case, so I installed a
> change to only preserve point.
>
> Please try the recent emacs-26 branch, it should now be fixed there.

Thanks. It works for me.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

[Eli Zaretskii]

> From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> Cc: 29734@debbugs.gnu.org
> Date: Sun, 17 Dec 2017 01:59:50 +0900
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think save-excursion is too heavy in this case, so I installed a
> > change to only preserve point.
> >
> > Please try the recent emacs-26 branch, it should now be fixed there.
> 
> Thanks. It works for me.

Thanks for testing, closing.

bug#29734: 27.0.50; shr-insert-document modify point of buffer in corner case

[Katsumi Yamaoka]

On Sat, 16 Dec 2017 19:18:48 +0200, Eli Zaretskii wrote:
> Thanks for testing, closing.

The function `shr-string-pixel-width' should return pixel-width,
not a position.  Fixed in emacs-26.