#470 MH-E discards text properties in emacs > v24

[Ted Phelps]

While chasing bug 267, I noticed that the text-mode function, which is called by mh-show-mode, discards all text properties. HTML e-mail rendered via shr.el gets rendered first, and then mh-show-mode is invoked: all of the careful highlighting and coloring is thrown away. Amusingly, the overlays shr uses to pad tables remain, so the result not the improvement you might think.

-Ted

[Ted Phelps]

The attachment above, 12703, is an example e-mail that displays the behavior. Note that you must enable shr as your HTML renderer to see the true horror.

Also attached is a patch that works around the problem. It might make more sense to render the HTML after mh-show-mode is invoked, but that looks trickier to achieve.

-Ted

[Mike Kupfer]

I recently ran into this problem on Emacs 24.5 and Emacs 25 without knowing about this bug. I agree with Ted's analysis of the root cause. It's non-obvious from just looking at the code, because the definition of text-mode doesn't show any font-lock calls. I don't know if it's some magic associated with derived modes or if there's a hook getting invoked invisibly. But you can see the rendering change if you set a breakpoint at entry to mh-show-mode and then walk through with the debugger. I eventually got this stack. On entry to font-lock-default-function, the message was rendered using shr. On return from font-lock-default-function, the rendering had changed.

  Debugger entered--entering a function:
  * font-lock-default-function(nil)
  * font-lock-mode(-1)
  * font-lock-change-mode()
  * kill-all-local-variables()
  * text-mode()
  * #[nil ...  ("/usr/local/share/emacs/24.5/lisp/mh-e/mh-show.elc" . 103006) nil]()
  * apply(#[nil ...  ("/usr/local/share/emacs/24.5/lisp/mh-e/mh-show.elc" . 103006) nil] nil)
  * mh-show-mode()
    mh-display-msg(5 "+emacs/test-cases/html")
    mh-show-msg(nil)
    mh-show(nil t)
    call-interactively(mh-show nil nil)
    command-execute(mh-show)

I tried moving the mh-show-mode call to come before the MIME rendering.

         (mh-show-mode)
         ;; Use mm to display buffer
         (when (and mh-decode-mime-flag (not formfile))
           (mh-add-missing-mime-version-header)
           (setf (mh-buffer-data) (mh-make-buffer-data))
           (mh-mime-display))
         ;; Header cleanup

That doesn't quite work, because mh-show-mode wants to make the buffer read-only. But if I comment out that line in mh-show-mode, the message appears to display correctly.

[Mike Kupfer]

adding this to my queue

[Mike Kupfer]

I was dinking around with this, and the problem is related to this block of code in mh-show-mode:

  (cond
   ((equal mh-highlight-citation-style 'font-lock)
    (setq font-lock-defaults '(mh-show-font-lock-keywords-with-cite t)))
   ((equal mh-highlight-citation-style 'gnus)
    (setq font-lock-defaults '((mh-show-font-lock-keywords)
                               t nil nil nil
                               (font-lock-fontify-region-function
                                . mh-show-font-lock-fontify-region)))
    (mh-gnus-article-highlight-citation))
   (t
    (setq font-lock-defaults '(mh-show-font-lock-keywords t))))

If I comment it out, the shr rendering is no longer clobbered. But commenting out that block breaks the multicolor rendering of quoted text, as in

Person A wrote:

Person B wrote:

blah

reply to blah

[Mike Kupfer]

A little more experimentation shows that even if mh-highlight-citation-style is nil, MH-E ends up throwing away the fontification that shr did.

Gnus doesn't seem to have these problems, so one possible way forward is to see how the MH-E message display code differs from the corresponding code in Gnus.

[Mike Kupfer]

I was able to "fix" the problem by disabling font-lock as part of the buffer reset that mh-display-msg does:

             (setq buffer-read-only nil)
             ;; Cleanup old mime handles
             (mh-mime-cleanup)
             (erase-buffer)
             ;; Changing contents, so this hook needs to be reinitialized.
             ;; pgp.el uses this.
             (if (boundp 'write-contents-hooks) ;Emacs 19
                 (kill-local-variable 'write-contents-hooks))
             (font-lock-mode -1)

And this does not break the fontification of the header or attributed text. But I can't tell whether this is a clean fix or just a hack.

(kill-local-variable 'font-lock-defaults) did not do the trick.

[Stephen Gildea]

I've seen three proposals for how to fix this display problem:

1. call mh-show-mode earlier
2. call mh-show-mode only if not already in MH-Show mode.
3. call (font-lock-mode -1)

To these I'll add a fourth;

1. kill the Show buffer at the start of mh-show-msg.

With all the resetting we do in mh-display-msg, just starting with a
fresh buffer each time has the appeal of simplicity. Thus, option 4
seems good.

There's also something to be said for calling mh-show-mode, which calls
kill-all-local-variables, before trying to set up any fonts. I'm not
sure why it ever works to do call mh-show-mode after, but it does work
the first time. Thus, option 1. A downside is that this changes when
mh-show-mode-hook is called, and thus the change might be visible to
users. An upside is that this seems most likely to be robust in the
long term. Decoupling MH-Show mode and the read-only flag also seems
to simplify the code.

The simplest and least likely to break things now option is to continue
down the road we are on, trying to reset everything, and add
(font-lock-mode -1) to the set of things we do to reset. Thus, option 3.

We should do at least one of these for Emacs 27.1. I'm inclined to do both
option 1, calling mh-show-mode earlier, AND option 3, (font-lock-mode -1).
I've been testing the attached patch.

Any other opinions?

[Stephen Gildea]

Thanks, Mike, for reviewing my patch. I committed it to the emacs-27 release branch as 7f01dfca56.