1048658 - implement MAPISendMailW for unicode messages and Windows 8

Status: RESOLVED FIXED

(MailNews Core :: Simple MAPI, defect)

Description

[Matt]

Apparently windows 8 does not support MAPISendMAil and the new MAPISendMailW replaces it.

Right click > send mail to among other functions.

Comment 1

[Mike Kaganski]

Attachment: Bug 1048658: Implement MAPISendMailW

Since Windows 8, a new Simple MAPI fucntion is available:
MAPISendMailW (https://msdn.microsoft.com/en-us/library/hh707275).
It allows for sending UTF-16-encoded string to default mail provider.

Currently, is an application tries to use MAPISendMailW (available
in mapi32.dll), the library checks if default mail provider exports
a function with that name, and if not, it internally converts data to
8-bit CP_ACP before sending to old MAPISendMail. In this case, any
data that can not be encoded in active 8-bit codepage, will arrive
replaced with some default replacement character, usually question
mark ("?").

This has a number of problems:
1. Text may be garbled.
2. Attachments may be inaccessible.
3. Attachments may fail to save.

The problem #2 is when the attachment temporary file name includes
such characters. Replacing correct character in names naturally
makes files not accessible.

The problem #3 is when attachment temporary file is accessible, but
its user-visible name is different and contains such characters. In
that case, the 8-bit string encoded in CP_ACP would contain "?",
which are illegal in file names on Windows. TB tries to save the
file locally with the user-visible name, which would fail, and then
fail to send or save the message.

The patch adds MAPISendMailW exported function to MapiDll, and
its implementation to CMapiImp. To do that, it introduces a new
COM interface: nsIMapi2, which inherits from nsIMapi, adding the
new function. To use the IIDs in the MapiDll, it links to MIDL-
generated msgMapi_i.c, and thus removes the definition from
MapiDll.cpp. The old interface is changed a little (nsMapiFileDesc
now explicitly uses 8-bit strings, which is consistent with MS
definition of MapiFileDesc structure - see MSDN documentation at
https://msdn.microsoft.com/en-us/library/dd296737), but this change
is ABI-compatible and doesn't change behaviour, and only replaces
some casts in code with opposite casts (btw, it reduces the number
of required casts :)).

Review commit: https://reviewboard.mozilla.org/r/226436/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226436/

Comment 2

[Mike Kaganski]

Attachment: aбγ.odt - a sample file to test MAPISendMailW using LibreOffice

Windows 8 and later do support MAPISendMail. Still, a number of problems are associated with the 8-bit MAPI, that are listed in the proposed change commit message:

1. Text may be garbled.
2. Attachments may be inaccessible.
3. Attachments may fail to save.

The problem #2 is when the attachment temporary file name includes characters not representable using 8-bit CP_ACP. Replacing correct character in names naturally makes files not accessible.

The problem #3 is when attachment temporary file is accessible, but its user-visible name is different and contains such characters. In that case, the 8-bit string encoded in CP_ACP would contain "?", which are illegal in file names on Windows. TB tries to save the file locally with the user-visible name, which would fail, and then fail to send or save the message.

The change may be tested using current master of LibreOffice, which includes a series of changes by me (starting https://cgit.freedesktop.org/libreoffice/core/commit/?id=c2f7759e85f3e5cc9f56aaf03eefa0f1ba834734) that allow using MAPISendMAilW and pass UTF-16. To test, use an attached document, which is a simple text document, that is named "aбγ.odt", thus containing a Latin, Cyrillic and Greek characters, and so isn't representable in any single codepage that could be used as active codepage on Windows (which still lacks an ability to use UTF-8 as ACP).

Open the file in a LibreOffice daily from https://dev-builds.libreoffice.org/daily/master/, and use its File->Send-E-Mail Document... (of course, the TB in comm-central\obj-i686-pc-mingw32\dist\bin must be registered by using its uninstall\helper.exe /SetAsDefaultAppGlobal or /SetAsDefaultAppUser, and also regsvr32 path\to\comm-central\obj-i686-pc-mingw32\dist\bin\MapiProxy.dll; I also had to replace mozMapi32_InUse.dll with mozMapi32.dll in DLLPath under HKEY_LOCAL_MACHINE\SOFTWARE\Clients\Mail\Mozilla Thunderbird).

Comment 3

[Jorg K (CEST = GMT+2)]

Mike, this bug has no audience and you didn't ask anyone for review. In the meantime, review board has been phased out. Can you please attach the a simple text patch.

Also, I've tried to do the same thing in bug 1506488. That patch actually doesn't work, I don't know why. It's similar to yours. As you've seen, there's a flurry of activities in bug 393302, and we're actually removing code like

+	else
+		lpFiles = lpMessage->lpFiles;
+
+	hr = pNsMapi2->SendMailW(lhSession, lpMessage,
+		(short)lpMessage->nRecipCount, lpRecips,
+		(short)lpMessage->nFileCount, lpFiles,
+		flFlags, ulReserved);

in favour of copying the memory.

Comment 5

[Mike Kaganski]

(In reply to Jorg K (GMT+1) from comment #3)

well - let me postpone updating this until the changes are settled in bug 393302 then...

Comment 6

[Mike Kaganski]

Attachment: MAPISendMailW.diff

This is updated after bug 393302. I saw that in the latter, the COM interface was changed easily, thus I decided to not create a "nsIMapi2", but simply extend existing nsIMapi.

After this patch, both LibreOffice and Explorer's "Send to -> mail recipient" start using this method in my testing on Windows 10.

Comment 7

[Mike Kaganski]

Of course, the change has big parts copy-pasted. Yet, I didn't try to refactor the duplicated code. Unable to afford this at the moment, sorry.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9037406 [details] [diff] [review]
MAPISendMailW.diff

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

::: mailnews/mapi/mapihook/src/msgMapiHook.cpp
@@ +478,5 @@
> +            pFile->InitWithPath (nsDependentString(aFiles[i].lpszPathName));
> +
> +            bool bExist ;
> +            rv = pFile->Exists(&bExist) ;
> +            MOZ_LOG(MAPI, mozilla::LogLevel::Debug, ("nsMapiHook::HandleAttachments: filename: %s path: %s exists = %s \n", (const char*)aFiles[i].lpszFileName, (const char*)aFiles[i].lpszPathName, bExist ? "true" : "false"));

I think %s is wrong here for wchar file names. See attachment 9024554 [details] [diff] [review], and there msgMapiHook.cpp and HandleAttachmentsW.

Comment 9

[Mike Kaganski]

Oh thank you! - of course you are right. Will update.

Comment 10

[Mike Kaganski]

Attachment: MAPISendMailW.diff

Patch updated after comment 8

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch

Added HG header and commit message. Fixed white-space issues, overlong lines, one copy/paste error (in debug output).

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch

More copy/paste errors.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch

... and one more :-(

Comment 15

[Mike Kaganski]

Jorg: thank you so much! Sorry for all these errors. I admit that I didn't pay attention to comments/debug string :-( - but I hope that the executed code (apart from debug strings) should be OK? I tested it before sending, be assured!

Comment 16

[Jorg K (CEST = GMT+2)]

Comment on attachment 9037507 [details] [diff] [review]
1048658-MAPISendMailW.patch

Works for me. My copy of LibreOffice 5.4.6.2 doesn't use the new interface, but Window own St>Mr does as per my previous observation in the duplicate bug I started.

I can now send a file named テスト.txt which was possible via the non-W interface only with the system locale set to UTF-8.

Comment 17

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch

More white-space tweaks.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/aaf39a9bd368
Implement MAPISendMailW(). r=jorgk

Comment 19

[Frank-Rainer Grahl (:frg)]

I backported it to SeaMonkey 2.53 based on 56 and this seems to work fine there in Windows 8.1 and Server 2016. But under Windows 7 files names with unicode chars no longer result in a mail window shown. Previously they would show ?? for the unicode chars in the attachment.

Could someone test this with a Nighly Thunderbird under Windows 7. Regular file names still work so it is probably a unicode only regression if any. I am still on TB 60.5 and don't want to mess with the profile.

Comment 20

[Jorg K (CEST = GMT+2)]

Thanks for testing!

There's actually no such thing as a "unicode character". Unicode is character encoding, so for example Ä or テ can be encoded in many different ways, UTF-8 being one of them.

So what you mean is that on a Windows system with a Western code page, like windows-1252, you used a file name which can't be encoded in that code page, right?

Unless I'm reading something incorrectly, MAPISendMailW is not available on Windows 7 (https://docs.microsoft.com/en-us/windows/desktop/api/mapi/nc-mapi-mapisendmailw). So this would be a regression of bug 393302, or maybe bug 1521007? Are you 100% sure that ever worked, see my comment in bug 393302 comment #69 where I tried with Japanese and Polish file names and no compose window came up. I'm also not sure where the ? replacement would have happened and how attaching a file could have worked, since it didn't exist with the ? name.

Comment 21

[Frank-Rainer Grahl (:frg)]

I know. Was just mean't as a simpification. Used the attached test file here.

It seems Virtualbox might be the culprit. If I try to use a unicode file from a virtual box shared folder in Win 7 it won't work. With a local file it seems to work but it displays the short Windows name for the attachment name. Both are ok under 8.1 and up. SeaMonkey has some different attachment handling which might be the cause here and it is also a downlevel Gecko 56 albeit heavily patched up. I wouldn't bother if the unicode case generally works in Win 7. Great work so far.

Comment 22

[Mike Kaganski]

(In reply to Frank-Rainer Grahl (:frg) from comment #19)

I backported it to SeaMonkey 2.53 based on 56 and this seems to work fine there in Windows 8.1 and Server 2016. But under Windows 7 files names with unicode chars no longer result in a mail window shown. Previously they would show ?? for the unicode chars in the attachment.

It's very important to also state which program was used as calling application. E.g., last year I have changed LibreOffice to create a temporary file with ASCII-only names on send, and pass that filename to MAPI, to workaround that deficiency (in https://git.libreoffice.org/core/+/5566df47de17eca8c0a5b447bcb906051c869db6). So using LibreOffice, that would previously work (and now, too). Word also uses that technique. But Windows Explorer right-click menu launches some other application, which never could send files with names not representable in ACP ("Unicode names") previously to this patch. And Windows 7 doesn't have the W function in MAPI, as Jorg mentioned.

Please check the "Previously they would show ??" part, I guess it's some mistake.

Comment 23

[Jorg K (CEST = GMT+2)]

Or an artefact of using the VM? Kai in bug 393302 comment #59 and cmt #64 claimed that he could send files with "unicode names" on his VM when I couldn't on "real" Windows 10.

Comment 24

[Mike Kaganski]

(In reply to Jorg K (GMT+1) from comment #23)

I cannot say anything definitive regarding what could that be, except that Kai should have debugged that using VS debugger back then. I doubt, though, that UTF-8, even if actually sent by Explorer in the data, could be processed properly by TB back then, without what we implemented in bug 1521007, unless one used beta UTF-8 ACP in Windows 10. And I cannot see anything in our code that could describe VM differences.

Comment 25

[:aceman]

Thank you guys for movement in this area. This looks like a reasonably standalone patch, would it be a candidate for backport to ESR60? Looks like bug 393302 is too.

Comment 26

[Jorg K (CEST = GMT+2)]

Sure both (and more) are on my mental list. I plan to ship all MAPI fixes in TB 60.6.

Comment 27

[bblack]

(In reply to Frank-Rainer Grahl (:frg) from comment #19)

But under Windows 7 files names with unicode chars no longer result in a mail window shown.

(In reply to Mike Kaganski from comment #22)

And Windows 7 doesn't have the W function in MAPI, as Jorg mentioned.

MS says:
On Windows 8 and later: Call MAPISendMailW to send a message.
On Windows 7 and earlier: Use MAPISendMailHelper to send a message.
and:
MAPISendMailHelper function:
Takes Unicode message information and sends the message using MAPISendMailW or, if necessary, converts the message to ANSI and sends the message using MAPISendMail.
(https://docs.microsoft.com/de-de/windows/desktop/api/mapiunicodehelp/nf-mapiunicodehelp-mapisendmailhelper)

I think that first MAPISendMailHelper must be implemented so that unicode chars in the attachment works under Windows 7.

Comment 28

[Mike Kaganski]

(In reply to bblack from comment #27)

I think that first MAPISendMailHelper must be implemented so that unicode chars in the attachment works under Windows 7.

No. MAPISendMailHelper is not something anyone needs to implement, because it's a function fully implemented in MS in Windows SDK (in MAPIUnicodeHelper.h). Its source code is open (in the said header), and is built-in into any application that decides to use this helper function.

Its deficiencies (related to handling of CP_UTF8) are raised in my request [1].

[1] https://developercommunity.visualstudio.com/content/problem/431508/mapisendmailhelper-implementation-is-inconsistent.html

Comment 29

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch - ESR60 version

Comment 30

[Jorg K (CEST = GMT+2)]

Attachment: 1048658-MAPISendMailW.patch - ESR60 version

Comment 33

[Jorg K (CEST = GMT+2)]

TB 65 beta 4:
https://hg.mozilla.org/releases/comm-beta/rev/d6f7c0483b263905ed42ceebb5d61e0a5e5448a2

Comment 34

[Jorg K (CEST = GMT+2)]

Comment on attachment 9038024 [details] [diff] [review]
1048658-MAPISendMailW.patch - ESR60 version

Let's leave some of the fun for TB 68. We had enough MAPI chagrin in TB 60. The patch doesn't apply anyway any more.