bug#46552: 27.1; image-mode should not move current point

bug#46552: 27.1; image-mode should not move current point

[ynyaaa]

If an image which is a binary file is opened in image-mode,
the current point is moved from the beginning of the buffer to
elsewhere. And if the image display is toggled off and on again,
the current point is moved to the end of the buffer.
Then some operations, such as image-increase-size, cause an error.

Example:
C-x C-f /emacs-27.1-x86_64/x86_64/share/emacs/27.1/etc/images/splash.png RET
M-: (point) RET
  => 3 ;; check current point
C-c C-c ;; change to text display
M-: (point) RET
  => 1 :: check current point
C-c C-c ;; change to image display
M-: (point) RET
  => 25161 ;; check current point, value is EOB
+ ;; image-increase-size
  -| Error running timer ‘image--change-size’: (error "No image under point")
r ;; image-rotate
  -| No image under point


In GNU Emacs 27.1 (build 1, x86_64-w64-mingw32)
 of 2020-08-22 built on CIRROCUMULUS
Repository revision: 86d8d76aa36037184db0b2897c434cdaab1a9ae8
Repository branch: HEAD
Windowing system distributor 'Microsoft Corp.', version 10.0.18363
System Description: Microsoft Windows 10 Pro (v10.0.1909.18363.1379)

Recent messages:

Configured using:
 'configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2
HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: JPN
  locale-coding-system: cp932

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort emacsbug sendmail mail-extr message rmc puny dired
dired-loaddefs format-spec rfc822 mml mml-sec password-cache epa derived
epg epg-config gnus-util rmail rmail-loaddefs text-property-search
time-date subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader cl-seq
image-mode exif term/bobcat help-mode easymenu cl-loaddefs cl-lib
japan-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer 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 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 threads w32notify w32 lcms2 multi-tty make-network-process
emacs)

Memory information:
((conses 16 52863 8451)
 (symbols 48 6373 1)
 (strings 32 17684 1753)
 (string-bytes 1 552218)
 (vectors 16 11210)
 (vector-slots 8 213266 11242)
 (floats 8 33 39)
 (intervals 56 268 0)
 (buffers 1000 13))

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

ynyaaa@gmail.com writes:

> Example:
> C-x C-f /emacs-27.1-x86_64/x86_64/share/emacs/27.1/etc/images/splash.png RET
> M-: (point) RET
>   => 3 ;; check current point
> C-c C-c ;; change to text display
> M-: (point) RET
>   => 1 :: check current point
> C-c C-c ;; change to image display
> M-: (point) RET
>   => 25161 ;; check current point, value is EOB

This is most puzzling.  I can reproduce this problem in Emacs 28 without
a problem.  But:

C-c C-c runs the command image-toggle-display (found in
image-minor-mode-map), which is an interactive Lisp closure in
‘image-mode.el’.

If I instead say `M-: (image-toggle-display) RET' or even
`M-: (call-interactively 'image-toggle-display) RET', I can't.

And `C-c C-c' should be totally equivalent to the latter, right?

So what's moving point?  It must be some...  hook function or something
that reacts differently to the two things...  but looking through the
image-mode.el code, it's not obvious what that could be.

Anybody got any ideas here?

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

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: ynyaaa@gmail.com
> Date: Tue, 16 Feb 2021 18:21:43 +0900
> 
> 
> If an image which is a binary file is opened in image-mode,
> the current point is moved from the beginning of the buffer to
> elsewhere. And if the image display is toggled off and on again,
> the current point is moved to the end of the buffer.
> Then some operations, such as image-increase-size, cause an error.

The initial movement of point when visiting the image the first time
is a regression in Emacs 27 due to adding the Exif support: the code
which detects Exif moves point as side effect.  I've now fixed that on
the emacs-27 branch.

Once point moves into the image data, the second "C-c C-c" moves point
to EOB, because we don't let point end up inside a display property.
So this part is not a separate problem, but a side effect of the first
part.

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 16 Feb 2021 13:36:18 +0100
> Cc: 46552@debbugs.gnu.org
> 
> This is most puzzling.  I can reproduce this problem in Emacs 28 without
> a problem.  But:
> 
> C-c C-c runs the command image-toggle-display (found in
> image-minor-mode-map), which is an interactive Lisp closure in
> ‘image-mode.el’.
> 
> If I instead say `M-: (image-toggle-display) RET' or even
> `M-: (call-interactively 'image-toggle-display) RET', I can't.
> 
> And `C-c C-c' should be totally equivalent to the latter, right?
> 
> So what's moving point?

It's the "point-adjustment" feature, which moves point out of
invisible/intangible/display-property/etc text, where we don't want
the user to see point, ever.  Try setting
global-disable-point-adjustment non-nil, and you will see what happens
when this is disabled.

> It must be some...  hook function or something that reacts
> differently to the two things...  but looking through the
> image-mode.el code, it's not obvious what that could be.

It's not in image-mode.el, it's a general feature of Emacs not
directly related to images.

> Anybody got any ideas here?

The point adjustment needs to decide whether to move point before or
after the display property.  Its logic is based on heuristics that can
break/change behavior depending on how the command was invoked,
because it's based on the previous value of point (if point moved
forward, the logic prefers to adjust point in the forward direction,
and vice versa).

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> It's the "point-adjustment" feature, which moves point out of
> invisible/intangible/display-property/etc text, where we don't want
> the user to see point, ever.  Try setting
> global-disable-point-adjustment non-nil, and you will see what happens
> when this is disabled.

Ah!  Thanks; that fixes the problem.  I've now made `image-mode' set
`disable-point-adjustment' buffer-locally.

>> Anybody got any ideas here?
>
> The point adjustment needs to decide whether to move point before or
> after the display property.  Its logic is based on heuristics that can
> break/change behavior depending on how the command was invoked,
> because it's based on the previous value of point (if point moved
> forward, the logic prefers to adjust point in the forward direction,
> and vice versa).

Right.  It is pretty odd that `M-: (call-interactively
'image-toggle-display)' works differently here than `C-c C-c', but that
what happens with heuristics, I guess.

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

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ah!  Thanks; that fixes the problem.  I've now made `image-mode' set
> `disable-point-adjustment' buffer-locally.

Oh, I see you've already fixed this in Emacs 27 differently, so I've
reverted my patch.

(The Emacs mailing lists sure have been slow the past few days...)

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

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: ynyaaa@gmail.com,  46552@debbugs.gnu.org
> Date: Tue, 16 Feb 2021 17:40:52 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It's the "point-adjustment" feature, which moves point out of
> > invisible/intangible/display-property/etc text, where we don't want
> > the user to see point, ever.  Try setting
> > global-disable-point-adjustment non-nil, and you will see what happens
> > when this is disabled.
> 
> Ah!  Thanks; that fixes the problem.  I've now made `image-mode' set
> `disable-point-adjustment' buffer-locally.

Why? that's the wrong thing to do, I think.

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Ah!  Thanks; that fixes the problem.  I've now made `image-mode' set
>> `disable-point-adjustment' buffer-locally.
>
> Why? that's the wrong thing to do, I think.

I see you've fixed the exif point movement problem, but that's just one
particular instance of the general problem.

The user can move point into the image data, and then hit `C-c C-c',
which will then move point to the end of the image (making commands like
`+' no longer work).

So that doesn't seem like a complete fix.

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

bug#46552: 27.1; image-mode should not move current point

[Juri Linkov]

> Right.  It is pretty odd that `M-: (call-interactively
> 'image-toggle-display)' works differently here than `C-c C-c', but that
> what happens with heuristics, I guess.

`C-c C-c' as a function `image-toggle-display' moves point to the end of
the image buffer (this is the bug fixed by Eli).  But when invoked
via `M-: (image-toggle-display) RET', exiting the minibuffer restores the
previous window configuration where point was at the beginning of
the buffer, thus restoring the previous point position.

This is related to bug#45072 that is a request for an option to not
restore a previous window configuration after exiting the minibuffer.

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: ynyaaa@gmail.com,  46552@debbugs.gnu.org
> Date: Tue, 16 Feb 2021 18:09:17 +0100
> 
> The user can move point into the image data, and then hit `C-c C-c',
> which will then move point to the end of the image (making commands like
> `+' no longer work).

If the user moves point, there won't be any reason to be surprised
that commands which need point on the image don't work.  Right?

IOW, from the user's POV, the image is a single "cell" on display, so
any forward movement of point ends up _after_ the image, where the
commands like '+' aren't supposed to work.

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> If the user moves point, there won't be any reason to be surprised
> that commands which need point on the image don't work.  Right?
>
> IOW, from the user's POV, the image is a single "cell" on display, so
> any forward movement of point ends up _after_ the image, where the
> commands like '+' aren't supposed to work.

I think that sounds quite surprising.  There's nothing much to do in an
image-mode buffer than interact with the image, and this makes
interaction not work.

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

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: ynyaaa@gmail.com,  46552@debbugs.gnu.org
> Date: Tue, 16 Feb 2021 18:41:40 +0100
> 
> > IOW, from the user's POV, the image is a single "cell" on display, so
> > any forward movement of point ends up _after_ the image, where the
> > commands like '+' aren't supposed to work.
> 
> I think that sounds quite surprising.  There's nothing much to do in an
> image-mode buffer than interact with the image, and this makes
> interaction not work.

But so does "C-c C-c".  "If it hurts, don't do that."

More importantly, I think disabling the point adjustments will cause
very surprising behavior: it allows moving point many times (as many
as there are bytes in the image file) without any visible effect.
That's something we want very much to avoid, and it is the main reason
why we have this point-adjustment feature.  Disabling it is a
debugging tool, not something to show to users.

bug#46552: 27.1; image-mode should not move current point

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: ynyaaa@gmail.com,  46552@debbugs.gnu.org
> Date: Tue, 16 Feb 2021 18:09:17 +0100
> 
> I see you've fixed the exif point movement problem, but that's just one
> particular instance of the general problem.

Btw, I think on master we should fix exif-parse-buffer not to move
point.  I didn't want risky changes on the release branch, but on
master I think it would be okay, and a cleaner solution.

WDYT?

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Btw, I think on master we should fix exif-parse-buffer not to move
> point.  I didn't want risky changes on the release branch, but on
> master I think it would be okay, and a cleaner solution.
>
> WDYT?

Yes, good idea.

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

bug#46552: 27.1; image-mode should not move current point

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> More importantly, I think disabling the point adjustments will cause
> very surprising behavior: it allows moving point many times (as many
> as there are bytes in the image file) without any visible effect.

Oh, definitely.  I didn't realise that `disable-point-adjustment' was
the thing that controlled all that -- sorry for the confusion.

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