bug#69920: Proposed fix - Toggling MIME inline attachment previews adds superfluous newlines

[Alcor]

Eli Zaretskii <eliz@gnu.org> writes:

> Are you saying that the problem is with the function that
> "un-displays" the inline image, in that it fails to remove the
> inserted newline?  (AFAIU, the code before the above commit also had
> the same issue.)  That wasn't clear from the description of the
> problem, and the Subject is ambiguous wrt what newlines are deemed
> "superfluous".  So please clarify what is the problem you are flagging
> here.

The problem is that the undisplayer does not correctly undo what the
displayer does.

The undisplayer (as it is right now) just removes the image. That can be
confirmed by stepping through the code with edebug or by inspecting the
"b" variable.

I think we can agree that (delete-region b (1+ b)) will always delete
_exactly one_ character, and in that case that would be the propertized
"x" with the image. That would leave a dangling newline.

There are two ways to solve this:

1. Do not add the extra newline (this is what the patch does).
2. Remove the extra newline via (delete-region b (+ b 2)) – note that I
have not tried this, but it would make sense to me.

I happen to prefer option #1 as the extra newline does not seem to have
any meaningful function. But this is just my own preference (Emacs/gnus
maintainers may wish to retain the extra newline if it serves a valid purpose).

If it helps clarify things, I'm okay with renaming the bug report to
something like "MIME inline image preview undisplayer does not clean up
displayed image correctly" or something along these lines. When I filed
the report, I wasn't sure about the cause so I described the user-facing
issue instead of the actual technical problem.

Cheers,
-A.

PS: I'm not sure the original code from
before 14ff920dc885636a763d6ab7f256cc9981c24781 was correct either. It
used to insert "x\n\n" (3 characters) on display (x being the
propertized image) and removed via (delete-region b (+ b 2)) exactly 2
characters. The new code after that revision inserted "x\n" (2
characters, x being the propertized image) and removed via
(delete-region b (1+ b)) exactly 1 character. So it might be possible
that this off-by-one error in `mm-inline-image' has always existed.