Hiding hidden image

Hiding hidden image

[Jon Fineman]

This occurs using either:

notmuch version 0.37 and emacs version 29.1 on Debian trixie/sid X11/XFCE

or

OpenBSD OpenBSD 7.3-current (GENERIC.MP) #1327 X11/awsomewm.

I received an email with a hidden image/png. When I click on it, the
image displays fine. However when I click on it again to hide it the
email body text following it moves up as expected, but the picture
doesn't hide (go away). So I am left with the image overlaying the
email text. When I hide it the status of the attach line does change
to (hidden).

In past emacs/notmuch releases the picture would expand/shrink and the
email would adjust.

What else might I do to debug this?

Jon

Re: Hiding hidden image

[David Bremner]

Jon Fineman <jon@fineman.me> writes:

> This occurs using either:
>
> notmuch version 0.37 and emacs version 29.1 on Debian trixie/sid X11/XFCE
>
> or
>
> OpenBSD OpenBSD 7.3-current (GENERIC.MP) #1327 X11/awsomewm.
>
> I received an email with a hidden image/png. When I click on it, the
> image displays fine. However when I click on it again to hide it the
> email body text following it moves up as expected, but the picture
> doesn't hide (go away). So I am left with the image overlaying the
> email text. When I hide it the status of the attach line does change
> to (hidden).

Hi Jon;

Thanks for the report. I suspect this has to do with the new overlay
handling code in 29.1, but I haven't managed to isolate what exactly is
going on yet. I did verify that the problem does not occur with Emacs
28.2 on Debian testing. I


I also noticed that it does not happen for all png image parts, but the
pattern of what works and what does not is also unclear to me so far. I
observed that the following function

(defun hideit ()
  (interactive)
  (overlay-put (car (last (overlays-at (point)))) 'invisible t))

exhibits the same behaviour as toggling the button: i.e. one works iff
the other does. The fact that the overlay in question is the last one is
a I think a notmuch specific hack.

I don't have much to suggest, but I guess a start would be to identify
some distinguishing characteristics for the messages where the part
visibility fails to toggle. The problem does seem to manifest in more
cases than not. 

d

Re: Hiding hidden image

[David Bremner]

David Bremner <david@tethera.net> writes:

> I also noticed that it does not happen for all png image parts, but the
> pattern of what works and what does not is also unclear to me so far. I
> observed that the following function
>
> (defun hideit ()
>   (interactive)
>   (overlay-put (car (last (overlays-at (point)))) 'invisible t))

I think I'm missing something about overlays. The following code
does not work (i.e. hide the image) unless I extend the overlay to the
left.

(let ((buf (get-buffer-create "image-buffer"))
      (img (find-image '((:type xpm :file "attach.xpm"))))
      (overlay nil))
  (switch-to-buffer buf)
  (insert "0123456789")
  (insert-image img "x")
  (insert "0123456789")
  (insert "\n")
  (setq overlay (make-overlay 11 12))
  (overlay-put overlay 'invisible t)
  (message "props=%s" (overlay-properties overlay)))

This behaviour is the same in Emacs 28.2, unlike the problem Jon
describes with notmuch-emacs.

Re: Hiding hidden image

[David Bremner]

David Bremner <david@tethera.net> writes:

> David Bremner <david@tethera.net> writes:
>
>> I also noticed that it does not happen for all png image parts, but the
>> pattern of what works and what does not is also unclear to me so far. I
>> observed that the following function
>>
>> (defun hideit ()
>>   (interactive)
>>   (overlay-put (car (last (overlays-at (point)))) 'invisible t))
>
> I think I'm missing something about overlays. The following code
> does not work (i.e. hide the image) unless I extend the overlay to the
> left.
>
> (let ((buf (get-buffer-create "image-buffer"))
>       (img (find-image '((:type xpm :file "attach.xpm"))))
>       (overlay nil))
>   (switch-to-buffer buf)
>   (insert "0123456789")
>   (insert-image img "x")
>   (insert "0123456789")
>   (insert "\n")
>   (setq overlay (make-overlay 11 12))
>   (overlay-put overlay 'invisible t)
>   (message "props=%s" (overlay-properties overlay)))
>
> This behaviour is the same in Emacs 28.2, unlike the problem Jon
> describes with notmuch-emacs.

If I understand the comment from Emacs developer Eli Zaretskii [1]
correctly, the way that notmuch has been hiding images for the last
decade or so was never really supposed to work.

I have started to investigate an alternative strategy of using the
"undisplayer" functions provided by gnus (see the end of
mm-inline-image). I got as far as (permanently) removing the image, but
not restoring it yet. I suspect this method will be more resource
intensive, but it should not matter much since toggling images is not
something that happens for multiple images at a time.


[1]: https://lists.gnu.org/archive/html/emacs-devel/2023-08/msg00593.html

[PATCH] WIP: use gnus supplied undisplay function to hide images

[David Bremner]

According to emacs upstream [1], we can't expect overlay invisibilityy
and images to get along. This is a first attempt to use the
"undisplayer" functions saved by gnus (specificially mm-insert-image)
to do the hiding.

There are some potential efficiency concerns. A second copy of all
parts is saved. That could (perhaps?) be optimized to only be done for
the toggled ones. Profile / test first, optimize later

[1]: https://lists.gnu.org/archive/html/emacs-devel/2023-08/msg00593.html
---
 emacs/notmuch-lib.el  |  1 +
 emacs/notmuch-show.el | 38 ++++++++++++++++++++++++++++++--------
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 84ba8c5e..a09f4ab8 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -703,6 +703,7 @@ current buffer, if possible."
 	  (when (mm-inlinable-p handle)
 	    (set-buffer display-buffer)
 	    (mm-display-part handle)
+	    (plist-put part :undisplayer (mm-handle-undisplayer handle))
 	    t))))))
 
 ;;; Generic Utilities
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 36cce619..7911be5d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -618,7 +618,7 @@ message at DEPTH in the current thread."
   (let ((button (or button (button-at (point)))))
     (when button
       (let ((overlay (button-get button 'overlay))
-	    (lazy-part (button-get button :notmuch-lazy-part)))
+	    (lazy-part 	(button-get button :notmuch-lazy-part)))
 	;; We have a part to toggle if there is an overlay or if there
 	;; is a lazy part.  If neither is present we cannot toggle the
 	;; part so we just return nil.
@@ -644,8 +644,25 @@ message at DEPTH in the current thread."
 		(when show
 		  (button-put button :notmuch-lazy-part nil)
 		  (notmuch-show-lazy-part lazy-part button))
-	      ;; else there must be an overlay.
-	      (overlay-put overlay 'invisible (not show))
+	      (let* ((part (plist-get properties :notmuch-part))
+		     (undisplayer (plist-get part :undisplayer))
+		     (mime-type (plist-get part :content-type))
+		     (redisplay-data (button-get button
+						 :notmuch-redisplay-data))
+		     ;; XXX FIXME use computed content type
+		     (imagep (string-match "^image/" mime-type)))
+		(cond
+		 ((and imagep (not show) undisplayer)
+		  ;; call undisplayer thunk created by gnus.
+		  (funcall undisplayer)
+		  ;; there is an extra newline left
+		  (delete-region
+		   (+ 1 (button-end button))
+		   (+ 2 (button-end button))))
+		 ((and imagep show redisplay-data)
+		  (notmuch-show-lazy-part redisplay-data button))
+		 (t
+		  (overlay-put overlay 'invisible (not show)))))
 	      t)))))))
 
 ;;; Part content ID handling
@@ -1023,6 +1040,7 @@ will return nil if the CID is unknown or cannot be retrieved."
       (save-restriction
 	(narrow-to-region part-beg part-end)
 	(delete-region part-beg part-end)
+	(button-put button :notmuch-redisplay-data part-args)
 	(apply #'notmuch-show-insert-bodypart-internal part-args)
 	(indent-rigidly part-beg
 			part-end
@@ -1106,14 +1124,18 @@ is t, hide the part initially and show the button."
 			     (and deep button)
 			     (and high button)
 			     (and long button))))
-	 (content-beg (point)))
+	 (content-beg (point))
+	 (part-data (list msg part mime-type nth depth button)))
     ;; Store the computed mime-type for later use (e.g. by attachment handlers).
     (plist-put part :computed-type mime-type)
-    (if show-part
-	(notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
+    (cond
+     (show-part
+      (notmuch-show-insert-bodypart-internal msg part mime-type nth depth button)
+      (when button
+	(button-put button :notmuch-redisplay-data part-data)))
+     (t
       (when button
-	(button-put button :notmuch-lazy-part
-		    (list msg part mime-type nth depth button))))
+	(button-put button :notmuch-lazy-part part-data))))
     ;; Some of the body part handlers leave point somewhere up in the
     ;; part, so we make sure that we're down at the end.
     (goto-char (point-max))
-- 
2.40.1