430614 - [GSoC] Thunderbird integration into Windows Vista/Windows Search indexer

Status: RESOLVED FIXED

(Thunderbird :: Search, enhancement, P2)

Description

[Rain [:rain1] (inactive)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier: version 2.0.0.12 (20080213)

So far, users of Microsoft's Windows Vista search engine and Windows Search product (for older versions of Windows) have had to use a Microsoft email client or an out-of-date closed source third party tool (which works only with old versions of Windows Search, and does not work at all with Windows Vista) [1] to enable their emails to be indexed.

The aim of this project is to develop an open source plugin to allow these search engines to index Thunderbird emails.

[1] http://www.citeknet.com/Products/ProtocolHandlers/WDSThunderbirdMozillaEudoraMailAddin/tabid/70/Default.aspx

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Confirming due to approved GSoC work.

Comment 2

[Rain [:rain1] (inactive)]

The original proposal was to use the mbox files directly and serve them, along with some metadata stored in an SQLite database, to the indexer.

However there are concurrency issues with this approach. (Here COM means the Windows Search COM component)

The SQLite database and mbox file will be accessed from both Tb and COM. The SQLite database should work out to be thread-safe, returning errors if opened concurrently in both TB and COM (and I'll be able to handle the error). How will the mbox be handled, though? The closed source third party addin does have the same issue.

One way might be to check for parent.lock in COM, but that would mean that people who keep Tb open all the time wouldn't ever have their mails indexed. Also, this only resolves the issue in one direction.

Another alternative is the shadow copy feature that Microsoft provides on NTFS in Windows XP and Vista [1]. I'll have to see whether this can work with Windows Search at all. This will also make the code not work on FAT32 partitions. (It actually sounds like overkill)

[1] http://msdn2.microsoft.com/en-us/library/aa384648(VS.85).aspx

So it might be that writing out all mails into individual files is the only workable, failproof solution with the mbox format.

Does anybody have any other ideas how this might be resolved?


Last year's SoC had a project on Tb integration with Beagle (mentored by Beagle). The way chosen was individual files per email. The sources are available at [2].

[2] http://svn.gnome.org/svn/beagle/trunk/beagle/thunderbird-extension/

What I want to do and what is done in the extension are quite similar -- this can definitely be ported over with a few changes. The license is MIT, so not a problem.

If this is the way chosen, I can add support for contacts, and possibly even backport changes to the Beagle extension.

Comment 3

[Rain [:rain1] (inactive)]

Unfortunately, it seems as if Tb doesn't use any locks apart from parent.lock. This, coupled with the fact that mbox isn't really designed for concurrent access, means that we just might have to write out mails individually. Does anyone have any idea how else this could be resolved? Could any method using last modification dates be workable?

(Ah, maildir would be a whole lot easier to work with)

Comment 4

[Rain [:rain1] (inactive)]

The Spotlight plugin by David Bienvenu [1] seems a great choice to port.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=290057

The messages are first read (in Javascript, using streamMessage) as RFC822, then converted to UTF-8. We can skip the conversion and store the RFC822 form itself.

The way results are handled is also good: the files are named by their message IDs. When a result is selected, the message ID is extracted, and the correct message is then selected.

According to David we should use the same approach for Windows Search, as we don't want any platform-dependent code. I think so too.

My first job will be to port the Spotlight component with all the necessary changes.

Comment 5

[Rain [:rain1] (inactive)]

I'm posting somewhat regular updates to the bug status to http://sid0.blogspot.com .

Comment 6

[Rain [:rain1] (inactive)]

Attachment: nsWinSearchIntegration component (also enables nsSpotlightIntegration on Mac)

This is a rough (but working) version of the Windows Search component (derived from the Spotlight component), as part of the midterm evaluation (along with the bugs this one depends on) -- several things still need to be automated and refined.

The code's probably a bit painful to look at right now :)

Comment 7

[Rain [:rain1] (inactive)]

Attachment: Reg file to add .wdseml support to the index

This will of course be automated for the final evaluation.

Comment 8

[Rain [:rain1] (inactive)]

Attachment: Snapshot of work so far

This "unforks" come of the common code between nsWinSearchIntegration.js and nsSpotlightIntegration.js, moving it to content/searchCommon.js.

Comment 9

[Rain [:rain1] (inactive)]

Attachment: fixed license block

eh, sorry for that.

Comment 10

[Rain [:rain1] (inactive)]

Attachment: Code combined between the two components

There are several things still to be fixed, but this is in a working state and I'd like to see this landed after a2 so that we can build on it.

Since this is "turned off" by default, it shouldn't have an effect by default. One effect will be for Mac users with mail.spotlight.enable turned on (carried over from Tb2) -- they'll start seeing generated files again.

Comment 11

[Jeff Beckley]

Comment on attachment 329499 [details] [diff] [review]
Code combined between the two components

> mail/components/search/nsSpotlightIntegration.js:

Mode-line should be 2 for tabs and offset.


> const Cc = Components.classes;
> const Ci = Components.interfaces;

Don't need these as they are in searchCommon.js.  Also, there's quite a bit of pre-existing code that doesn't use the consts that could benefit in readability by using them.  It would also help to better see similarities between this file and nsWinSearchIntegration.js so as to notice when things can be combined in to searchCommon.js.


> Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> 
> var gPrefBranch = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefService).getBranch(null);

These are in both files, so can be moved to searchCommon.js.


> var gIndexMsgsToSpotlight;

Why is this a global when it's only used in one function?


>   try {
>     gIndexMsgsToSpotlight = gPrefBranch.getBoolPref(gPrefBase + ".enable");
>     gLastFolderIndexedUri = gPrefBranch.getCharPref(gPrefBase + ".lastFolderIndexedUri");
>   } catch (ex) {}
>   if (!gIndexMsgsToSpotlight)
>     return;

This code could be moved in to the base searchCommon.js just using a local variable in place of gIndexMsgsToSpotlight.

Since there will be nothing left in Init{Spotlight,WinSearch}Integration() but the SIDump() call, you could make the message to dump be a parameter to InitSupportIntegration


> var gStreamListener = {

This object shares about half of its code between the two versions.  It would make sense to make a prototype object in searchCommon.js, and then derive a new class in each of the two files.  

> /* XPCOM boilerplate code */

This code is almost all shared as well.  The only differences are the description, class ID, and contact ID, which could be passed in as parameters to the constructor.  The SIDump call could use the description for its output.


All of these changes are pretty simple, except for the gStreamListener one, which is a little more involved.  So if you're wanting to get this checked in soon you could wait to do that until later.  With the simple changes, r=me.

Comment 12

[David :Bienvenu]

> const Cc = Components.classes;
> const Ci = Components.interfaces;

this actually breaks spotlight because these are redeclared. I'll try removing them and see if spotlight integration starts working.

Comment 13

[Rain [:rain1] (inactive)]

(In reply to comment #11)
> 
> This code could be moved in to the base searchCommon.js just using a local
> variable in place of gIndexMsgsToSpotlight.
> 
> Since there will be nothing left in Init{Spotlight,WinSearch}Integration() but
> the SIDump() call, you could make the message to dump be a parameter to
> InitSupportIntegration

I'd like that code to remain as it is, because the plan is to add to InitWinSearchIntegration, adding support for first-run, testing whether the locations have been included, and so on.

>
> > /* XPCOM boilerplate code */
> 
> This code is almost all shared as well.  The only differences are the
> description, class ID, and contact ID, which could be passed in as parameters
> to the constructor.  The SIDump call could use the description for its output.

Well, that is already a shortened version of the usual boilerplate code, and uses XPCOMUtils, so I like to think of that block as one unit. I'm not sure separating the boilerplate code into three parts -- ns{WinSearch,Spotlight}Integration.js, searchCommon.js, and XPCOMUtils.jsm, would be the best idea.

I've fixed the remaining nits locally (sorry about the redeclaration, David).

Comment 14

[David :Bienvenu]

Comment on attachment 329499 [details] [diff] [review]
Code combined between the two components

no need for local var here:

+    var msgHdr = gMsgHdrsToIndex[0];
+    GenerateSupportFile(msgHdr);

same here:

+      var msgHdr = aMsgs.queryElementAt(i, Ci.nsIMsgDBHdr);
+      var file = GetSupportFileForMsgHdr(msgHdr);

We're on the trunk now, can you try uncommenting out the code here, and not use encodeURIComponent?

+    // this should work on the trunk, but not in 2.0
+//    messageId = netUtils.escapeString(messageId, 3 /* netUtils.ESCAPE_URL_PATH */);

this was a totally arbitrary choice on my part - maybe we should remove it, or increase it. What do you think? :
+    // ignore stuff after the first 20K or so
     if (this.message && this.message.length > 20000)
       return 0;

again, no need for local var msgHdr.
+  if (gMsgHdrsToIndex.length > 0)
+  {
+    var msgHdr = gMsgHdrsToIndex[0];
+    GenerateSupportFile(msgHdr);
+  }

why isn't the stream listener shared in the common code?

sr=me, with the above addressed, along with the searchCommon folder iteration changes I sent you...

Comment 15

[Rain [:rain1] (inactive)]

Attachment: [checked in] Addressed review comments, merged folder iteration changes

Thanks for the fixes David.

Carrying forward r/sr.

Comment 16

[Mark Banner (:standard8)]

Checking in mail/components/Makefile.in;
/cvsroot/mozilla/mail/components/Makefile.in,v  <--  Makefile.in
new revision: 1.14; previous revision: 1.13
done
Checking in mail/components/search/Makefile.in;
/cvsroot/mozilla/mail/components/search/Makefile.in,v  <--  Makefile.in
new revision: 1.3; previous revision: 1.2
done
Checking in mail/components/search/nsSpotlightIntegration.js;
/cvsroot/mozilla/mail/components/search/nsSpotlightIntegration.js,v  <--  nsSpotlightIntegration.js
new revision: 1.3; previous revision: 1.2
done
RCS file: /cvsroot/mozilla/mail/components/search/nsWinSearchIntegration.js,v
done
Checking in mail/components/search/nsWinSearchIntegration.js;
/cvsroot/mozilla/mail/components/search/nsWinSearchIntegration.js,v  <--  nsWinSearchIntegration.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mail/components/search/content/searchCommon.js,v
done
Checking in mail/components/search/content/searchCommon.js;
/cvsroot/mozilla/mail/components/search/content/searchCommon.js,v  <--  searchCommon.js
initial revision: 1.1
done

Comment 17

[Biju]

I started using Mozilla Mail (later Tb) instead of Outlook to reduce integration with OS.

So please ensure I can disable this feature when implemented.

Comment 18

[Rain [:rain1] (inactive)]

(In reply to comment #17)
> I started using Mozilla Mail (later Tb) instead of Outlook to reduce
> integration with OS.
> 
> So please ensure I can disable this feature when implemented.
> 

Biju: this will probably be an opt-in feature.

Comment 19

[Rain [:rain1] (inactive)]

Attachment: [checked in] Make the file generation code use getMsgTextFromStream

There are several advantages to doing so -- this reduces file size, gets only relevant parts, and in case the message fails to download, will at least generate the headers.

Comment 20

[Mark Banner (:standard8)]

Comment on attachment 332525 [details] [diff] [review]
[checked in] Make the file generation code use getMsgTextFromStream

Checked in, changeset id 64:9d26fe956df7

Comment 21

[Rain [:rain1] (inactive)]

Attachment: [WIP] Support C++ component, UAC-aware enabler and first run UI

Most of it's done, I'm just not satisfied with the UI right now. (The first run dialog, that's non-modal.)

Right now it pops up on first run, with the new account dialog stealing focus immediately after. The way to resolve this seems to be to include code in msgMail3PaneWindow.js to initialize the component only after the new account dialog has been closed. I had wanted to make the component not depend on any other code in mail/, but that seems to be inevitable now.

Asking for review for suggestions. David and Bryan, what do you think?

Comment 22

[Rain [:rain1] (inactive)]

Bryan, what do you think about merging the default client dialog and this one? This will avoid two dialogs. The dialog can be shown either-or, and elements can be hidden by default and made visible as needed.

The default client dialog can now be called the "Operating System Integration" dialog, I guess.

Comment 23

[David Ascher (:davida)]

It might be good to get this into b1, before Sid turns into a student again.

Comment 24

[Rain [:rain1] (inactive)]

(In reply to comment #23)
> It might be good to get this into b1, before Sid turns into a student again.
> 

I'm sure most of the code will be ready for the b1 freeze, but I'm not sure bug 447842 will be resolved before then, and I'd like the code to use that.

There's a startup UI issue right now... the startup is really not structured as well as it could be.

Comment 25

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #22)
> Bryan, what do you think about merging the default client dialog and this one?
> This will avoid two dialogs. The dialog can be shown either-or, and elements
> can be hidden by default and made visible as needed.
> 
> The default client dialog can now be called the "Operating System Integration"
> dialog, I guess.

Sid, that sounds like a good plan lets try that as the dialog point.  I think we'll just need a little restructuring of the dialog, no need to hide or show elements.  We can probably just use checkboxes to show what's on and what's off.

Comment 26

[Rain [:rain1] (inactive)]

Attachment: Fixes + integration with Windows + UI

This patch combines:

- Several fixes to the indexing code, including a backoff mode, meant to operate when the integration is disabled
- Quite a bit of integration with Windows, including a C++ component to help and a small executable, meant to run elevated
- A basic UI to work at first run -- this is meant to be combined with the default client dialog later

Comment 27

[Rain [:rain1] (inactive)]

Comment on attachment 335162 [details] [diff] [review]
Fixes + integration with Windows + UI

Bryan: since the merging of the default client and the search integration is not a minor undertaking, what do you think of having a separate dialog for now, and then building the merged UI on top of this?

Comment 28

[Rain [:rain1] (inactive)]

Comment on attachment 335162 [details] [diff] [review]
Fixes + integration with Windows + UI

Jim: Thanks for consenting to review the patch.

To clarify: WSEnable.exe, built by WSEnable.cpp, is the program that runs with elevated privileges to add the integration. The elevation point is when the "Enable Windows Search" button is clicked. The elevation is done only if required (if either the folders are not in the Windows Search indexing scope, or required registry entries are missing). I'm still looking at a way to get the shield icon to display next to the button.

Comment 29

[Simon Bünzli]

(In reply to comment #26)
> - A basic UI to work at first run -- this is meant to be combined with the
> default client dialog later

Couldn't we do without any UI at all (or at most a pref in the advanced options)? As long as we can detect whether Windows Search is installed or we're on Vista, we could just enable integration when that's the case and use a hidden pref for disabling auto-detection/integration.

Comment 30

[Rain [:rain1] (inactive)]

(In reply to comment #29)
> (In reply to comment #26)
> > - A basic UI to work at first run -- this is meant to be combined with the
> > default client dialog later
> 
> Couldn't we do without any UI at all (or at most a pref in the advanced
> options)? As long as we can detect whether Windows Search is installed or we're
> on Vista, we could just enable integration when that's the case and use a
> hidden pref for disabling auto-detection/integration.
> 

The trouble with that is that enabling or disabling the integration requires a UAC prompt. (Try adding file locations through the UI on Vista.) It's extremely jarring for the user to be greeted with a UAC prompt on first run without warning.

Windows Mail, which uses a method quite similar to what we're doing here, doesn't have a problem because it doesn't support multiple profiles in custom locations.

Comment 31

[Jeff Beckley]

Comment on attachment 335162 [details] [diff] [review]
Fixes + integration with Windows + UI

Looks good.  Just some minor comments below.  With that r=me.


mail/components/search/content/searchIntegrationDialog.js:

Why have this file at all?  Just have searchIntegrationDialog.xul call window.arguments[0].callback() in ondialogaccept and ondialogcancel.  You can then also remove the line from mail/components/search/jar.mn referring to searchIntegrationDialog.js.


mail/components/search/nsMailWinSearchHelper.cpp:
>  ISearchManager* searchManager;

Make that a nsRefPtr<ISearchManager> and you won't have to worry about calling Release() on it.


>    subdir->AppendNative(relativeStr);
>    subdir->GetPath(subdirPath);

Need to check for errors here.


>NS_IMETHODIMP nsMailWinSearchHelper::SetFANCIBit(nsIFile* aFile, PRBool aBit, PRBool aRecurse)
...
>  aFile->Exists(&exists);
...
>  aFile->GetPath(filePath);
...
>  aFile->IsDirectory(&isDirectory)
...
>    aFile->GetDirectoryEntries(getter_AddRefs(children));

You've got several calls on aFile that you don't check to see if they failed or not.	


>      nsCOMPtr<nsISupports> childSupports;
>      children->GetNext(getter_AddRefs(childSupports));
>      nsCOMPtr<nsIFile> childFile(do_QueryInterface(childSupports));

You can actually bypass the indirect ISupports and go straight to the interface you want.  You also should check for failure here as well.  Would look like this:

      nsCOMPtr<nsIFile> childFile;
      rv = children->GetNext(getter_AddRefs(childFile));
      NS_ENSURE_SUCCESS(rv, rv);


>  IApplicationAssociationRegistration *pAAR;

nsRefPtr<> this to ensure release.  There's a couple of these in the file.  I realize that the usage of these is pretty simple, but it's good to just make a habit of using the wrapper classes, and could come in to play if the code gets updated later and becomes more complicated.


>    mCurProcD->Append(NS_LITERAL_STRING("WSEnable.exe"));

Similar here as to above, there's a few calls in mCurProcD and mProfD that don't check for failure.


>  HWND hWnd = GetForegroundWindow();

GetForegroundWindow() can return NULL.  I can't imagine that would cause a problem for the UAC prompt, since it's global to all windows, but might be worth testing to see what it does.


mail/components/search/nsWinSearchIntegration.js:

>  var enabled;
>  try {
>    enabled = gPrefBranch.getBoolPref(gPrefBase + ".enable");
>    gLastFolderIndexedUri = gPrefBranch.getCharPref(gPrefBase + ".lastFolderIndexedUri");
>  } catch (ex) {}
...
>  if (enabled === undefined)
>    // First run has to be handled after the main mail window is open
>    return true;

I think that comment needs to be expanded upon.  It took me a little bit to figure out what exactly what was going on.  enabled will only be undefined if the pref is not defined (exception thrown on the getBoolPref call), and the setting of that pref happens in a different function.


mail/components/search/wsenable/WSEnable.cpp:

>static const int sFolderCount = 3;
>static const int sRegKeyCount = 5;

You can use the _countof () macro instead of putting hand-counted values in there.


>  ISearchManager* searchManager;
>    ISearchCatalogManager* catalogManager;

I don't know if you want to bring ATL in to this (maybe it is already?), but you could use the CComPtr<T> class and stop worrying/potentially forgetting about releasing.


>        if (wcscmp(argv[0], L"1") == 0)
>        else if (wcscmp(argv[0], L"0") == 0)

How about the more simple *argv[0] == L'1' and *argv[0] == L'0'?

Comment 32

[David :Bienvenu]

Comment on attachment 335162 [details] [diff] [review]
Fixes + integration with Windows + UI

very nice. sr=bienvenu, with some nits:

+  HRESULT hr = HRESULT_FROM_WIN32(dwRet);
+  if (FAILED(hr))
+    return NS_ERROR_FAILURE;
+
+  return NS_OK;
+}
 maybe :
return (FAILED(HRESULT_FROM_WIN32(dwRet)) ? NS_ERROR_FAILURE : NS_OK;

or the more positive SUCCEEDED(...) :-)

searchIntegrationDialog.xul
shouldn't that just be 2008 and initial developer you? Or did you borrow it from somewhere?

+    regKey.close();
+    if (!valuePresent) return false;

the return should be on its own line.

Since I'm not on Vista, I can't really test this out, right?

Comment 33

[Rain [:rain1] (inactive)]

(In reply to comment #32)
> 
> searchIntegrationDialog.xul
> shouldn't that just be 2008 and initial developer you? Or did you borrow it
> from somewhere?

I did, from the default client dialog XUL file. :)

> Since I'm not on Vista, I can't really test this out, right?
> 

Yes, that's right. However it should build successfully, whether you're using the Vista SDK or not (one should have |ac_add_options --disable-vista-sdk-requirements| in mozconfig in case one doesn't have the Vista SDK. I disable the whole search/ directory for Windows in that case.)

I've also asked Jim to review it, to cover the Vista specific parts.

Comment 34

[Jim Mathies [:jimm]]

Looks like there's a few places in GetFoldersInCrawlScope where you're not cleaning up ISearchManager* searchManager after you're finished. You released it on one failure case but missed it for the rest of the method.  

> pAAR->QueryAppIsDefault(L".wdseml", AT_FILEEXTENSION, AL_EFFECTIVE, L"Thunderbird", aResult);

Let's put the app name up on top of the file as we do in the other app registration code so it's easy to find. Please use the same name so it can be found in a search.

Looks pretty tight overall. You're doing a lot of registration of app / file / mime types but only removing a single file association on uninstall. Is there any way we can reset some of that stuff (like the default app) on uninstall?

Comment 35

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 335162 [details] [diff] [review]
Fixes + integration with Windows + UI

Looks good for now.  We've already talked quite a bit about getting the default client and search integration dialog merge after this initial piece gets in.

Comment 36

[Rain [:rain1] (inactive)]

Thanks for reviewing, Jim.

(In reply to comment #34)
> Looks like there's a few places in GetFoldersInCrawlScope where you're not
> cleaning up ISearchManager* searchManager after you're finished. You released
> it on one failure case but missed it for the rest of the method.

OK, I've switched to nsRefPtr anyway.

> 
> > pAAR->QueryAppIsDefault(L".wdseml", AT_FILEEXTENSION, AL_EFFECTIVE, L"Thunderbird", aResult);
> 
> Let's put the app name up on top of the file as we do in the other app
> registration code so it's easy to find. Please use the same name so it can be
> found in a search.

OK.

> 
> Looks pretty tight overall. You're doing a lot of registration of app / file /
> mime types but only removing a single file association on uninstall. Is there
> any way we can reset some of that stuff (like the default app) on uninstall?

I think the .wdseml extension removal *should* take care of most of the additions. There's one more in KindMap that should be taken care of, but isn't right now.

As for indexed locations, I'm not sure about how to handle them on uninstall. How do you figure out all the profiles for each user and call the function for each of them? The user might just also have another install of Tb (I'm not sure how common this is), in which case he wouldn't like the locations to be de-registered.

Comment 37

[Jim Mathies [:jimm]]

If the data sticks around, leaving them in there sounds fine. r+ with those other changes.

Comment 38

[Rain [:rain1] (inactive)]

Attachment: [checked in] addressed review comments

Carrying forward reviews by beckley and jmathies, ui-review by clarkbw and sr by bienvenu.

Comment 39

[Mark Banner (:standard8)]

Comment on attachment 337314 [details] [diff] [review]
[checked in] addressed review comments

Checked in, changeset id: 279:e7a685e6761a

Comment 40

[Rimas Kudelis [use needinfo to ping]]

I can't see anything related to Windows Search in interface of pre-b1 builds I'm using; currently this: 
Mozilla/5.0 (Windows; U; Windows NT 5.1; lt; rv:1.9.1b1pre) Gecko/20080916030709 Shredder/3.0b1pre

Is this feature enabled in these builds already? I'm running XP with SP3 and WS4.0...

Comment 41

[Rain [:rain1] (inactive)]

(In reply to comment #40)
> I can't see anything related to Windows Search in interface of pre-b1 builds
> I'm using; currently this: 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; lt; rv:1.9.1b1pre)
> Gecko/20080916030709 Shredder/3.0b1pre
> 
> Is this feature enabled in these builds already? I'm running XP with SP3 and
> WS4.0...

It's Vista only for now.

Comment 42

[Rimas Kudelis [use needinfo to ping]]

Then when will it work in XP? I hope it will make it into the final version, or maybe even one of the betas?

Comment 43

[Rain [:rain1] (inactive)]

(In reply to comment #42)
> Then when will it work in XP? I hope it will make it into the final version, or
> maybe even one of the betas?

The problem with the XP version of Windows Search is that it lacks a property handler for MIME messages. On Vista, you can search and sort by sender, subject, date received, etc, and have them displayed neatly as columns in your search. (We use the Microsoft provided handler for this.) The XP version doesn't actually have one. Notice that on XP, you can't use Windows Search to properly search through Windows Live Mail messages either. You have to do it from within the WLM interface.

Comment 44

[Rimas Kudelis [use needinfo to ping]]

I think that even searching through just body would benefit.

On the other hand, I just searched for my name, and between the results came out a few Word documents that only contain my name in metadata (the Author tag). Is it possible in XP to at least index those tags, even if they wouldn't be displayed?

Comment 45

[Rain [:rain1] (inactive)]

(In reply to comment #44)
> I think that even searching through just body would benefit.
> 
> On the other hand, I just searched for my name, and between the results came
> out a few Word documents that only contain my name in metadata (the Author
> tag).

That's because the XP version has a property handler for Word documents. It doesn't have one for MIME messages.

> Is it possible in XP to at least index those tags, even if they wouldn't
> be displayed?

Yes, that's what currently happens on XP. (It's slightly confusing, but XP has a filter for MIME messages, but not a property handler. The filter does the indexing, the property handler handles the display of results.) The check for Vista is hardcoded at the moment, but I guess it can be changed to possibly use a hidden pref instead to make it work on XP.

Comment 46

[Rimas Kudelis [use needinfo to ping]]

Well, like I said, I think that even if we index/display only the body part, it would still be a benefit over not indexing at all for XP users.

Perhaps that check could be removed at all, with an appropriate note added in release notes and/or the UI that enables this feature?

Comment 47

[Bryan Clark (DevTools PM) [:clarkbw]]

> with an appropriate note added in
> release notes and/or the UI that enables this feature?

If we haven't already, we should probably make a clear note on the release notes that this is Vista only.

Comment 48

[Rain [:rain1] (inactive)]

As discussed with Bryan, a better UI is needed. Pushing to beta 2 for the UI.

Comment 49

[RNicoletto]

Is this implementation valid for Copernic Desktop Search?

Comment 50

[Rain [:rain1] (inactive)]

(In reply to comment #49)
> Is this implementation valid for Copernic Desktop Search?

No.

Comment 51

[Rain [:rain1] (inactive)]

Attachment: [checked in] A couple more fixes

- I'd forgot to include the xpt
- Doh, need to unset the File Attribute Not Content Indexed bit for the profile directory and all subdirectories.

Comment 52

[Mark Banner (:standard8)]

Comment on attachment 339997 [details] [diff] [review]
[checked in] A couple more fixes

Checked in, changeset id: 406:1f2f13351b1f

Comment 53

[Wayne Mery (:wsmwk)]

Can someone who has used vista search write one or more tests for litmus? If you don't have admin access to litmus I'll gladly take your text and create the test.

Nothing verbose, just list a few steps of what testers should do and what they should see as a result. Perhaps 4 distinct tests?
1. enable search (and/or what they see on first startup after install)
2. <a test to verify that it's working correctly>
3. <a test checking for a problem that has already fixed by development, i.e. should NOT be seen>
4. disable search

Comment 54

[Rain [:rain1] (inactive)]

(In reply to comment #53)
> Can someone who has used vista search write one or more tests for litmus? If
> you don't have admin access to litmus I'll gladly take your text and create the
> test.
> 
> Nothing verbose, just list a few steps of what testers should do and what they
> should see as a result. Perhaps 4 distinct tests?
> 1. enable search (and/or what they see on first startup after install)
> 2. <a test to verify that it's working correctly>

There will probably be 4-5 tests that will be needed to verify that it is working correctly, since there are several related parts to the integration.

> 4. disable search

This hasn't been really implemented yet.

Comment 55

[Lee Houghton]

Hmm, it's not quite working properly for me. It does appear to have created the .wdseml files properly, but they're not all showing up in the start menu. This is on Vista SP1 - 64-bit, UAC turned off :)

When I search from the start menu, I can see messages from newsgroups but not messages from my IMAP folders. Additionally, the results show only the message ID (presumably from the file name), not the Subject or anything else.

When I search from explorer (Windows+F), I see messages from both IMAP folders and Newsgroups, but the From/Subject/Date Received/To Names are all blank!

Looking in the .wdseml files, they do look valid. I can attach one if needed, though. The indexing control panel shows that wdseml files are being processed with the MIME filter. It's currently rebuilding the index at my behest, but it looks like the problem is still occurring. After copying one of the results to another of my indexed folders and starting rebuilding the index, it also shows up with blank properties.

This is starting to look like it's Windows's fault, but I should probably mention it here anyway in case it's useful.

Comment 56

[Rain [:rain1] (inactive)]

Interesting -- could you confirm if HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\PropertySystem\\PropertyHandlers\\.wdseml (those should be single backslashes, of course) has its "default" value {5FA29220-36A1-40f9-89C6-F4B384B7642E}? It'd also be great if you could ensure that ..\\PropertyHandlers\\.eml has the same default value. It seems like the property handler isn't kicking in.

Please do attach a .wdseml file which isn't working for you.

Comment 57

[Rain [:rain1] (inactive)]

OK, now I'm thinking it's a Windows 64-bit problem, and that some registry locations have been virtualized because the Windows Search enabler is a 32-bit program. Thanks for the bug report!

Comment 58

[Rain [:rain1] (inactive)]

Attachment: [checked in] Fix for 64-bit Windows

per http://msdn.microsoft.com/en-us/library/aa384129(VS.85).aspx

We also disable CheckRegistryKeys as that won't work on 64-bit Windows right now. As a result, we'll just harmlessly elevate every time we're enabled.

Comment 59

[David :Bienvenu]

Comment on attachment 341893 [details] [diff] [review]
[checked in] Fix for 64-bit Windows

can we file a follow up bug to handle the ToDo items and mark it in b1?

Comment 60

[Mark Banner (:standard8)]

Comment on attachment 341893 [details] [diff] [review]
[checked in] Fix for 64-bit Windows

Checked in, changeset id: 526:e1a4229ff535

Also checked in for Shredder alpha 3 respin (a=Standard8,bienvenu), changeset id: 490:504bc8871087

Comment 61

[Rain [:rain1] (inactive)]

(In reply to comment #59)
> (From update of attachment 341893 [details] [diff] [review])
> can we file a follow up bug to handle the ToDo items and mark it in b1?

Yes, that's a good idea. I'll get to this, possibly today.

Comment 62

[Lee Houghton]

(In reply to comment #56)
> Interesting -- could you confirm if
> HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\PropertySystem\\PropertyHandlers\\.wdseml
> (those should be single backslashes, of course) has its "default" value
> {5FA29220-36A1-40f9-89C6-F4B384B7642E}? It'd also be great if you could ensure
> that ..\\PropertyHandlers\\.eml has the same default value. It seems like the
> property handler isn't kicking in.
> 
> Please do attach a .wdseml file which isn't working for you.

This key didn't exist. You guessed right, though, it was there under
Wow6432Node. It's working with the key added, thanks!

I'm not sure how to disable the indexing so that I can enable it again and verify that the patch works. (It would be nice if it showed up in the Indexing Options control panel, too :)

Comment 63

[Rain [:rain1] (inactive)]

(In reply to comment #62)
> 
> This key didn't exist. You guessed right, though, it was there under
> Wow6432Node. It's working with the key added, thanks!
> 
> I'm not sure how to disable the indexing so that I can enable it again and
> verify that the patch works. (It would be nice if it showed up in the Indexing
> Options control panel, too :)

build3's currently being respun. It has the fix.

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/3.0a3-candidates/build3/

Once it's ready, please download and install it, then in the config editor, reset mail.winsearch.enable. You should get the prompt again on restarting Tb. It's best if you also remove the key you just added before doing this, and ensure that the key's added back when you click Enable, to verify that the fix is working correctly.

Thanks for catching this.

Comment 64

[Rain [:rain1] (inactive)]

Unfortunately I can't see myself having time to improve this for beta 1. I think I should have time for beta 2.

Comment 65

[David Ascher (:davida)]

If this blocks a blocking bug, it blocks.

Comment 66

[Mark Banner (:standard8)]

The Thunderbird drivers wish to release Thunderbird 3 as soon as possible. As a
result, we feel that this bug shouldn't stand in the way of all the other good
work getting into the hands of users sooner rather than later. Therefore we are
retargeting it for 3.1. See http://ccgi.standard8.plus.com/blog/archives/242
for more details. The 3.1 release is expected to be a quick release soon after
Thunderbird 3.

Comment 67

[RNicoletto]

Just FYI information, Thunderbird 3 RC2 do integrate well with Windows Desktop Search 4. Integration have been tested on Windows XP Professional SP3 32-bit, on Windows Vista Ultimate SP2 32-bit and on Windows 7 Ultimate 32-bit.

Comment 68

[Dan Mosedale (:dmosedale, :dmose)]

Given that 3.1 is mostly about fixing stuff that blocks folks from upgrading from Thunderbird 2, I suspect this doesn't want to block 3.1 at all, so I'm removing the blocking-thunderbird3.1+ flag for now.  That said, it's hard for me to tell exactly what work remains to be done here just from skimming recent comments.  Sid0, can you summarize the end-user visible changes, the work that's still needed, and what the risk is?  For now, I'm going to set this as wanted-thunderbird+.  If you think it should really block Thunderbird 3.1, you're welcome to renominate.

Comment 69

[Jens Müller (:tessarakt)]

Please make dependent on 567073.

Comment 70

[Jens Müller (:tessarakt)]

Please add 567212.

Comment 71

[Magnus Melin [:mkmelin]]

Time to close this. What was done was done.