[PATCH] emacs: add buttons for all multipart/related parts

[PATCH] emacs: add buttons for all multipart/related parts

[Istvan Marko]

When text/html parts include images as multipart/related and the
text/plain alternative is used these images can be completely hidden
with no easy way to access them or even find out that they are there.

Make notmuch-show-insert-part-multipart/related add buttons for all
parts, the first one visible the rest hidden.
---
 emacs/notmuch-show.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 82b70ba..20844f0 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -621,6 +621,10 @@ message at DEPTH in the current thread."
 
     ;; Render the primary part.
     (notmuch-show-insert-bodypart msg (car inner-parts) depth)
+    ;; Add hidden buttons for the rest
+    (mapc (lambda (inner-part)
+	    (notmuch-show-insert-bodypart msg inner-part depth t))
+	  (cdr inner-parts))
 
     (when notmuch-show-indent-multipart
       (indent-rigidly start (point) 1)))
-- 
1.8.1.4




-- 
	Istvan

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

On Fri, Aug 16 2013, Istvan Marko <notmuch@kismala.com> wrote:
> When text/html parts include images as multipart/related and the
> text/plain alternative is used these images can be completely hidden
> with no easy way to access them or even find out that they are there.
>
> Make notmuch-show-insert-part-multipart/related add buttons for all
> parts, the first one visible the rest hidden.

Hey, Istvan.  Thanks so much for this patch.  It seems to be addressing
the same issue I reported in id:87ob8u8l6k.fsf@servo.finestructure.net.

However, the behavior of the part button that now appears seems to be a
bit strange.  Clicking/hitting enter on the part attempts to save it
rather than open it.  I wouldn't be surprised if has nothing to do with
the correctness of this patch, though.  So is it possible that the
button is not inserted in the most ideal way, or is there some other
issue preventing it from behaving properly (could it have to do with the
part being "inline"?)?

jamie.


>  emacs/notmuch-show.el | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 82b70ba..20844f0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -621,6 +621,10 @@ message at DEPTH in the current thread."
>  
>      ;; Render the primary part.
>      (notmuch-show-insert-bodypart msg (car inner-parts) depth)
> +    ;; Add hidden buttons for the rest
> +    (mapc (lambda (inner-part)
> +	    (notmuch-show-insert-bodypart msg inner-part depth t))
> +	  (cdr inner-parts))
>  
>      (when notmuch-show-indent-multipart
>        (indent-rigidly start (point) 1)))
> -- 
> 1.8.1.4

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Istvan Marko]

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> However, the behavior of the part button that now appears seems to be a
> bit strange.  Clicking/hitting enter on the part attempts to save it
> rather than open it.  

This is controlled by notmuch-show-part-button-default-action. 

There are also the "." bindings, hit ". C-h" to see them.

-- 
	Istvan

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Tue, Aug 20 2013, Istvan Marko <notmuch@kismala.com> wrote:
> Jameson Graef Rollins <jrollins@finestructure.net> writes:
>
>> However, the behavior of the part button that now appears seems to be a
>> bit strange.  Clicking/hitting enter on the part attempts to save it
>> rather than open it.  
>
> This is controlled by notmuch-show-part-button-default-action. 
>
> There are also the "." bindings, hit ". C-h" to see them.

Hi, Istvan.  Yes, I'm aware of these options, and they're not the issue.
This button is behaving differently than buttons for inline image/*
parts in other messages.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Istvan Marko]

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> On Tue, Aug 20 2013, Istvan Marko <notmuch@kismala.com> wrote:

>> This is controlled by notmuch-show-part-button-default-action. 
>>
>> There are also the "." bindings, hit ". C-h" to see them.
>
> Hi, Istvan.  Yes, I'm aware of these options, and they're not the issue.
> This button is behaving differently than buttons for inline image/*
> parts in other messages.

I see. Not sure what's going on then, for me they behave like other
buttons, with notmuch-show-part-button-default-action set to
notmuch-show-view-part RET shows them inline or with an external viewer.

-- 
	Istvan

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Mark Walters]

Hi

What does the (mis-behaving)  part button say? is it [image/jpeg] or
[application/octet-stream as image/jpeg] or? and what do correctly
behaving part buttons say?

Best wishes

Mark

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> On Fri, Aug 16 2013, Istvan Marko <notmuch@kismala.com> wrote:
>> When text/html parts include images as multipart/related and the
>> text/plain alternative is used these images can be completely hidden
>> with no easy way to access them or even find out that they are there.
>>
>> Make notmuch-show-insert-part-multipart/related add buttons for all
>> parts, the first one visible the rest hidden.
>
> Hey, Istvan.  Thanks so much for this patch.  It seems to be addressing
> the same issue I reported in id:87ob8u8l6k.fsf@servo.finestructure.net.
>
> However, the behavior of the part button that now appears seems to be a
> bit strange.  Clicking/hitting enter on the part attempts to save it
> rather than open it.  I wouldn't be surprised if has nothing to do with
> the correctness of this patch, though.  So is it possible that the
> button is not inserted in the most ideal way, or is there some other
> issue preventing it from behaving properly (could it have to do with the
> part being "inline"?)?
>
> jamie.
>
>
>>  emacs/notmuch-show.el | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 82b70ba..20844f0 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -621,6 +621,10 @@ message at DEPTH in the current thread."
>>  
>>      ;; Render the primary part.
>>      (notmuch-show-insert-bodypart msg (car inner-parts) depth)
>> +    ;; Add hidden buttons for the rest
>> +    (mapc (lambda (inner-part)
>> +	    (notmuch-show-insert-bodypart msg inner-part depth t))
>> +	  (cdr inner-parts))
>>  
>>      (when notmuch-show-indent-multipart
>>        (indent-rigidly start (point) 1)))
>> -- 
>> 1.8.1.4
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

On Tue, Aug 20 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> What does the (mis-behaving)  part button say? is it [image/jpeg] or
> [application/octet-stream as image/jpeg] or? and what do correctly
> behaving part buttons say?

Hey, Mark.  That's the perplexing part: the parts seem otherwise very
similar.  For the case of the part that is *not* handled correctly, the
Content info and button are:

  Content-Type: image/jpeg; name="photo.JPG"
  Content-Transfer-Encoding: base64
  Content-Id: <...>
  Content-Disposition: inline; filename="photo.JPG"

  [ photo.JPG: image/jpeg ]

And in the case of a test message that seems to behave as expected:

  Content-Type: image/jpeg
  Content-Disposition: inline; filename=monkey.jpg
  Content-Transfer-Encoding: base64
  Content-Description: monkey!

  [ monkey.jpg: image/jpeg ]

In the first case clicking on the part button has no effect.  In the
later case the image opens fine in the external viewer.

I guess the only other difference is the rest of the MIME structure that
these parts are embedded in...

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Mark Walters]

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> On Fri, Aug 16 2013, Istvan Marko <notmuch@kismala.com> wrote:
>> When text/html parts include images as multipart/related and the
>> text/plain alternative is used these images can be completely hidden
>> with no easy way to access them or even find out that they are there.
>>
>> Make notmuch-show-insert-part-multipart/related add buttons for all
>> parts, the first one visible the rest hidden.
>
> Hey, Istvan.  Thanks so much for this patch.  It seems to be addressing
> the same issue I reported in id:87ob8u8l6k.fsf@servo.finestructure.net.
>
> However, the behavior of the part button that now appears seems to be a
> bit strange.  Clicking/hitting enter on the part attempts to save it
> rather than open it.  I wouldn't be surprised if has nothing to do with
> the correctness of this patch, though.  So is it possible that the
> button is not inserted in the most ideal way, or is there some other
> issue preventing it from behaving properly (could it have to do with the
> part being "inline"?)?


Hi

I think I have diagnosed this bug. There is a potential fix in
id:1377246875-7784-1-git-send-email-markwalters1009@gmail.com 

The bug is nothing to do with Istvan's patch: his patch just triggers the
underlying bug in the lazy part handling.

Best wishes

Mark

Re: [PATCH] emacs: add buttons for all multipart/related parts

[Mark Walters]

On Fri, 16 Aug 2013, Istvan Marko <notmuch@kismala.com> wrote:
> When text/html parts include images as multipart/related and the
> text/plain alternative is used these images can be completely hidden
> with no easy way to access them or even find out that they are there.
>
> Make notmuch-show-insert-part-multipart/related add buttons for all
> parts, the first one visible the rest hidden.

Hello

This patch LGTM +1.

It is a clear improvement anyway but would benefit from my fix for the
bug this exposes
id:1377246875-7784-1-git-send-email-markwalters1009@gmail.com (or some
alternative). I don't know if we want to push the two together or just
push each as it gets reviewed.

Best wishes

Mark






> ---
>  emacs/notmuch-show.el | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 82b70ba..20844f0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -621,6 +621,10 @@ message at DEPTH in the current thread."
>  
>      ;; Render the primary part.
>      (notmuch-show-insert-bodypart msg (car inner-parts) depth)
> +    ;; Add hidden buttons for the rest
> +    (mapc (lambda (inner-part)
> +	    (notmuch-show-insert-bodypart msg inner-part depth t))
> +	  (cdr inner-parts))
>  
>      (when notmuch-show-indent-multipart
>        (indent-rigidly start (point) 1)))
> -- 
> 1.8.1.4
>
>
>
>
> -- 
> 	Istvan
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: add buttons for all multipart/related parts

[David Bremner]

Istvan Marko <notmuch@kismala.com> writes:

> When text/html parts include images as multipart/related and the
> text/plain alternative is used these images can be completely hidden
> with no easy way to access them or even find out that they are there.

pushed,

d