After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 546664 - Some tweaks to the prefer plain-text plugin
Some tweaks to the prefer plain-text plugin
Status: RESOLVED DUPLICATE of bug 470291
Product: evolution
Classification: Applications
Component: Plugins
unspecified
Other Linux
: Normal minor
: ---
Assigned To: evolution-plugin-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2008-08-06 20:00 UTC by Paul Bolle
Modified: 2009-08-06 12:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
two minor tweaks and some minor cleanup (2.80 KB, patch)
2008-08-06 20:02 UTC, Paul Bolle
none Details | Review
Now with correct changelog entry (2.80 KB, patch)
2008-08-06 20:05 UTC, Paul Bolle
reviewed Details | Review

Description Paul Bolle 2008-08-06 20:00:34 UTC
0) In text only mode the text/html part of a multipart message will now be attached as:
    HTML document attachment (attachment.html)

If one tries to view that attachment inline, it will end up as:
    HTML document attachment (attachment.html)
    unknown attachment (attachment.html)
which cannot acrually be viewed. Besides, this will be printed to stderr:
    (evolution:[...]): evolution-mail-WARNING **: unable to expand the attachment

That seems to be because viewing it inline will once again invoke this plugin, which has been configured to not show text/html.

The solution is to format that part as NULL, so that it cannot be viewed inline:
    em_format_part_as (format, stream, part, NULL);

1) In text only mode a text/html only message will show up as an
    unknown attachment
For consistency with text/html parts of multipart messages, let it also show up as (unless it already has a name, which is unlikely):
    unknown attachment (attachment.html)

2) While I was at it, I cleaned up the code a little:
    Add EPP_DISABLED and NUM_EPP_TYPES to the enum and
    actually use the constants defined in the enum wherever we can.

3) Patch to be attached shortly.
Comment 1 Paul Bolle 2008-08-06 20:02:12 UTC
Created attachment 116002 [details] [review]
two minor tweaks and some minor cleanup
Comment 2 Paul Bolle 2008-08-06 20:05:20 UTC
Created attachment 116003 [details] [review]
Now with correct changelog entry
Comment 3 Srinivasa Ragavan 2008-08-11 05:04:02 UTC
Milan, Can you review it?
Comment 4 Milan Crha 2008-08-11 11:32:36 UTC
I like the ability to expand attachment.html to view it, which is available in the Prefer mode. It's fine to not have it in Text only mode, but I would like to
see "HTML document attachment", instead of "unknown attachment" text, because it is an HTML document attachment.

In that way, would you mind to add some way to force the attachment button disabled, based on the property of the part probably?

The patch is fine, otherwise.
Comment 5 Paul Bolle 2008-08-26 23:52:47 UTC
(In reply to comment #4)
> [...] I would like to see "HTML document attachment", instead of "unknown
> attachment" text, because it is an HTML document attachment.
> 
> In that way, would you mind to add some way to force the attachment button
> disabled, based on the property of the part probably?
>
> The patch is fine, otherwise.

If I understand you correctly, you actually agree with the patch (which basically adds consistency to the way text/html parts and messages are displayed in text only mode) but you do have an enhancement suggestion (to show "HTML document attachment", instead of "unknown attachment").

If I'm correct I'd suggest to use my patch without your enhancement, and add that enhancement later.

Please understand that I do agree with your suggestion. However, I'm _guessing_ that it might be harder to implement than it looks at first sight. As far as I remember the "HTML document attachment" and the "unknown attachment" text is generated by code not really touched by the plugin. Same goes for enabling and disabling of the attachment button. Not sure at all if that's something a plugin can influence. But I haven't looked at the relevant code in three weeks (which is more than enough to forget most details).
Comment 6 Milan Crha 2008-08-27 08:17:30 UTC
Yes, I "only" suggest different solution, because I believe it would be better. The thing is the solution will be really different from that yours so I'm not much willing to put the former one in the code, even it is possible way to let this work better. (Just a side note, not related to HTML at all: when I have itip-formatter plugin disabled, then I can reproduce the issue in 0) too, (but with the vcalendar attachment), so we can benefit with disable-able attachment button even here, but with change in non-plugin.)
Comment 7 Paul Bolle 2008-08-27 08:37:44 UTC
(In reply to comment #6)
> (Just a side note, not related to HTML at all: when I have
> itip-formatter plugin disabled, then I can reproduce the issue in 0) too, (but
> with the vcalendar attachment), so we can benefit with disable-able attachment
> button even here, but with change in non-plugin.)

Is this in trunk? With ".vcf" attachments? (I remember having problems with the ".vcf" attachments the TNEF Attachment Decoder plugin generated in trunk some time ago. Simply changing the extension to ".ics" in the code solved those problems for me then. (Those problems were, basically, that ".vcf" attachments weren't detected as Calendar attachments.) If so, that should probably be a separate bug. I haven't filed it/searched for it yet.

I'm not running trunk (right) now, but something 2.22.3 based, so can't check though.
Comment 8 Paul Bolle 2008-08-27 09:03:39 UTC
(In reply to comment #6)
> Yes, I "only" suggest different solution, because I believe it would be better.
> The thing is the solution will be really different from that yours so I'm not
> much willing to put the former one in the code, even it is possible way to let
> this work better.

Well, my trivial patch fixes some inconsistencies and unexpected behavior in text only mode of this plugin right now. Applying it shouldn't lessen the chances for a better, more general solution in the future.
Comment 9 Milan Crha 2008-08-27 16:37:31 UTC
(In reply to comment #7)
> Is this in trunk? With ".vcf" attachments? ...

I saw this on meeting invite sent by evolution, with itip-formatter plugin *disabled*.
Comment 10 Milan Crha 2009-08-06 12:17:17 UTC
Oops, I totally forgot of this, and did some enhancements in bug #470291. As the time passed, the idea about a solution changed. I'm marking this as a duplicate of the older bug.

*** This bug has been marked as a duplicate of 470291 ***