1681824 - Search in Outlook address book doesn't work

Status: RESOLVED FIXED

(Thunderbird :: Address Book, defect)

Description

[IT Support]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

After fixing the crash in the Outlook address book search in bug 1680923, it turned out that the search doesn't return any results, and has been defective since at least Thunderbird version 60.

Comment 1

[IT Support]

Attachment: 1681824-demo.patch

contents->Restrict() fails with 0x80040117 (MAPI_E_TOO_COMPLEX) here:
https://searchfox.org/comm-central/rev/ea46a911666aedc9b7832f27bf0363a32a4a1ae2/mailnews/addrbook/src/nsAbWinHelper.cpp#788

We looked at the query string passed into nsAbOutlookDirectory::Search() which basically comes from this pref:
https://searchfox.org/comm-central/rev/1c7609a0edd9752c0ae7f5000e9ffe6a53168824/mailnews/mailnews.js#232

This was introduced in bug 118624 here: https://hg.mozilla.org/comm-central/rev/4f4f6a3674c9#l6.30
replacing a less ambitious search https://hg.mozilla.org/comm-central/rev/4f4f6a3674c9#l3.21

However, the complexity of the search is not relevant. As you can see in the enclosed patch, even running a very simple query (or(PrimaryEmail,c,mimi)) doesn't work. This is the output observed with the patch when looking for "mimi" with is part of an e-mail address in our test data:

=== Original query |(and(or(DisplayName,c,mimi)(FirstName,c,mimi)(LastName,c,mim
i)(NickName,c,mimi)(PrimaryEmail,c,mimi)(SecondEmail,c,mimi)(and(IsMailList,=,TR
UE)(Notes,c,mimi))(Company,c,mimi)(Department,c,mimi)(JobTitle,c,mimi)(WebPage1,
c,mimi)(WebPage2,c,mimi)))|
=== Simple query |(or(PrimaryEmail,c,mimi))|
Cannot set restriction 80040117.
=== rt (RES_CONTENT==3) 3
=== property (PR_EMAIL_ADDRESS_A==3003001e) 3003001e
=== value mimi

It shows that the table restriction is properly constructed restricting the rows to PR_EMAIL_ADDRESS_A containing value mimi. Right now, we can't tell what's going wrong since everything looks OK in the simple query case. The puzzling part is: Has this code ever worked and if so, what's changed so it doesn't work any more?

Further reading:
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/imapitable-restrict
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/srestriction
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/spropertyrestriction
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/scontentrestriction
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/spropvalue

Comment 2

[IT Support]

This article mentions the same issue.
https://microsoft.public.win32.programmer.messaging.narkive.com/ueQtsOmF/imapitable-restrict-returns-mapi-e-too-complex-in-outlook-xp

They build a very simple "exist" query:

pRes->rt = RES_EXIST;
pRes->res.resExist.ulReserved1 = 0;
pRes->res.resExist.ulPropTag = PR_DISPLAY_NAME;
pRes->res.resExist.ulReserved2 = 0;
hResult = pTable->Restrict(pRes,0);
ASSERT(hResult == S_OK); // hResult is MAPI_E_TOO_COMPLEX:

and the subsequent Restrict() call fails. The article states:
Most address book providers (unlike message store providers) support only a very limited set of restrictions, only enough to make them work in Outlook. The only restriction guaranteed to work with just about any address book provider is a restriction on PR_ANR property (required for ambiguous name resolution).

However, in this case Outlook is the provider and should allow some simple queries on its own address book. BTW, the author of this reply, Dmitry Streblechenko is the author of OutlookSpy (Outlook and MAPI Developer Tool).

Comment 3

[IT Support]

Attachment: outlook-spy.png

Looking at the Outlook address book with OutlookSpy, we can see that is has one a few columns: The only useful fields are PR_DISPLAY_NAME_W and PR_EMAIL_ADDRESS_W. Note the Unicode properties with the _W suffix. The Thunderbird code tries to query PR_EMAIL_ADDRESS_A which isn't there. So the next attempt needs to be to switch this to unicode and see whether it works then.

Comment 4

[IT Support]

Attachment: 1681824-demo.patch - wide string version

Switching to wide strings, and still no dice :-(

Comment 5

[IT Support]

Attachment: 1681824-fix.patch

OK, we finally fount the clue. We can't search on RES_CONTENT, the things we want to search for are properties, hence we must use RES_PROPERTY as was already used for GT and LT. We can also only search PR_ANR, even a search on PR_DISPLAY_NAME_W fails. If you want to prove that, change the table to

+static const OutlookTableAttr OutlookTableStringToProp[] = {
+    {kPriEmailProperty, PR_DISPLAY_NAME_W}};
+

While we were there, we've changed the search to wide string.

Here are the articles that led us to the solution:
https://docs.microsoft.com/en-us/office/client-developer/outlook/mapi/address-book-restrictions
but mostly:
https://social.msdn.microsoft.com/Forums/office/en-US/9aa03a9e-a254-48a4-9a32-4dffd394e4df/searching-for-contacts-through-mapi-in-c?forum=outlookdev
There is an example with

spv.ulPropTag = PR_ANR;
spv.Value.lpszA = "steve pen";
srName.res.resProperty.lpProp = &spv;
srName.res.resProperty.relop = RELOP_RE;
srName.res.resProperty.ulPropTag = PR_ANR;

Needless to say that this depends on bug 1680923. To conclude the work, we'll turn to bug 1679525. That would make the Outlook address book as good as the other ones.

Comment 6

[IT Support]

Attachment: 1681824-fix.patch

Fixed typo in comment.

Comment 7

[IT Support]

Attachment: PR_ANR-search.png

Illustrating fuzzy PR_ANR search:
First: hoho
Last: haha
Display: hihiDisplay
Email: [e-mail]
is found with haha, hihi, hoho and huhu.

Comment 8

[IT Support]

Attachment: 1681824-fix.patch

Sorry, had to correct a comment.

Comment 9

[IT Support]

Comment on attachment 9192898 [details] [diff] [review]
1681824-fix.patch

We were looking at implementing cardForEmailAddress() of the Outlook address book and noticed a few problems with the overall approach. For example, the Mac address book seems to cache the entire address book content, so hasCard() can be done in a single lookup:
https://searchfox.org/comm-central/rev/5640a40e0d1d800e461050adadc46e29bdbe3982/mailnews/addrbook/src/nsAbOSXDirectory.mm#765

The Outlook address book attempts the same here:
https://searchfox.org/comm-central/rev/5640a40e0d1d800e461050adadc46e29bdbe3982/mailnews/addrbook/src/nsAbOutlookDirectory.cpp#200
However, mCardList doesn't seem to be consistently initialised, and is also cleared on a search in nsAbOutlookDirectory::Search() :-(

So the entire code needs to be carefully looked at. That made us think that we should start the overhaul right now and remove a whole heap of dead code. For example, none of the recursive restriction building/free'ing is needed since the only restriction we'll ever run is PR_ANR. BTW, most of the code has already switched to Unicode _W attributes, see:
https://searchfox.org/comm-central/rev/5640a40e0d1d800e461050adadc46e29bdbe3982/mailnews/addrbook/src/nsAbOutlookDirectory.h#119
so it's really unclear why the search, that never worked, was done with the _A-type attributes.

Comment 10

[IT Support]

Attachment: 1681824-fix-radical.patch

So this removes all the unneeded and untested code. It works the same as the other patch, only with less code. There's a bit of clean-up thrown in for good measure, like renaming functions since we found that the C++ polymorphism only added to the confusion. We also cleaned up a few void* pointers.

Since there is only one restriction now, a whole lot of the dynamic tree building and destroying can go. The only restriction left is a "regular" variable. Makes memory management simpler, too.

Ben, I added the f+ for you here to kindly request to delay your array changes until this has been committed to the code base. Ideally this will go to Thunderbird 78 so the patch needs to apply there as well.

I won't supersede the original patch just now, so you can see the "light" versus the "radical" approach. Note that the "light" patch needs rebasing since I made more changes in bug 1680923.

Finally, we saw that cardForEmailAddress() is used quite a bit in the code base, so it would be good to have it for Outlook as well. There we'd have to decide whether to run a query or to get the value from the content in mCardList. But that's for another bug.

Comment 11

[IT Support]

Comment on attachment 9192943 [details] [diff] [review]
1681824-fix-radical.patch

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

::: mailnews/addrbook/src/nsAbOutlookDirectory.cpp
@@ +476,5 @@
>        do_QueryInterface(supports, &retCode);
>    NS_ENSURE_SUCCESS(retCode, retCode);
> +
> +  // Outlook can only query on one value. So get this value from the
> +  // PrimareEmail condition.

Sigh, typo, we'll fix it later.

Comment 12

[IT Support]

Attachment: 1681824-fix-radical.patch

Fixed comments.

Comment 13

[Geoff Lankow (:darktrojan)]

I've been trying to see if this works with the Windows Contacts folder instead of Outlook. I have the other two patches applied and get no results from .search with or without this patch. Even using the exact email address of a contact that appears in .childCards.

Would you see if you can make that work too? The set-up is the same but you use moz-aboutlookdirectory://oe/ as the pref value, and you should be able to have both at once.

Comment 14

[IT Support]

The "other two patches"? You mean from bug 1680923? And what else?

We were under the impression that "oe" is for Outlook Express? That uses .wab files and there appears to be code in nsWabAddressBook.cpp/h. Are you sure that Windows Contacts can be queried via MAPI? Anyway, we'll investigate that.

Comment 15

[Geoff Lankow (:darktrojan)]

(In reply to IT Support from comment #14)

The "other two patches"? You mean from bug 1680923? And what else?

And bug 1679525.

We were under the impression that "oe" is for Outlook Express? That uses .wab files and there appears to be code in nsWabAddressBook.cpp/h. Are you sure that Windows Contacts can be queried via MAPI? Anyway, we'll investigate that.

It once meant Outlook Express, yes, and it does use nsWabAddressBook, however most of the code is in nsAbOutlookDirectory. Calls to nsAbWinHelperGuard mapiAddBook(mAbWinType); could return nsMapiAddressBook or nsWabAddressBook. I've never really looked any deeper than that, but the code your changing here affects both.

Comment 16

[IT Support]

... but the code your changing here affects both.

Indeed. For both the Outlook address book and so-called "Windows Contacts", stored as .contacts XML files in C:\Users\xxx\Contacts we run nsAbWinHelper::GetContents() here:
https://searchfox.org/comm-central/rev/5640a40e0d1d800e461050adadc46e29bdbe3982/mailnews/addrbook/src/nsAbWinHelper.cpp#787
which makes a Restrict() call.

The problem with Outlook was that this call returned MAPI_E_TOO_COMPLEX since the restriction wasn't correct. After some investigation we found that only a restriction based on PR_ANR is permissible. Constructing that makes the Outlook search work.

If the change the pref from "op" to "oe" and now search "Windows Contacts", those a properly displayed, but cannot be searched. Debugging this we found that the Restrict() call always succeeds regardless what you pass in, either the wrong restriction without the patch applied, or a correct restriction that works for Outlook. We've also tried to restrict based on PR_DISPLAY_NAME_A and PR_EMAIL_ADDRESS_A, but the effect is always the same: The call succeeds and subsequently no cards are found.

We haven't found any publications on how to use MAPI together with the "Windows Contacts". All the articles we've seen refer to Outlook and suggest to search via property PR_ANR.

So there are two options here:
Use the "light" approach which leaves all the recursive restriction building/free'ing in place in the hope that some of the code can be used for querying "Windows Contacts". Or use the drastic patch that rips out anything not needed for Outlook.

Would you see if you can make that work too?

Sorry, can't make it work. After the .Restrict() call made for "Windows Contacts", nothing is returned any more. There's no documentation :-(

Comment 17

[IT Support]

Attachment: 1681824-fix-radical.patch

For now, we've updated the "radical" patch with a comment documenting the findings. There was also an error check missing which we've added.

Please let us know how you'd like to proceed. We can also refresh the "light" version although it may be unlikely that the Restrict() approach will ever work for "Windows Contacts".

Comment 18

[IT Support]

OK, we've done a bit more research on this. Supporting "Windows Contacts" though WAB/MAPI is like we're saying in Australia difficult, no offence intended. Here's why:

First we turn to
https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wab/-wab-overview
Quote: New applications should not use this set of interfaces. These interfaces exist for backward compatibility with legacy applications. These interfaces will be unavailable in the future.
Note: In Windows Vista, Windows Contacts replaces Windows Address Book (WAB).
So in other words: WAB has been mostly discontinued. The documentation is still there:
https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wab/-wab-interfaces
Quote: IMAPITable - Do not use.
https://docs.microsoft.com/en-us/windows/win32/api/wabdefs/
Quote: SRestriction - Do not use. Describes a filter for limiting the view of a table to particular rows.

Also not the "previous-versions" as part of the URL with the general disclaimer (quote): We're no longer updating this content regularly. Check the Microsoft Product Lifecycle for information about how this product, service, technology, or API is supported. Windows Vista is from 2006.

So while some functions still seem to work, setting a restriction doesn't. So in our opinion it's not recommendable to pursue access to "Windows Contacts" through WAP/MAPI any more. If access to "Windows Contacts" is desired in Thunderbird, then the new set of API should be used:
https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wincontacts/-wincontacts-entry-point
https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wincontacts/-wincontacts-example-entry
However, turning to
https://docs.microsoft.com/en-us/previous-versions/windows/desktop/wincontacts/-wincontacts-reference
we see a lot of (quote) "Do not use" - so once again, it's doubtful whether this technology is future proof.

In our experience we haven't see use of "Windows Contacts" in real life. We've just configured "Mail", Window's "native" e-mail client. We have no experience with that application, but as far as we could see, the "Windows Contacts" don't show there. Maybe you can confirm that.

In summary: We think the patch for Outlook should go ahead. If supporting "Windows Contacts" in the future is desirable, and whether and how it would be supported, is unclear.

Comment 19

[IT Support]

Comment on attachment 9193014 [details] [diff] [review]
1681824-fix-radical.patch

As per the previous comment, we think that not being able to search "Windows Contacts" for the "OE" provider shouldn't stop the repair work for Outlook.

Comment 20

[Geoff Lankow (:darktrojan)]

Comment on attachment 9193014 [details] [diff] [review]
1681824-fix-radical.patch

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

Okay, I'm happy with this. It's a shame that Outlook's incapable of doing a proper search, but I guess they have no real incentive to help competitors out. As for Windows Contacts … well there wasn't much of any value there anyway. Looks like it's for the chopping block.

Thanks for your efforts here. I suspect a number of the things you've run into are why this code had been largely abandoned.

Comment 21

[IT Support]

Thanks. Maybe you can ping (your colleague?) Ben to get this on the way for committing to the code base. It would be nice to see it in Thunderbird 78 soon.

When you do your overhaul (we might be able to assist with testing or suggestions), you might want to look into the significance of "Windows Contacts".

As mentioned earlier, we'll look into implementing cardForEmailAddress() in another bug. We haven't done much testing related to editing/creating/deleting contacts or mailing lists, so we'll take a look while we're there. Some editing operation seem to have been successful. Our main aim here is to investigate whether Thunderbird can be recommended to Outlook users who might want to try it out in parallel on an IMAP setup. Being able to access contacts in that situation is a plus. As mentioned before, a stable ID for an entry (EntryID) is already provided in Thunderbird, see bug 1680952 comment #13. That's also a bonus. We're still researching whether any part of the EntryID is a GUID usable for the card's UID.

Comment 22

[IT Support]

Comment on attachment 9192898 [details] [diff] [review]
1681824-fix.patch

(No need for the "light" approach any more. Outlook can only search via PR_ANR and "Windows Contacts" via WAP are likely to be discontinued.)

Comment 23

[IT Support]

It's a shame that Outlook's incapable of doing a proper search ...

Actually, the PR_ANR search is pretty good as you can see in attachment 9192893 [details]. It's achieving with one "fuzzy pseudo-property" what Thunderbird does by looking at many properties of the card. As per the MS documentation quoted above, the PR_ANR property was created for best user experience.

Comment 24

[IT Support]

Ben, we've added the f? for you here to kindly request to delay your array changes until this has been committed to the code base.

Maybe that feedback isn't required. We just wanted to make aware of the changes planned here. We'll leave it in your hands.

Comment 25

[Geoff Lankow (:darktrojan)]

(In reply to IT Support from comment #23)

It's a shame that Outlook's incapable of doing a proper search ...

Actually, the PR_ANR search is pretty good as you can see in attachment 9192893 [details]. It's achieving with one "fuzzy pseudo-property" what Thunderbird does by looking at many properties of the card. As per the MS documentation quoted above, the PR_ANR property was created for best user experience.

Thunderbird lets the user do advanced address book searches which is why the weird Lisp-style string and all of the mechanism to support it exists. I can't see the benefit myself but somebody must be using it as there were complaints when I broke it (accidentally, of course!). Most of the time though what Outlook can do is absolutely fine, as you say.

Ben is away today. I'll leave the flag behind but let's ship this.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8774b97638ee
Fix Outlook address book search. Only search PR_ANR_W and convert to wide string. r=darktrojan

Comment 27

[Ben Campbell]

Comment on attachment 9193014 [details] [diff] [review]
1681824-fix-radical.patch

Just took a quick look over the patch and it goods good to me, so adding my f+ and big thanks for digging into this stuff!

Comment 28

[Magnus Melin [:mkmelin]]

I'll note that the future support of these proprietary ab providers type is... not clear. We have discussed potentially dropping the OSX one in favour of iCloud CardDAV access (which would provide read-write, not just read), and ripping out the old junk in the LDAP cpp, implementing only the parts we need in js.
The Outlook address book would then be the last of the dinosaurs and as it's never really been too much exposed... It's all just plans yet of course. We'll first have to see how CardDAV works out.

Comment 29

[IT Support]

The MacOS address book integrates access to contacts in iCloud, Exchange, Google, Yahoo, AOL and a few others. Do you really intend to remove that functionality?

Comment 30

[Magnus Melin [:mkmelin]]

iCloud, Google, Yahoo, AOL can all be accessed through standard CardDAV, and note, writable, which our native MacOS address book is NOT. That's pretty bad for UX: you write the mail, but then you still don't have it accessible when you need it on another device.
AFAIK, the MacOS addressbook support we have do not give you access to contacts synced from other places either, it's just the local addresses, no?

Nothing has yet been decided, and there' s no timeline, since we just CardDAV enabled. Still, I think IF things work out well, the writing is on the wall: seems to me pretty much pointless keeping that support if better (writable!!) support can be obtained though very standard means. The amount of work it's caused over the years is rather significant.

Comment 31

[IT Support]

Attachment: Mac Exchange AB.png

AFAIK, the MacOS addressbook support we have do not give you access to contacts synced from other places either, it's just the local addresses, no?

That is not the case. Our colleagues use the Mac AB and the addresses are coming from Exchange. In the picture you can see the Mac Contacts from Exchange (above) and Thunderbird address entry (below, with mail.autoComplete.commentColumn set to 1).

The Outlook address book is writeable, but MAPI programming is a lot of pain.