Closed Bug 1417187 Opened 2 years ago Closed 2 years ago

Port bug 1390708 to C-C: Remove BinHex decoder from mailnews

Categories

(MailNews Core :: Networking, enhancement, P2)

x86_64
Windows 7
enhancement

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 59.0

People

(Reporter: jorgk, Assigned: jorgk)

References

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1390708 +++

Bug 81352 implemented a BinHex decoder to M-C and C-C:
M-C part: Attachment 39730 [details] [diff]
C-C part: Attachment 40138 [details] [diff]
(Note: there are two more attachments in that bug).

Bug 1390708 removed the M-C part:
https://hg.mozilla.org/mozilla-central/rev/eed77d18d006
remove BinHex support from stream converter r=mcmanus
https://hg.mozilla.org/mozilla-central/rev/5638d0a184ff
remove nsBinHexDecoder r=mcmanus

Although this didn't cause bustage, BinHex decoding in TB won't work now.

Joshua said about this situation on dev-planning (quote):
  "I've considered ripping out the binhex decoding from mailnews code."

So let's do that in this bug here. We basically have to visit these sites:
https://searchfox.org/comm-central/search?q=binhex&case=false&regexp=false&path=

Looks like the removal from nsMessenger.cpp, nsMsgAppleDouble.h, nsMsgCompUtils.cpp and mimeTypeCategories.js seems pretty straight forward.

mimeunty.cpp/h needs more looking into.
The main problem with this bug is to understand what it's all about.

The complaint in bug 1390708 comment #14 is that on non-Mac platforms BinHex files are downloaded and *decoded* without removing the .hqx file extension.

The same happens in TB. If you save the attachment from the message, it will be saved as test-file.txt.hqx, but in fact it is already decoded and should be called test-file.txt. If on the other hand you drag the attachment to the desktop (that's working on Windows), it's not decoded.

Since BinHex files are very rare these days, https://en.wikipedia.org/wiki/BinHex, M-C have decided to simply remove the stream decoding of BinHex files. So in fact, they will just be saved with the original content and the original file extension.

Since the stream decoder has gone away, we will just remove the hacky call and be done here. If people really still need to decode BinHex attachments, they need to use a utility which can do that (like WinZip) or an online service.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Simple removal as discussed in the preceding comment.
Attachment #8929782 - Flags: review?(acelists)
Comment on attachment 8929782 [details] [diff] [review]
1417187-binhex.patch

Review of attachment 8929782 [details] [diff] [review]:
-----------------------------------------------------------------

What about the other occurrences from comment 0 ?
They all stay, "application/mac-binhex40" is still a valid MIME type. Surely I could remove 'write_as_binhex' from mailnews/compose/src/nsMsgAppleDouble.h, but it doesn't do any harm.
Comment on attachment 8929782 [details] [diff] [review]
1417187-binhex.patch

Review of attachment 8929782 [details] [diff] [review]:
-----------------------------------------------------------------

More like the stuff in mimeunty.cpp.

But if you do not want to delve into that, we can land this partial removal and leave the bug open for the rest.
Attachment #8929782 - Flags: review?(acelists) → review+
I don't understand the comment. There is nothing else to remove. M-C removed the BinHex (stream) decoder, so we're removing its hacky call. That's all. Other then that, it's still a valid MIME type that we need to handle somehow. If you touch mimeunty.cpp, you'll break it. So there's no "leaving the bug open for the rest", since there is no rest.

M-C haven't removed it either:
https://searchfox.org/mozilla-central/search?q=application%2Fmac-binhex40&case=false&regexp=false&path=
Well, you reported that at least the 6 files need touching. If you reconsidered that since then (but haven't explained) then I'm fine with that. You are the reporter of the bug, you can make the call when to close it :)
(In reply to :aceman from comment #7)
> Well, you reported that at least the 6 files need touching. ...
I was wrong.
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0fca2b648074
Port bug 1390708 to mailnews: Remove implicit stream decoding of BinHex encoded attachment on non-Mac platforms. r=aceman
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 59.0
Duplicate of this bug: 1430469
You need to log in before you can comment on or make changes to this bug.