vm-reply-include-xxxx broken when font is in vm-mime-default-face-charsets

[blueman]

The vm-reply-include-xxxx functions (e.g., vm-reply-include-text, vm-reply-include-presentation) and the associated vm-followup-include-xxxx functions fails to include the text when the content-type of the original email is non-mime and when the associated font is an element of vm-mime-default-face-charsets

This was working properly as recently as 8.0.12...
Not sure what new change broke this...

In the past this function used to work whether or not the font was in
the vm-mime-default-face-charsets alist.

Note the standard vm function vm-reply-include-text is similarly
broken...

I am using vm 8.1.1.

[Uday Reddy (reddyuday)]

Welcome to Launchpad!

It is working fine for me.

By non-MIME message do you mean that it doesn't have a MIME-version
header? What are the MIME headers exactly?

There are three versions of include in VM right now. Please place a
debug-on-entry on all of them: vm-yank-message-presentation,
vm-yank-message-mime and vm-yank-message-text, to see which one is
getting entered. vm-yank-message-mime is the preferred version and it
is selected by the default settings of VM. If you are getting
something different, let me know what settings you are using.

Cheers,
Uday

[blueman (blueman3)]

By non-mime version messages I meant messages that don't have Mime boundaries.

I think I have narrowed it down a bit.
1. vm-reply-include-presentation & content type is *not* an element of the vm-mime-default-face-charsets alist or is not text/plain
    Works properly!

Note the debugger isn't entered unless I also debug on vm-yank-message (which was the original and only yanking function in version vm 8.0)

Debugger entered--entering a function:
* vm-yank-message([[#<marker at 37482822 in Inbox> #<marker at 37482936 in Inbo$
  vm-do-reply(nil t 1)
  vm-reply-include-presentation(1)
  call-interactively(vm-reply-include-presentation t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

2. vm-reply-include-presentation & content type *is* an element of the vm-mime-default-face-charsets alist.
    Fails to include text!

Debugger entered--entering a function:
* vm-yank-message-text([[#<marker at 37482733 in Inbox> #<marker at 37482847 in$
  vm-yank-message([[#<marker at 37482733 in Inbox> #<marker at 37482847 in Inbo$
  vm-do-reply(nil t 1)
  vm-reply-include-presentation(1)
  call-interactively(vm-reply-include-presentation nil nil)

3. vm-reply-include-text - content type is text/plain, *independent* of whether the font is in vm-mime-default-face-charsets or not
 Fails to include text!

Debugger entered--entering a function:
* vm-yank-message-text([[#<marker at 37482733 in Inbox> #<marker at 37482847 in$
  vm-yank-message([[#<marker at 37482733 in Inbox> #<marker at 37482847 in Inbo$
  vm-do-reply(nil t 1)
  vm-reply-include-text(1)
  call-interactively(vm-reply-include-text t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

4. vm-reply-include-text - content type is text/html, *independent* of whether the font is in vm-mime-default-face-charsets or not
Works properly!

Debugger entered--entering a function:
* vm-yank-message-text([[#<marker at 37489495 in Inbox> #<marker at 37489676 in$
  vm-yank-message([[#<marker at 37489495 in Inbox> #<marker at 37489676 in Inbo$
  vm-do-reply(nil t 1)
  vm-reply-include-text(1)
  call-interactively(vm-reply-include-text t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

So, in summary, the original vm-yank-message function seems to be working properly. The problem seems to be with any function that calls vm-yank-message-text on text/plain. Because it works on text/html and it works using vm-reply-include-presentation on text/plain when the font is not in vm-mime-default-face-charsets in which case it calls vm-yank-message directly which still works.

Let me know what else is needed to help you reproduce and/or debug this.
If you want me to help, perhaps explain to me what changes were made to vm-yank-message and what the rationale was for adding the new vm-yank-message-xxxx functions.
My guess is that some use cases were not tested when these changes were made which may be what I am encountering here.

Thanks

[Uday Reddy (reddyuday)]

The function vm-reply-include-presentation is not a VM function. It
is in rfaddons. I am not taking responsibility for whether it works
or not.

When you call vm-reply-include-text, you are getting to use
vm-yank-message-text. That means that you have set
vm-included-mime-types-list to some non-nil value. What did you set
it to? Can you try it after removing this setting and see what you
get?

The 3 versions of the function are:

vm-yank-message-text is the original Kyle's code.
vm-yank-message-mime is Rob's code for VM that was in trunk when I
  took over. It took a while to debug it and get it working fully.
vm-yank-message-presentation is essentially what was in rfaddon's. It
  was used as a stop-gap until the second version worked.

vm-yank-message has to call one of these three. It doesn't do
anything on its own.

As I said, vm-yanke-message-mime is the preferred version. You are
not getting it because of your variable settings.

Cheers,
Uday

[blueman (blueman3)]

OK - I figured out the problem.
It appears that there was an (undocumented) change to the variable vm-included-mime-types
In earlier versions it defaulted to:
   ("text/plain" "text/enriched" "message/rfc822")
So, I then added "text/html" and got the list I wanted.

In the new version, it defaults to nil which has a new meaning now, specifically "include all types that
are handled by VM's MIME decoding mechanism."

So, when I just added "text/html", I got the isolated list of ("text/html") which explains the above behavior more-or-less.

This time I checked in the NEWS file and this change was NOT documented. I really can't emphasize enough the importance of not breaking old behaviors unless such changes are:
 (A) necessary or significant improvements
and (B) well-documented
and (C) done in a way that minimizes the chance of breakage.

 In this case, I would suggest that none of the 3 conditions were satisfied. I'm sorry if I come across as a nag or critic here but I really do care about VM and about all the work everybody is putting into it, but I think that VM in general is at the stage where it is quite stable and mature (except for perhaps IMAP and other new email functionality) so that the bar for changes in core working code needs to be pretty high to justify fixing something that "ain't" broken.

[Uday Reddy (reddyuday)]

blueman writes:

> OK - I figured out the problem.
> It appears that there was an (undocumented) change to the variable
> vm-included-mime-types

Sorry about that. If you were only reading or writing the variable,
you would have been ok. But reading AND writing broke my expected use
case.

I will rename the variable so that people won't get burned again.

The stability of VM is open to question because the bug reports list
on Launchpad is growing at the moment rather than shrinking...

Cheers,
Uday