Thunderbird should integrate with the Spotlight Search

RESOLVED FIXED in Thunderbird 3.0b2

Status

Thunderbird
General
--
enhancement
RESOLVED FIXED
13 years ago
7 years ago

People

(Reporter: u49640, Assigned: Bienvenu)

Tracking

(Blocks: 1 bug, {fixed1.9.1})

Trunk
Thunderbird 3.0b2
All
Mac OS X
fixed1.9.1
Dependency tree / graph
Bug Flags:
wanted-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(27 attachments, 13 obsolete attachments)

9.46 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
11.23 KB, patch
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.99 KB, patch
Josh Aas
: review+
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
1013 bytes, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
1.27 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
3.11 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
1.39 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
5.56 KB, patch
Details | Diff | Splinter Review
738 bytes, patch
Details | Diff | Splinter Review
19.37 KB, text/plain
Details
31.28 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
26.80 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
7.64 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
7.58 KB, application/octet-stream
Details
30.39 KB, application/zip
Details
1.09 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
1.31 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
37.70 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
1.28 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
7.40 KB, application/zip
Details
208.98 KB, image/png
Details
6.80 KB, patch
Scott MacGregor
: superreview+
Details | Diff | Splinter Review
2.56 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
661 bytes, patch
Bienvenu
: review+
standard8
: superreview+
Details | Diff | Splinter Review
1.62 KB, patch
Details | Diff | Splinter Review
2.87 KB, patch
standard8
: review+
Details | Diff | Splinter Review
3.73 KB, patch
Josh Aas
: review+
Benjamin Smedberg
: superreview+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; de-de) AppleWebKit/125.5.7 (KHTML, like Gecko) Safari/125.12
Build Identifier: 

The new Mac Os X 10.4 has a feature called Spotlight that allows fast finding of serveral Informations.

One of theese infos are Mail messages. Thunderbird should expose the stored Mails so that Spotlight 
can index and find them.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

13 years ago
Bug 290057 (although that only speaks about the subjects).

This should be a lot easier when we've moved to SqlLite to hold the mailfolders,
because SpotLight is also based on SqlLite.
(Reporter)

Comment 2

13 years ago
did you mean Bug 287122? (the other # is this bug)

The other bug wants a Spotlight-style search within Thunderbird, i want the Mac-Spotlight Search to 
find Thunderbird emails (and not only Mail.app Mails)

Comment 3

13 years ago
Found no dupes and valid RFE -> confirming.

Having received my Tiger on the 28 i can only deeply encourage such an
integration -> Thunderbird 1.1?

Maybe there are other Open Source projects writing Spotlight support for the
mbox format? Isn't Mail.app itself not using some variant of mbox?

Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → Thunderbird1.1
(Reporter)

Comment 4

13 years ago
(In reply to comment #3)
> Found no dupes and valid RFE -> confirming.
> 
> Having received my Tiger on the 28 i can only deeply encourage such an
> integration -> Thunderbird 1.1?

the last thin i've heard was that Spotlight needs one File/search result, but
apples own programs can somehow work around this
see http://arstechnica.com/reviews/os/macosx-10.4.ars/9

the intesting part begins with "The Address Book and iCal applications use
monolithic data "

Comment 5

13 years ago
patrick, please don't triage my bugs for me. The developer owns the target
milestone category. Thanks. 
Keywords: helpwanted
Target Milestone: Thunderbird1.1 → Future

Comment 6

13 years ago
I'll add my vote to this one.

Incidentally, http://developer.apple.com/macosx/spotlight.html provides an
overview of Spotlight.
*** Bug 287122 has been marked as a duplicate of this bug. ***
no, apple's apps do not get around the one-per-file restriction. Both safari and
address book work by making a flat, mutli-file "cache" in the caches folder, one
file per item. The Ars Technica article even mentions this. Look at
~/Library/Caches/Metadata/Safari.

Note Mail on Tiger uses an "emlx" format with a single message per file. You'd
have to perform a translation step for people upgrading (which is also what
mail.app does). 

While this isn't trivial, it's wicked cool. The more i use spotlight, the more
it grows on me, and the more apps that don't fit into it bug me. I just
implemented this for camino's bookmarks. 

Comment 9

12 years ago
It might be wicked cool but IMO creating one file per email to please Spotlight
would be a very, very bad thing to do.

Spotlight obviously has some (serious) deficiencies in it's current state. Apple
seems to know that, Microsoft already complained about this issue because it's
currently impossible to create an importer for Entourage that's not a dirty
hack. Apple and Microsoft are working on this issue so IMO it would be best to
wait a bit with this feature until Spotlight matures a bit, at least to the
point where it can let go of the one file per hit paradigm. I'm really, really
surprised that Spotlight got implemented this way.

Comment 10

12 years ago
registration of a spotlight meta-data plugin for TBird's flat-file mail message database is fairly straightforward. if we register the plugin for the "file type" (make one up if we have to) for the TBird message store files, said plugin will be called each time one of those files (mailbox folders) is updated. the GetMetadataForFile("INBOX") (for example) would then feed Spotlight with the new message(s) index data, and a Spotlight query for a term in that message would register a hit with our plugin. 

what is not obvious to me is how to disassociate the individual message in the flat-file, from the flat-file itself. we can do that easily on our end of course, but I can't tell at first glance how to tell Spotlight that the search term(s) is in messageX, and then how to associate the "opening" of that virtual reference to the TBird db, *and* have TBird fire up and select said message.

seems to me that Spotlight is hardwired for paths (1 to 1), and doesn't accommodate databases (1 to many) very well.
> what is not obvious to me is how to disassociate the individual message in the
> flat-file, from the flat-file itself. 

You can't. Welcome to spotlight ;)

Comment 12

12 years ago
Creating one file per e-mail might simplify some other things too.

For example, bug 114656 (allow arbitrary number of labels)

https://bugzilla.mozilla.org/show_bug.cgi?id=114656

In fact, why not just switch from mbox to maildir format? That's one file per e-mail. It's a standard format. Standard conversion utilites can use it. It's well understood. It integrates well with various IMAP servers and other utilities. 

Comment 13

12 years ago
Isn't there someone from Mozilla.org that can get in contact with someone high up in Apple to come to a solution for this? There are many more programs out there that have exactly the same problem. It would be really cool if Spotlight could index 'databases' but it currently can't. Entourage has the same problemn, Delicious Library has the same problem, etc, etc. Currently the only solution seems to be switching to one file per entity but that's horribly inefficient, especially if you use this Caches dir and duplicate your content. IMO Spotlight has a pretty large design problem here that needs to be fixed. It would be very nice if Mozilla could work on a solution together with Apple. All these workarounds just suck, let's try to fix this properly. If Apple sticks to their 'one file per entity' paradigm then I'm afraid that we don't have much choise, but until then we should try to get Apple to improve Spotlight.
(Reporter)

Comment 14

12 years ago
btw: if i understand the Mail Format for MS Mail in Windows Vista correctly, it is one File per Mail too. (maybe Vista's search function requieres also one File/Search result?)

see this Video: http://channel9.msdn.com/Showpost.aspx?postid=116711

We should make sure that a new Format (if there is one) works fine with OS X's Spotlight and Windows Vista's search function

Comment 15

12 years ago
In another bug, it's mentioned that Thunderbird will switch to a maildir format, just not as soon as 2.0. So when that happens, everything will be stored in one e-mail per file, and that will solve this problem.

Storing every e-mail in one big file isn't necessarily a good solution. One of the big problems with Microsoft's Exchange Server for years has been that it stores all mail (from every user) in one huge file (with an index). If the file ever gets corrupt, it screws up everyone's mail rather than just one mailbox of one user.

Plus putting things in multiple files can be much faster. Rather than searching through one big file (does Tbird even have an index?), you can setup a hashing system so that if you do any searching you do it through very small files. Additionally, rather than storing all status flags in the header of the message, you can encode them in the filename (if one message per file). 

It's premature to exclude splitting things out into multiple files simply because it's "less efficient" because in reality there are just as many efficiency objections with leaving lots of e-mail in a big file. It's true that storing things in two places seems silly, but considering someday switching to one file per e-mail isn't a entirely bad idea.

Comment 16

12 years ago
For what it's worth, Microsoft just released an update to Entourage 2004 that adds Spotlight integration.

Looks like they went the one-file-per-entity cache route: after the update, it creates a hierarchy under ~/Library/Caches/Metadata/Microsoft/Entourage on startup and then proceeds to populate it in the background. It stores "Microsoft Entourage message pointer" files in Apple's plist XML format, like Camino, with the content (entity-encoded plaintext only, no attachments) in one of the keys and the various fields and metadata in others.

So, while we can debate whether or not this is a wise approach, if Mac BU felt compelled to do it this way, I think it's probably fair to assume that Apple won't be changing the way Spotlight works anytime soon.
Is file-per-message (cache or otherwise) something to which the Thunderbird developers are amenable, or would another solution (what, I have no idea) be necessary to meet with their approval?
(Assignee)

Comment 18

12 years ago
I've said it in other bugs, but I guess not here - yes, I'm amenable to supporting something like maildir, one message per file. However, I'd like to make it an option, and I don't think we're going to get to it for 2.0, unless someone else does a lot of the heavy lifting. It's relatively straightforward, but it's a fair amount of work.

Comment 19

11 years ago
So, to conclude it a little bit, is there some workaround to search mails in TB via Spotlight? None I know about. 
Is there someone who can shred a light on it? When it might be in TB? In 2 months, in 1/2 year, in year, never, so one could decide if better switch back to Mail.app or any other mail client?

Comment 20

11 years ago
FYI "support qmail's maildir format" is Bug 58308. I think they should depend on each other.
(Assignee)

Comment 21

11 years ago
Created attachment 245653 [details] [diff] [review]
expose ability to get text of a message, in order for potential clients like spotlight support..

This just exposes the ability to parse a message for its text...
Assignee: mscott → bienvenu
Status: NEW → ASSIGNED
Attachment #245653 - Flags: superreview?(sspitzer)
(Assignee)

Comment 22

11 years ago
Created attachment 245654 [details]
snapshotting some work to generate files spotlight importer can read

This is just some stuff I've been working on to generate .mozeml files for each incoming new mail - these .mozeml files are read by a spotlight mdimporter I've written for thunderbird, which I'll attach when I get a chance.

This is all very preliminary, but I figured I'd better get this into bugzilla. Instead of using hiddenwindow.js, we'll probably end up with a new js component for this (or even a c++ one - not sure)
(Assignee)

Comment 23

11 years ago
Created attachment 246185 [details] [diff] [review]
teach Mac TB how to open .mozeml files from the command line

This teaches Mac TB how to open .mozeml files from the command line. W/O other changes, this code never gets invoked on the mac, so it's pretty much a noop. I moved the handy utility function that knows how to get a folder uri based on a directory path into nsMsgUtils.cpp, and changed it to take an nsCString reference.

We might want to make it open a stand-alone message window, or maybe use an existing 3-pane window...
Attachment #246185 - Flags: superreview?(mscott)

Comment 24

11 years ago
Comment on attachment 246185 [details] [diff] [review]
teach Mac TB how to open .mozeml files from the command line

Should we check the rv value here? 

+  nsCOMPtr<nsIMsgAccountManager> accountManager = 
+    do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);

Instead of a nsCString, should we be passing in a nsACString& ? 

Instead of using a file spec and nsFilePath, can we get the file spec, turn it into a modern file object and get the file path that way? Might make it easier to fix this code later on when we get rid of nsFileSpec/nsFilePath.
Attachment #246185 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 25

11 years ago
we should check rv, you're right

re passing in nsACString - I don't see the need. This is a helper routine with just two callers, both of who already have an nsCString handy.

I'm really just moving the utility function into base/utils, so I can get access to it - at this point, I'd rather not rewrite it to use nsIFile because I don't want to break anything for 2.0 - if I were introducing new code, yes, definitely, I'd want to use nsIFile...
(Assignee)

Comment 26

11 years ago
Created attachment 246316 [details] [diff] [review]
fix addressing comments, and change the file name parsing...

This is basically the same patch, except that I changed the mac command line processor code to pass in actual file names instead of file urls, which means the code that parses the command line file needed to change a bit. The big advantage of that is that now we can open .eml files from the command line, i I can get the mac command line processing change reviewed...carrying forward Scott's review.
Attachment #246185 - Attachment is obsolete: true
Attachment #246316 - Flags: superreview+
(Assignee)

Comment 27

11 years ago
Created attachment 246624 [details] [diff] [review]
pass mac filenames to command line processor

Josh, this is the change I told you about recently - it basically turns the apple script open document event into a filename passed into the app-specific command line handler, so nsMessengerBootstrap can parse the filename and decide what to do with it. This allows us to open spotlight search results from TB, which have the file extension .mozeml, and it also allows us to open plain old .eml files, if TB is registered as the app to open those files with. 

This patch is against the 2.0 branch, and that's where I'm really interested in landing it, since, as you mentioned, this code doesn't even get called on the trunk.
Attachment #246624 - Flags: review?(joshmoz)
(Assignee)

Comment 28

11 years ago
Created attachment 246626 [details] [diff] [review]
ignore .mozmsgs subdirectories

we put spotlight files in .mozmsgs subdirectories of the containing mail folder - we need to ignore these sub-directories when doing folder discovery.

We might want to remove the #ifdef MAC_OSX in case people point windows/linux builds at mac profiles...
Attachment #246626 - Flags: superreview?(mscott)

Comment 29

11 years ago
Comment on attachment 246626 [details] [diff] [review]
ignore .mozmsgs subdirectories

I agree we might want to remove the ifdef all together. Maybe we leverage the same directory structure  on other platforms for other  desktop search technologies...
Attachment #246626 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 30

11 years ago
ignore .mozmsgs subdirectories patch landed on trunk and branch.
(Assignee)

Updated

11 years ago
Attachment #245653 - Flags: superreview?(sspitzer) → superreview?(mscott)

Updated

11 years ago
Attachment #245653 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 31

11 years ago
Created attachment 246871 [details] [diff] [review]
reserve string for spotlight integration pref ui

'O' is used, but 'o' seems available - is that OK?
Attachment #246871 - Flags: superreview?(mscott)

Comment 32

11 years ago
Comment on attachment 246871 [details] [diff] [review]
reserve string for spotlight integration pref ui

access keys are case insensitive so that will conflict with "O".

You'll probably need to use a key that's not in your string for now like "Q".
Attachment #246871 - Flags: superreview?(mscott) → superreview+

Updated

11 years ago
Attachment #246624 - Flags: review?(joshmoz) → review+

Comment 33

11 years ago
Note that having MOZ_THUNDERBIRD there is not really a good idea since you're not supposed to have app-specific ifdefs in that code afaik. Among other reasons for that would be that xulrunner won't work with thunderbird. But I guess you don't care about that on the 1.8 branch and I'd rather you did that than not ifdef it for safety's sake.

+#include "nsICommandLineRunner.h"

Please also ifdef that for tbird.
(Assignee)

Comment 34

11 years ago
Comment on attachment 246624 [details] [diff] [review]
pass mac filenames to command line processor

since this allows us to open .eml files on the mac, it might be nice for 2.0 beta...
Attachment #246624 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #246624 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 35

11 years ago
thx for the speedy sr, Scott.

I've noticed that Firefox manages to open html files, and gets passed a file: url - I wonder if there's someway to get that to work with TB...maybe I should look at this a bit more...
(Assignee)

Comment 36

11 years ago
Created attachment 247471 [details] [diff] [review]
unescape message id filenames, send global notification when msg is loaded 

I need to escape the message ids before converting them to filenames, because characters like '/' in messageids confuse the file name. So I need to unescape when trying to convert the filename into a messageid.

I also added a global observer service notification when a message is loaded - this should make it really easy for extensions to know when a message is loaded. 

In general, those global notifications seem really easy and clean - is there some sort of downside I'm not seeing, other than the limitations to the data that gets passed around?
Attachment #247471 - Flags: superreview?(mscott)
(Assignee)

Comment 37

11 years ago
the reason the global notification is part of this bug is that I'm going to make sure a message is indexed after the user loads the message...

Comment 38

11 years ago
Comment on attachment 247471 [details] [diff] [review]
unescape message id filenames, send global notification when msg is loaded 

I'm wondering if we should document a list of all the known observer notification we want to fire in the message pane. Or wrap them all up into a single interface...
Attachment #247471 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 39

11 years ago
we should document them, but I think we should only put them in an interface if they need extra args in the notifications themselves.
(Assignee)

Comment 40

11 years ago
Created attachment 247685 [details] [diff] [review]
change notification to MsgMsgDisplayed

after talking to Scott, we decided to send the notification after the message has been laid out, so extensions can manipulate the dom or whatever they want. I changed the notification to MsgMsgDisplayed as well.
(Assignee)

Comment 41

11 years ago
Comment on attachment 247685 [details] [diff] [review]
change notification to MsgMsgDisplayed

carrying forward Scott's sr.
Attachment #247685 - Flags: superreview+
(Assignee)

Comment 42

11 years ago
Comment on attachment 247685 [details] [diff] [review]
change notification to MsgMsgDisplayed

oops, we wanted to pass in a reference to the msg hdr pane somehow where extensions can hang extra info off of, instead of the folder...obsoleting
Attachment #247685 - Attachment is obsolete: true
Attachment #247685 - Flags: superreview+
(Assignee)

Comment 43

11 years ago
Created attachment 247745 [details] [diff] [review]
open messages from spotlight results in stand-alone message window

this wasn't nearly as easy as I thought - we were ignoring the window type, so I had to add some code to pay attention to the window type, and then pass in the right args so that messageWindow.js would know how to open it.

So this basically works - but I think we'll want to do more here eventually, like try to find the message if it's already open in a window, and if we have tabbed messages, open a new tab instead of a new message window. We might also have to check prefs that control window opening behavior. But this is a start.
Attachment #247745 - Flags: superreview?(mscott)
(Assignee)

Comment 44

11 years ago
Created attachment 247746 [details] [diff] [review]
send MsgMsgDisplayed notification with header sink instead of folder

Is this what you had in mind, Scott?
Attachment #247746 - Flags: superreview?(mscott)

Comment 45

11 years ago
Comment on attachment 247745 [details] [diff] [review]
open messages from spotlight results in stand-alone message window

-        rv = OpenMessengerWindowWithUri("mail:3pane", folderUri.get(), msgKey);  
+        rv = OpenMessengerWindowWithUri("mail:messageWindow", folderUri.get(), msgKey);  

David, does this mean every .eml file passed through the command line will open in a stand alone message window?
Attachment #247745 - Flags: superreview?(mscott) → superreview+

Comment 46

11 years ago
Comment on attachment 247746 [details] [diff] [review]
send MsgMsgDisplayed notification with header sink instead of folder

Yeah, I think that's more helpful than the one had before. For those reading along, by moving it here, the dom for the message has already been constructed and is accessible to extensions.  And extension authors can use the property bag on the header sink to set extension specific message state.
Attachment #247746 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 47

11 years ago
No, that code was only for .mozeml files, i.e., our spotlight files, but yes, it means those will open in a new message window - before this patch, a new 3 pane ui window would open. I think ultimately this should be controlled by prefs, but I think a stand-alone message window is better than a new 3-pane. Switching the existing 3-pane window to the new message is an other possibility, and tabs, if we have them, are probably even better.

.eml files already would open in their own stand-alone message window...
(Assignee)

Comment 48

11 years ago
Created attachment 248104 [details] [diff] [review]
latest version of nsMessengerBootstrap changes

this is the patch I'll check in - it's the same as 247745 except that it also deals with case-insensitive imap INBOX
Attachment #247745 - Attachment is obsolete: true
(Assignee)

Comment 49

11 years ago
Created attachment 248105 [details] [diff] [review]
latest snapshot of work to stream newly arrived and read messages to spotlight

this fixes some problems with the previous snapshot - properly escape <>& in generated spotlight files, cleanup handling of queuing of messages to stream to spotlight, make sure we've streamed messages that the user reads, in case we've missed them otherwise.

The remaining work is to make sure we archive old messages with a background "process" that iterates over folders and makes sure all messages are indexed, and to make this a proper js component, not stuck in hiddenwindow.js. Also, need to implement a pref to turn this off.
Attachment #245654 - Attachment is obsolete: true

Comment 50

11 years ago
Created attachment 248120 [details] [diff] [review]
seamonkey bustage fix

I committed this as a bustage fix for SeaMonkey
(Assignee)

Comment 51

11 years ago
thx, Andrew, sorry about that.
(Assignee)

Comment 52

11 years ago
Created attachment 249715 [details]
snapshot of work to index new and existing messages

This code will index new messages, and in the background, index existing messages. I do that on a timer, rather slowly. And if the user reads messages, or new mail arrives, we delay the background indexing a bit more, in an attempt to mostly do our indexing when the user is idle...
Attachment #248105 - Attachment is obsolete: true
(Assignee)

Comment 53

11 years ago
Created attachment 251119 [details] [diff] [review]
backend support for notifications useful for indexers

It turns out that our existing notifications (nsIFolderListener, chiefly) are not sufficient for an indexing extension to use well. The biggest issue is that an indexer may want to know about move operations, instead of hearing about a copy followed by a separate delete. Also, nsIFolderListener's only receive notifications about folders that are open in the UI, which is of limited use.

This backend support doesn't really do anything if there are no listeners registered, so it's pretty harmless. I need to finish up some of the imap stuff, but the local operations are pretty much covered and I'd like to get this landed.

I chose the name nsIMsgFolderListener somewhat arbitrarily, and am open to suggestions for other names.
Attachment #251119 - Flags: superreview?(mscott)

Comment 54

11 years ago
Comment on attachment 251119 [details] [diff] [review]
backend support for notifications useful for indexers

some notes shared with David over IM:

1) we should break out nsIMsgFolderNotificationService into its own IDL file.

2) we should use ACString in the interfaces

3) 
+    // should notify nsIMsgFolderListeners about the folder getting deleted...

is that a to do comment?

Lemme know if you want me to review another patch with those changes or I can just plus this one.
Attachment #251119 - Flags: superreview?(mscott)
(Assignee)

Comment 55

11 years ago
I'll submit an other patch - up to you if you want to review the new one or the old one :-)

Yes, that's a TODO comment, or it might be a TODO question...
(Assignee)

Comment 56

11 years ago
Created attachment 251865 [details] [diff] [review]
address review comments.
Attachment #251119 - Attachment is obsolete: true
Attachment #251865 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #251865 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 57

11 years ago
Created attachment 253406 [details] [diff] [review]
js component for doing spotlight

This is the current state of the js component - it's turned off by default - you have to set a pref to enable it. I'd like to land this and get some feedback from folks.
Attachment #253406 - Flags: superreview?(mscott)

Comment 58

11 years ago
Comment on attachment 253406 [details] [diff] [review]
js component for doing spotlight

the spacing doesn't look quite right in the factory's createInstance method.

+  SIDump("initing spotlight integration\n");

initializing?

+  messenger = Components.classes["@mozilla.org/messenger;1"].createInstance().QueryInterface(Components.interfaces.nsIMessenger);

don't forget to declare messenger.

observe: function(aHeaderSink, aTopic, aData)

Do we need to make sure aTopic is MsgMsgDisplayed?

With regards to indexing a message every 60 seconds, seems like this feature could benefit from Bug 368591 when it gets implemented. We could link to that bug here in bugzilla so we can improve this down the line. 

do you have any test messages with non ascii strings (like japanese) that you can test against to make sure the conversions are working correctly when we write out the xml files (I have some I can send you if you need them).

Since this feature is pref'ed off, I wonder if we should land it on the branch right away too. Might be able to get a few more mac testers who are on nightly branch builds to try it out for us.

+var gSIDump = true;

Should that be false for checkin?
Attachment #253406 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 59

11 years ago
I've landed this on the 2.0 branch (Spotlight integration doesn't work on the trunk because apple events are broken). I fixed the gSIDump to be false, but spaced on the other review comments - I'll fix those today.

Test messages with non-ascii strings would be helpful

Finally, I need to figure out how to put the importer itself into CVS - it's a mac project file. I also need to figure out how to get the mac installer to install the importer...And, I need to figure out how to automatically register Thunderbird as a handler for opening .mozeml files...
(Assignee)

Comment 60

11 years ago
Created attachment 253545 [details] [diff] [review]
address review comments, and fix a couple bugs, particularly when deleting a folder/ emptying the trash
Attachment #253545 - Flags: superreview?(mscott)
(Assignee)

Comment 61

11 years ago
To get this to work, you need to install the Thunderbird mdimporter - I'm trying to attach it to this bug, but bugzilla is not happy. I'll have to figure out some way of doing that.

The other thing you currently need to do by hand is associate .mozeml files with the Thunderbird app. You can do this by finding a .mozeml file and double clicking on it - when it asks you what app to use, you find Thunderbird, and say to always use this app for files of this type.

And finally, you need to add the boolean pref "mail.spotlight.enable" using the config editor, set it to true, and restart TB. 

TB will then index new messages in the foreground, and index existing messages in the background, in the best approximation of idle time it can find. It will also index messages as you read them, if they're not indexed before.

Tomorrow's build should be worth trying.

Updated

11 years ago
Attachment #253545 - Flags: superreview?(mscott) → superreview+

Comment 62

11 years ago
Upload the importer somewhere else and post the link here. rapidshare should work for now.
(Assignee)

Comment 63

11 years ago
Created attachment 253622 [details]
zip of Thunderbird.mdimporter

Unzip this file into /System/Library/Spotlight (or unzip somewhere else and copy into /System/Library/Spotlight)

Comment 64

11 years ago
/System? A user should not modify /System. I guess /Library/Spotlight would be a better place.
(Assignee)

Comment 65

11 years ago
That should work as well - a lot more apps seems to use /System/Library/Spotlight, but not modifying System does seem like a better idea, thx.
(Reporter)

Comment 66

11 years ago
according to http://developer.apple.com/documentation/Carbon/Conceptual/MDImporters/Concepts/Troubleshooting.html#//apple_ref/doc/uid/TP40001690-222637 it should be in MyApp.app/Contents/Library/Spotlight

Other Locations are 
~/Library/Spotlight
and
/Library/Spotlight
(Assignee)

Comment 67

11 years ago
ooh, that makes sense, and it makes it easier for us to "install" the importer when TB is "installed".

Comment 68

11 years ago
If placing the importer within the app package, Spotlight probably won't auto-detect it if Thunderbird was updated via Check for Updates, since the bundle's mod date won't have changed. I just dragged TB to another folder and back to Applications, which did the trick. According to the Spotlight troubleshooting page, you can also do it manually with lsregister -f MyApp.app (found in /System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/LaunchServices.framework/Versions/A/Support/), so we may need to do that for people updating.

You can verify that the mdimporter is registered with /usr/bin/mdimport -L.

Testing it out now. One thing I noticed: messages with blank subjects come up as "null" or "Re: null". Mail.app displays the same messages as "No Subject" and "Re:".

Comment 69

11 years ago
Created attachment 253659 [details]
Updated plist file and a new document icon

Updated plist file to associate .mozeml (and .eml while I was at it) file with Thunderbird. Also added a document icon for these file types.
The LaunchServices database needs to be updated (probably the same lsregister command as in Comment #68 by Dan).

Comment 70

11 years ago
TB currently puts the whole raw mail in a .mozeml file. That's not the optimum and a waste of HDD space.

Attachments (unless they are forwared mails or plain text) should not be encoded into a .mozeml file. The file name might be enough.

HTML mails should be converted to plaintext first. Currently a HTML mail looks like this in a .mozeml file:

--Boundary_(ID_cH0MYdoHI8kUzl+Fd5qDUA)
Content-type: text/html; charset=iso-8859-1
Content-transfer-encoding: 8BIT

&lt;html&gt;
&lt;head&gt;
&lt;meta http-equiv="Content-Type" content="text/html; charset=iso-8859-15"&gt;
&lt;title&gt;
Infomaterial
&lt;/title&gt;
&lt;/head&gt;
&lt;body&gt;
(Assignee)

Comment 71

11 years ago
we don't put the whole raw e-mail in the .mozeml file - we put the text we managed to extract from the e-mail...including the text we managed to get out of html. If you have a message that's putting in raw html, I'd like to see that message...

Comment 72

11 years ago
I think there should be an option to exclude certain folders like Junk and Trash from indexing.

I also found that not all messages get indexed. My main Inbox folder has currently 3683 mails, but Inbox.mozmsgs contains only 3615 files. Too bad that's too many mails to find the affected mails.
Is there a way for force re-indexing? Maybe TB was not able to correctly resume the indexing process after I quit it.
(Assignee)

Comment 73

11 years ago
Option to exclude certain folders would be nice but it won't happen for 2.0.

We index in the background, so we might not be finished indexing your Inbox. We restart the background indexing every time you restart, so if we think there are un-indexed messages in your Inbox, we will try to index them.  When we index a message, we store that info in the .msf file for the folder. It may be that we failed to generate a .mozeml file for some of your messages, but I don't know why that would be...

We also try to index a message when you read a message, if we don't think we've indexed it.

We use the message-id as the name of the .mozeml file. You might be able to list all the .mozeml files, sort them, and then grep for message-id in your Inbox, sort that list, and compare them. But messages forwarded as attachments will throw off that list.

You could try a binary search of your Inbox to find a missing message - copy half your inbox to a temp folder and see how many .mozeml files get generated, copy half of those to an other temp folder, etc, until you have a small enough test case to find a missing message...
(Assignee)

Comment 74

11 years ago
Created attachment 253769 [details] [diff] [review]
fix handling of messages with BOUNDARY, not boundary

thx very much to Marcus for providing the test cases that weren't working
Attachment #253769 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #253769 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 75

11 years ago
BOUNDARY vs boundary fix landed on trunk and branch, thx again, Marcus.

Comment 76

11 years ago
I just wanna double check something: I'm running "version 2 beta 2 (20070204)" on Tiger, but the  "mail.spotlight.enable" is not present by default in the config editor.

I've added it manually now, but should this pref not appear automatically in the latest nightly branch builds?

Comment 77

11 years ago
One more thing, things seem to work fine, but hits show in spotlight as Documents rather than Mail messages.
(Reporter)

Comment 78

11 years ago
It would be a good idea to have a spotlight importer for the contacts in the address book as well. Should I file a different bug for this?

Comment 79

11 years ago
(In reply to comment #78)
> It would be a good idea to have a spotlight importer for the contacts in the
> address book as well. Should I file a different bug for this?


An other approach would be to integrate TB with the Mac OS X address book, see bug 203927. I guess it will vary from user to user, which approach is preferred.
(Assignee)

Comment 80

11 years ago
I haven't added "mail.spotlight.enable" to thunderbird.js yet - I should do that.

Re documents vs. Mail messages - there's probably some magic I can do to determine the document name.

Someone could write a spotlight importer for .mab files - it would be a bit large since it would link in mork and nspr and xpcom, I think. It would also get told the file changed rather frequently so import would probably run often.

Comment 81

11 years ago
How are the chances to use Spotlight search within Thunderbird? (Maybe as an extension.)
TB's full text search is very slow and navigating with the Finder to the mailbox folder (if I only want to search for mails, not the whole system) and search with the Finder is also not very convenient.

Comment 82

11 years ago
Another thing: in the search results window, all e-mails have a "Created" and "Modified" date & time stamp determined by when they were indexed. 

That is, since i installed mdimport today, all my mails are listed as if they were sent today. Ideally though they should be registered with the sent date from the e-mail.
(Assignee)

Comment 83

11 years ago
I think I had a hard time converting the Date: information as stored in the msg database into a format that Spotlight was happy with.  I think I'd want to set kMDItemContentCreationDate to that date. If anyone has tips for how to do that in js, that would be helpful.

Comment 84

11 years ago
(In reply to comment #77)
> One more thing, things seem to work fine, but hits show in spotlight as
> Documents rather than Mail messages.
> 

I noticed something simmilar. When I use Spotlight to search for mails, most (but not all) appear with their subject as result. But once I click on a message, it changes to the filename.
(Reporter)

Comment 85

11 years ago
FYI: i've filed bug 369283 for the same thing for Vista. 
maybe those two could be solved together/similar?
(Assignee)

Comment 86

11 years ago
Most of the work I've done for spotlight integration could probably be re-used for Vista.  

Comment 87

11 years ago
Today I re-indexed all my mails.
I found that there are weird line breaks in the mozeml files.

The DOCTYPE declaration in the second line breaks after "http://www.apple.":

<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.
com/DTDs/PropertyList-1.0.dtd">

I also found, that the mozeml exporter does not correctly threat mails if lines are wraped in the mail source with a = sign. Just like:

Blah1 blah2 bl=
ah3 blah4 blah5

The correct way would be to treat these 2 lines as "Blah1 blah2 blah3 blah4 blah5", but the exporter puts a line break where the = sign is.
("bl
ah3")
(Assignee)

Comment 88

11 years ago
Created attachment 255847 [details] [diff] [review]
add default value for turning on spotlight integration, starting w/ false

this adds a default value for this pref, currently off...next, I'm going to get the importer building...
Attachment #255847 - Flags: superreview?(mscott)

Updated

11 years ago
Attachment #255847 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 89

11 years ago
Created attachment 255943 [details] [diff] [review]
add search and search/mdimporter to build 

My plan is to land this turned off, and then land the last change (to mail/components/Makefile.in) to turn this all on...
Attachment #255943 - Flags: superreview?(mscott)

Comment 90

11 years ago
Comment on attachment 255943 [details] [diff] [review]
add search and search/mdimporter to build 

fix the typo in allmakefiles.sh
Attachment #255943 - Flags: superreview?(mscott) → superreview+

Comment 91

11 years ago
I think that the Spotlight importer should be renamed with respect to SeaMonkey and Correo. Maybe "Mozilla Mail.mdimporter"?

Comment 92

11 years ago
Hi all!
I just would like to understand how is going on..

I mean, will this feature be included into TB2?

If not, what's the way to build a TB with this feature?

Thanks in advance for the attention!

Comment 93

11 years ago
(In reply to comment #92)

> I mean, will this feature be included into TB2?

It is already a feature of the current TB2-pre builds. Set "mail.spotlight.enable" to TRUE, download the Spotlight Importer and read Comment #64.

Comment 94

11 years ago
I have installed the spotlight importer and changed mail.spotlight.enable to TRUE.  Indexing proceeds just fine, and clicking on the found file from Spotlight opens TB.  But TB opens to a Conpose Message window if it was not already running, and if it was already running, it just brings up whatever window is open.  That is, it does not find the message within TB.  What am I missing?
(Assignee)

Comment 95

11 years ago
DPDP - what version of TB are you running? You've told Mac OS/X to associate .mozeml files with TB?

Comment 96

11 years ago
> DPDP - what version of TB are you running? 

TB2 RC1, but when I first tried I was running TB2pre, latest version.  Both had the same problem.

>You've told Mac OS/X to associate .mozeml files with TB?

Yes.

But I now have more details to report.  Things work fine with mail stored in my inbox file on the IMAP server.  I click on the found .mozeml file, TB starts up or comes to the foreground, and displays the message just as it should.

It's for messages saved on my HD, in the folder hierarchy, where it doesn't work and behaves as I reported in my previous message.
(Assignee)

Comment 97

11 years ago
So it's not working for "local folders" messages? Were these messages copied from your imap folder to a folder in the local folders account? Or downloaded via a pop3 server? Is there anything non-standard about your local folders directory (e.g., a symlink?)

Comment 98

11 years ago
(In reply to comment #97)
> So it's not working for "local folders" messages? 

Exactly.

> Were these messages copied
> from your imap folder to a folder in the local folders account? 

Once upon a time, most of these messages started out elsewhere.  I switched from Eudora to Thunderbird about a year ago, and many of these messages started out on a pop3 server, got copied into Eudora folders, which then got converted to Thunderbird folders.  More recently, everything added to the Local Folders was copied there from the Inbox.  But perhaps I'm not sure what you had in mind.

> Or downloaded
> via a pop3 server?

See above...

>  Is there anything non-standard about your local folders
> directory (e.g., a symlink?)

Nothing that I can think of, no symlink.  The installation was copied in toto from a different machine to this one last summer, in case that's relevant, but I've had no problems or complications.

Note that the .mozeml files are created just fine.  It's clicking on them that isn't working right.
(Assignee)

Comment 99

11 years ago
So it doesn't work for either the old Eudora messages or the imap messages that were copied to the local folder? I wonder if it's an escaping problem with the uri we try to load...

Comment 100

11 years ago
(In reply to comment #99)
> So it doesn't work for either the old Eudora messages or the imap messages that
> were copied to the local folder? 

That's true.  But here's another detail, as I've been tinkering with this.

I keep my old mail in subfolders of a "Filed Mail" folder within "Local Folders".  I took a few messages from one of my old mail folders and moved them into "Sent" in "Local Folders".  They then opened properly from within Spotlight (after waiting a bit).  So it isn't everything in Local Folders that fails to open properly, but perhaps it's embedded folders that are the problem.

> I wonder if it's an escaping problem with the
> uri we try to load...

That's beyond my competence, alas.

(Assignee)

Comment 101

11 years ago
Can you tell if there's a relationship between the local folder having a space in the name, and whether it works or not?

Comment 102

11 years ago
I can exactly confirm the behaviour DPDP reported. I also use TB 2.0 RC 1 (German). My mails are all stored in /Users/marcus/Library/Thunderbird/Profiles/default/8ti6is20.slt/Mail/Local Folders, I only use POP3 accounts and global inbox. When TB is not running and I try to open a mail found by Spotlight, TB starts and just open up the compose window. When TB is running, it depends on the folder where the mail is in: In a folder called "Allgemein" TB opens the correct mail in an extra window, in a folder called "Accounts & Codes" only the TB application comes up as it is, as if I had clicked on the icon in the dock. 
About a year ago I migrated the TB profile from my Windows XP installation to my new Mac, so it is not a fresh profile.

Comment 103

11 years ago
(In reply to comment #101)
> Can you tell if there's a relationship between the local folder having a space
> in the name, and whether it works or not?
> 

Yes, I think there might be.  In fact, I think if there is a folder with a space in its name about the target in the hierarchy, it causes the problem.  I created two new folders in with Local Folders as its immediate parent:  one called "test" and one called "test space".  I then created two subfolders inside each of these, with no space in their filenames, and put copies of the same four messages in each.  Once the .mozmsgs files were created and populated in each of the lowest-level folders, clicking on the one inside "test" brings up the message in Thunderbird.  Clicking on the one inside "test space" does not.

So it looks like your hunch is on the right track, and the difference between the two folder names in Marcus Münch's message also suggests this.  And recall that almost all my messages are in a hierarchy below a folder called "Filed Mail" (plus most of them have spaces in their own names, since lots of them are "Lastname, Firstname").

Comment 104

11 years ago
(In reply to comment #103)
> I think if there is a folder with a
> space in its name about the target in the hierarchy, it causes the problem. 

I meant "*above* the target in the hierarchy".

Comment 105

11 years ago
Just wanted to add to what DPDP reported: after installing the mdi importer and setting mail.spotlight.enable to true, I am able to perform spotlight searches and find my items in Thunderbird.  Depending on a couple of variables, behavior is pretty, um, different ;-)

1) If the folder it is stored in has no spaces in it, Thunderbird launches if needed and opens up a message window as expected.
2) If the folder name has spaces and Thunderbird is not running, Thunderbird launches with an empty compose window.  Also, I don't seem to display the main UI (i.e. the 'Mozilla Thunderbird' window is not display nor in the Window menu)
3) If the folder name has spaces and Thunderbird is running, Thunderbird crashes. (Let me know how you want me to submit the crash reports if they're needed)

Also, it appears that indexing isn't happening (reliably, at least) for the subject lines.  I've run a few searches based on subject lines which returned no results.  However, if I search on something from the message body, Spotlight displays the message (displaying the subject line I couldn't search on initially)

Comment 106

11 years ago
One additional item:  I tried renaming the folder to see if that would be a viable workaround but still have the problem.  When I took a look at what was going on in the Thunderbird 'Local Folders' directory, the 'foldername' and 'foldername.msf' were renamed but not the 'foldername.mozmsgs' directory retained the original name.
QA Contact: general
Depends on: 380163

Comment 107

10 years ago
(In reply to comment #101)
> Can you tell if there's a relationship between the local folder having a space
> in the name, and whether it works or not?
> 

Hi,

I replied to your questions, but nothing new has been asked or added since.  Any progress?

Thanks!
(Assignee)

Comment 108

10 years ago
Created attachment 267650 [details] [diff] [review]
try forcing mdimporter to be deployed with 10.4, since that's where spotlight is.

I'd like to try this...
Attachment #267650 - Flags: superreview?(mscott)

Updated

10 years ago
Attachment #267650 - Flags: superreview?(mscott) → superreview+

Comment 109

10 years ago
(In reply to comment #77)
> One more thing, things seem to work fine, but hits show in spotlight as
> Documents rather than Mail messages.
> 

I've found a way to change this. You can read about it here -> http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html

Comment 110

10 years ago
Created attachment 271826 [details]
Thunderbird.mdimporter -- Updated with info from comment #109

I uploaded a new version of Thunderbird.mdimporter. It's a Universal Binary build from a fresh CVS checkout and includes the patch from comment #109.
The previous Thunderbird.mdimporter should be obsolete by now.
(Assignee)

Comment 111

10 years ago
Thomas and Kamikazow, thx for your help. I'll try to get the fixed info.plist into cvs...

Comment 112

10 years ago
(In reply to comment #109)
> I've found a way to change this. You can read about it here ->
> http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html
says:
"i don't know how to convince spotlight to reload its plugins so reboot please. reindex your spotlight now."

wouldn't reloading be done by cd'ing to 
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/Frameworks/LaunchServices.framework/Versions/A/Support 
and then a 
./lsregister -f ~/Library/Spotlight/Thunderbird.mdimporter/
(depending of course on where you put the mdimporter)

followed by a reindexing:
mdimport -r ~/Library/Spotlight/Thunderbird.mdimporter
(depending of course on where you put the mdimporter)?

Comment 113

10 years ago
Created attachment 271981 [details]
Picture describing 

There is a problem when searching for "Spotlight" or "Metadata". As seen in the attachment, all mails are shown. In this case over 19k. I guess if found the problem.
The kMDItemKind, which is obviously included when indexing mozeml files, is "Thunderbird Spotlight Metadata". It should be changed to something like "Thunderbird E-Mail". 
It think it should be really simple to change this... but frankly i have no clue how. And i don't know when or where it is set by Thunderbird. 
Got some pointers, somebody?

Comment 114

10 years ago
The string "Thunderbird Spotlight Metadata" is written in the info.plist of either Thunderbird itself or the mdimporter. Change that string and follow comment #112.

The problem is: mozeml files are Spotlight Metadata. They are not the e-mails. What you are describing seems to be a limitation of Spotlight. That should hopefully be not a big deal if you use Cmd-F in the Finder and restrict the search to "Content".

Comment 115

10 years ago
Now I'm confused. Really confused.
I should have tried it myself before I post. If the query is restricted the to "Content", ever single mail does indeed not appear in the results window. But if mails appear there, it's under "Documents", not "Mail Messages". Using the Spotlight menu icon puts mails under "Mail Messages".

Comment 116

10 years ago
(Warning: Wild guesses!;) 
Cmd-F is no option either. Because Cmd-F is used for searching directorys like your Home folder. So restricting a search to content "Spotlight" finds you .mozeml files and not... well... Spotlight indexed e-mails. Perhaps that's why it apperas in Documents? 
But i guess it pulls (or tries to pull) meta information from Spotlight to show e.g. E-Mail Subject in the results. But this works only 1 out of 10 times for me. And after you click on a result, the shown Subject reverts back to a <message-id>.mozeml file name. 
I just configured Mail.app for the same IMAP Account i use with Thunderbird. After a Cmd-F search only the Thunderbird "Mails" appear. In Spotlight both Mail.app and Thunderbird Mails appear. So i guess Cmd-F search really only searches for files (and in them). 


But here another suggestion. Wouldn't it be better to generate some kind of database file (sqlite?) and let the mdimporter index that instead? AFAIK Mail.app uses sqlite for exactly that purpose. 
It would be way easier and faster for the mdimporter to search through and it would not clutter up your harddrive with thousands and thousands of extra files. 

Spotlight would register a change in said database file and ask Thunderbird.mdimporter to index it, which in turn selects all records that are not indexed. Perhaps through a boolean column "indexed" that the mdimporter sets to true after it has indexed the records data. 

I know it's the same principle to just go through the mbox files, but a real database is easier to access and amipulate.  

Comment 117

10 years ago
Upon creating pop3 accounts, Thunderbird creates a corresponding account folder in the profile folder. In this account folder a default set of mailbox files are created such as inbox, sent, trash, regardless of the actual need for it: these files are redundant if one sets the account to use the global inbox (which lives in "Local Folders" in the profile rather than the account specific folder).

In the current implementation the indexing procedure tries to index these redundant mailbox files, which is not only useless but in my case it fails in doing so and hence refuses to proceed indexing the remaining mailbox files. This behaviour is observed based on the value for the mail.spotlight.lastFolderIndexedUri parameter in the prefs file which value sticks to a "hidden" mailbox file in the account folder and no further background indexing is observed for hours.

There is a workaround by disabling the global inbox setting for the pop3 account. But there must be a more user friendly implementation possible.
(Assignee)

Comment 118

10 years ago
Jasper, I will try that out. We shouldn't be indexing those files, I think.
(Assignee)

Comment 119

10 years ago
Created attachment 275651 [details] [diff] [review]
[checked in]add calls to folder rename notification, and delete notification for imap messages.

when working with the Beagle integration guy, I noticed we were missing a few notifications.
Attachment #275651 - Flags: superreview?(mscott)
(Assignee)

Comment 120

10 years ago
a.peyser, I was wondering if you were interested in working the same magic for the mdimporter that you worked for OSX address book integration, so that we can potentially turn this on on the trunk. If so, thx!

Updated

10 years ago
Attachment #275651 - Flags: superreview?(mscott) → superreview+
(Assignee)

Updated

10 years ago
Attachment #275651 - Attachment description: add calls to folder rename notification, and delete notification for imap messages. → [checked in]add calls to folder rename notification, and delete notification for imap messages.

Comment 121

10 years ago
The mdimporter seems to be broken under Leopard (10.5). Specifically, while the .mozeml files are being created, their kMDItemContentType is shown as dyn.<random string> rather than com.mozilla.thunderbird.mozeml as described at http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html. Unfortunately, I have no idea how to fix this...

Comment 122

10 years ago
(In reply to comment #121)
> The mdimporter seems to be broken under Leopard (10.5). Specifically, while the
> .mozeml files are being created, their kMDItemContentType is shown as
> dyn.<random string> rather than com.mozilla.thunderbird.mozeml as described at
> http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html. Unfortunately,
> I have no idea how to fix this...
> 

http://developer.apple.com/documentation/Carbon/Conceptual/MDImporters/Concepts/Troubleshooting.html#//apple_ref/doc/uid/TP40001690-221790

writes:

Typically, the return of a dynamic UTI indicates that the file is not mapping to a known UTI. You should check that:

1. The test file is the correct file type.
2. The test file has the correct filename extension or file type set.
3. If your application is declaring the UTI type for the document, that the application's Info.plist file has the correct entries in the UTExportedTypeDeclarations entry as shown ...

I have checked Thunderbird.app's Info.plist and seems it has no UTExportedTypeDeclarations entry at all.

Could it be somehow related to this fact?

Comment 123

10 years ago
I'm trying to get Spotlight to work with a very large existing mail spool. Unfortunately, the conversion process to individual files is insanely slow. I'm getting a little less than one per second with TB 2.0.0.6 on a G5 dual 2.5 GHz machine running Leopard 10.5.1. I have hundreds of thousands of emails, so this is literally going to take days.

I did an activity monitor sample of TB and it looks like it's processing the conversion in a timer. Based on David Bienvenu's comments above, this is intentional in order to limit the processing power dedicated to the conversion process. But the effect on my machine is that barely anything is happening.

I'd much rather TB just take over the machine and crank these out until done. Is there any way to make the initial conversion process faster?

Comment 124

10 years ago
Looks like I made a bad assumption before trying this option. The problem I'm *really* trying to solve is to make my Leopard Time Machine backup operations more efficient. Currently I'm seeing large amounts of data being backed up hourly. This is because Thunderbird stores mail in monolithic files, and I have some really big mailboxes that I haven't manually archived. As per Apple's warnings at WWDC, applications that make small changes to large files are Time Machine unfriendly.

My assumption was that this Spotlight fix would change Thunderbird's storage mechanism from monolithic files to individual, per-email files. Instead, it looks like the fix doesn't change the underlying storage mechanism. All it does is copy all of my mail into individual files so that the Spotlight importer can operate on them.

So unless I'm missing something, the Spotlight fix makes my problem worse. Because in addition to having large, monolithic, incremental-backup-unfriendly mailbox files, I'll have duplicates of every single email in a separate file. So my mail will take up twice the space that it should (or worse, considering the additional file overhead for 500k some odd email files, in my case). That's assuming of course that the workaround completes during my lifetime. After running overnight, it processed under 70k emails.

Wouldn't the better long term solution to be to enable Thunderbird to store emails in individual files rather than monolithic mailboxes? That way Thunderbird would be friendlier to both Spotlight indexing *and* incremental backup technologies like Time Machine.

Years ago I moved from Mail.app to Thunderbird, mainly for reliability reasons. I was never able to get Mail.app to work well with my mail spool. I lost tons of mail and got fed up wrestling with Mail.app. With Thunderbird, I've enjoyed 100% reliability.

Thunderbird is such a great application. But it feels like Thunderbird is not keeping up with the Mac platform. It would be a shame if Thunderbird's Mac market share dwindles to just the Mail.app disaffected.

Unfortunately I'm toying with giving Mail.app another chance. With the advent of Leopard, I'm probably not the only one.

Sorry for the somewhat OT nature of this comment. Maybe I should enter make a new bug specific to the incremental backup issue?

Comment 125

10 years ago
What you are asking for is already covered in Bug 58308. That bug was filed 7 years ago and is not really related to Mac technologies, but it would help Spotlight and Time Machine a lot.

Comment 126

10 years ago
I have the Thunderbird.importer installed properly and I'm getting hits, but they are showing up as Documents rather than Mail Messages. Looking through the comments, I think someone's solved this. Is there a newer version of the importer somewhere?

Comment 127

10 years ago
See Comment #115. It depends whether you use the Spotlight menu item or use the Finder. One puts them under Mail Messages, the other under Documents and I have no idea why.

Comment 128

10 years ago
Is this working for anyone on Leopard? I have successfully created thousands of mozmsg files, but they don't show up in Spotlight despite several reindexing attempts. Thunderbird.mdimporter shows up as being registered correctly. Also, running mdls on the mozmsg files does not return the "public.email-message" string, even though I am using the 7/11 version of the mdimporter posted above. Any ideas?

Comment 129

10 years ago
That is my experience too, alas. I've tried telling Spotlight to index a wider range of files, and even moved the entire TBird directory (the one containing my preferences, profile and messages) into ~/Library/Application Support/ without any success. I'm flat out of ideas.

Comment 130

10 years ago
The Thunderbird Spotlight plugin does work for me under Leopard. But you have to enable the search for "system files". It took me quite some time before I figured out. Read it here:
http://db.tidbits.com/article/9283

Comment 131

10 years ago
Unfortunately, I have had no luck at all getting Thunderbird e-mails to show up in Spotlight.  I have pretty religiously tried all the instructions and suggestions that I found online but no joy.  

Today, I did a little test -- I copied some of the mozeml files into a temp folder that I have under Documents.  Spotlight almost immediately showed them when I searched.  So, that means to me that it is POSSIBLE for the files to show up -- so the Thunderbird.mdimporter must be working.

So, it seems the problem (at least for me) is that Spotlight doesn't index the mozeml files if they are in ~/Library/Thunderbird/Profiles/default.a6j/Mail/Local Folders/Inbox.mozmsgs.  

I tried adding and deleting the directory from the Privacy tab in Spotlight Prefs.  And I even tried creating a link in the temp folder back to the Thunderbird Local Folders -- but that didn't help.

Comment 132

10 years ago
I've downloaded the last mdiimport. the mozeml file seems to be generated with stange names begining by long random string. Is it normal.

The problem is that if I search for emails with spotlight I find nothing. If I search for "thunderbird" I can see the mozeml files with their useless names... What's happening ?

I'm on Leopard and my profile folder is in ~documents/THUNDERBIRD



Comment 133

10 years ago
Futhermore i've seen a strange error message in /var/system.log :
[0x0-0x24024].org.mozilla.thunderbird[615]: [Exception... "Component returned failure code: 0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS) [nsILocalFile.create]"  nsresult: "0x80520008 (NS_ERROR_FILE_ALREADY_EXISTS)"  location: "JS frame :: file:///Applications/Thunderbird.app/Contents/MacOS/components/nsSpotlightIntegration.js :: GenerateSpotlightFile :: line 587"  data: no]


What does this mean ?
Flags: wanted-thunderbird3?

Updated

9 years ago
Duplicate of this bug: 427342
Duplicate of this bug: 446329

Comment 136

9 years ago
Is there an update on spotlight integration - including a set of instructions to make it work (or test it out) on recent versions? 

Comment 137

9 years ago
Once this is implemented is there a way to disable this feature.

see bug 369283 comment 10 
hwaara, do you have any interest in taking a run at getting the spotlight stuff into shape?
It'd be nice if this is made to work with the unified default client/search dialog.

It'd be great if
a. this was converted into a module (SpotlightIntegration.jsm?) with "SearchIntegration" as the exported symbol, which has a few methods (init(), shouldPrompt(), setStatus(enabled)), enabled being a boolean.
b.I don't know if you needs elevation (sudo) to add the mdimporter or the Tb profile directory to Spotlight scope, but if you do, and want to present some UI cue to the user, then please define a needsElevation() method in SearchIntegration as well.
c. setStatus adds/removes the mdimporter and folders to/from Spotlight's scope.

Comment 140

9 years ago
(In reply to comment #138)
> hwaara, do you have any interest in taking a run at getting the spotlight stuff
> into shape?

Sounds like a fun project, but I'm very busy with school + work right now, so unfortunately I cannot commit to it at this point.
Created attachment 351442 [details] [diff] [review]
Turn-on mdimporter by default in build system

This gets mdimporter building by default, and uses dist/package as a staging ground before putting it in the bundle dir.
Attachment #351442 - Flags: superreview?(bugzilla)
Attachment #351442 - Flags: review?(bugzilla)
It'll probably be a good idea to also get the fix mentioned on this page: http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html (comment 109) into the plist.
Created attachment 351488 [details] [diff] [review]
[checked in] Info.plist updated with public.email-message UTTypeConformsTo

As per Sid's suggestion, I've updated the Info.plist as documented here http://razal.de/~ibn/techblog/mac/thunderbird_n_spotlight.html.  NOTE: I used the Leopard's Property List Editor, so the diff is somewhat more than it needs to be with shuffling of some elements positionally.  I used plutil to confirm that it's right, though.
Attachment #351488 - Flags: superreview?(bugzilla)
Attachment #351488 - Flags: review?(bugzilla)
Depends on: 468198
Created attachment 351818 [details] [diff] [review]
[checked in] Add mail.spotlight.enable pref

This adds the pref, but I've left it false for now until there is some UI to enable users to modify it more easily.  I think this is wise, given the amount of disk space this can take.
Attachment #351818 - Flags: superreview?(bugzilla)
Attachment #351818 - Flags: review?(bugzilla)
Attachment #351442 - Flags: review?(bugzilla) → review?(bienvenu)
(Assignee)

Updated

9 years ago
Target Milestone: Future → Thunderbird 3.0b2
Attachment #351488 - Flags: review?(bugzilla) → review?(bienvenu)
Attachment #351818 - Flags: review?(bugzilla) → review?(bienvenu)
(Assignee)

Updated

9 years ago
Attachment #351442 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

9 years ago
Attachment #351818 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 145

9 years ago
Comment on attachment 351488 [details] [diff] [review]
[checked in] Info.plist updated with public.email-message UTTypeConformsTo

Are those tabs? It looks like the old file had the same, though... I wonder about that ostype of XXXX but that's just moved as well...
Attachment #351488 - Flags: review?(bienvenu) → review+
I'm trying to wrap this up ASAP, and these are the issues I can see:

* Need pref UI - filed bug 468373, I'll deal with it there

* Need to investigate why only new mail is being indexed on arrival and not
existing mail in the background.

* Spotlight refuses to show results for me, despite the fact that mdfind knows
about the .mozeml files, and mdls shows me all the correct metadata (see bug
468198).  My research leads me to the conclusion that Spotlight on Leopard
refuses to show results for so-called "System Files" and that includes things
under ~/Library, where the tb profile dir is located.  Who knows something
about Spotlight and can comment on this?

* I've updated the previous 1.8 patch to nsCommandLineServiceMac.cpp for trunk,
and will attach that in a min.

Anything else anyone can think of?
Created attachment 351823 [details] [diff] [review]
Add TB specific Mac file-handling so Spotlight can open mail from searches

This allows us to pass filenames into Thunderbird, which Spotlight needs to do in order to open individual mail returned from a search.  This is a work-around until bug 380163 is fixed.  I'm basing this off code bienvenu did and got in on 1.8 (see https://bugzilla.mozilla.org/attachment.cgi?id=246624&action=edit).
Attachment #351823 - Flags: superreview?(benjamin)
Attachment #351823 - Flags: review?(joshmoz)
> Are those tabs? It looks like the old file had the same, though... I wonder

Probably.  As I said above, and you may have missed with the review switch, I used the apple Property List Editor instead of emacs, so if you care, I can redo with emacs.

> about that ostype of XXXX but that's just moved as well...

Yeah, that's just moved.  I don't know what that should be tbh.
Most things under ~/Library, but not all. Once we get done rapping our knuckles for thinking about just putting our cuckoo's egg in the ~/Library/Mail/ nest, it looks like what other projects who've hit this have done is to either move into ~/Documents/ for things like chat logs that can reasonably be called documents, or ~/Library/Caches/Metadata/ for things that they already treat as caches, recreating them automatically if a restore-from-backup wiped them out.

The one other thing I wonder about, from http://support.apple.com/kb/TA23187?viewlocale=en_US, is ~/Library/Metadata/ - I don't have one, on either 10.4 or 10.5, but if creating one and dropping stuff in it actually does show up in 10.5 results, and gets backed up by Time Machine, then that seems like the perfect place for us.
After some more research [1], and discussions on irc with philor and bienvenu, here is what we've come to:

* Microsoft and Apple both use ~/Library/Caches/Metadata for Spotlight metadata (note: folder selector - kSpotlightMetadataCacheFolderType = 'scch'), and we should too.  It would mean we could get .mozmsg/.mozeml out of the profile and also has the benefit that Spotlight actually shows results from here (see comment 149).

* Following what others do, we should consider re-rooting our .mozmsg/.mozeml stuff from the profile dir to ~/Library/Caches/Metadata/Thunderbird/[profile_dir] (i.e., we need to be careful we support multiple profiles).

* The suggestion has been made that we a) not migrate data from the profile to the new metadata dir; and b) have this metadata automatically rebuilt in case of a recovery (i.e., Time Machine doesn't back this up by default), or the "pref is turned on for the first time" case.

[1] http://developer.apple.com/documentation/MacOSX/Conceptual/BPFileSystem/Articles/WhereToPutFiles.html#//apple_ref/doc/uid/TP40001411-BAJHCHJI
Comment on attachment 351442 [details] [diff] [review]
Turn-on mdimporter by default in build system


> 	$(PBBUILD) $(PROJECT_ARG) -target $(TARGET) -configuration Release $(PBBUILD_ARG)
...
>+	$(INSTALL) "$(XCODE_PRODUCT_DIR)/Thunderbird.mdimporter" $(DIST)/package

I think we should either use $(TARGET) in both these lines, or drop the $(TARGET) variable and just replace it with Thunderbird. Your choice, sr=me with one of those changes.
Attachment #351442 - Flags: superreview?(bugzilla) → superreview+
Created attachment 351886 [details] [diff] [review]
[checked in] Turn-on mdimporter by default in build system v2

I removed $(TARGET) because it can't have any other value.  This needs a checkin please.
Attachment #351442 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 351886 [details] [diff] [review]
[checked in] Turn-on mdimporter by default in build system v2

Checked in: http://hg.mozilla.org/comm-central/rev/73b79d33f884
Attachment #351886 - Attachment description: [checkin needed] Turn-on mdimporter by default in build system v2 → [checked in] Turn-on mdimporter by default in build system v2
Keywords: checkin-needed
Comment on attachment 351818 [details] [diff] [review]
[checked in] Add mail.spotlight.enable pref

sr=me and checked in.

Checked in: http://hg.mozilla.org/comm-central/rev/cf4d93253ff6
Attachment #351818 - Attachment description: Add mail.spotlight.enable pref → [checked in] Add mail.spotlight.enable pref
Attachment #351818 - Flags: superreview?(bugzilla) → superreview+
Depends on: 468471
(In reply to comment #150)

Filed bug 468471.

Updated

9 years ago
Attachment #351823 - Flags: superreview?(benjamin) → superreview-

Comment 156

9 years ago
Comment on attachment 351823 [details] [diff] [review]
Add TB specific Mac file-handling so Spotlight can open mail from searches

This is not an acceptable solution long-term because of the MOZ_THUNDERBIRD ifdef in shared code. Is there any reason we can't use this codepath for all apps? Also, I think it would be better to use a command line `-file <path>` rather than just `path`, because different apps may interpret the bare argument differently (Firefox interprets it as a URL, for example).

This may be acceptable in general on 1.9.1 branch, but the patch is a little weird: you calculate specBuf, but don't actually use it. I'd prefer an #if/#else pair to keep the thunderbird/non-tbird codepaths completely separate (and I think you still want -file <path>)
Created attachment 352606 [details] [diff] [review]
 Add Mac -file arg handling so Spotlight can open mail files from searches   

David, can you advise me on this before I go back to bsmedberg?  Thanks.
Attachment #351823 - Attachment is obsolete: true
Attachment #352606 - Flags: review?(bienvenu)
Attachment #351823 - Flags: review?(joshmoz)
Attachment #351488 - Attachment description: Info.plist updated with public.email-message UTTypeConformsTo → [checked in] Info.plist updated with public.email-message UTTypeConformsTo
Attachment #351488 - Flags: superreview?(bugzilla)
Comment on attachment 351488 [details] [diff] [review]
[checked in] Info.plist updated with public.email-message UTTypeConformsTo

I've just realised this is in mail/ so you don't need sr for this. So I've just checked it in:

http://hg.mozilla.org/comm-central/rev/c9fe915147d1
(In reply to comment #152)
> Created an attachment (id=351886) [details]
> [checkin needed] Turn-on mdimporter by default in build system v2

humph: you missed the changes to mail/app/Makefile.in in this patch. Additionally I think we need to add the old mdimporter to removed-files.in
Created attachment 352735 [details] [diff] [review]
[checked in/removed-files.in backed out] mdimporter extra build system changes missed last time + removed-files.in

Since this bug needs more attachments, here's one that supplements attachment 351886 [details] [diff] [review] based on Mark's comment.  This should have been part of that patch, but since it's already checked in, here is the rest.

I've also added the old mdimporter to removed-files.in, but I'm not sure I've done it correctly.
Attachment #352735 - Flags: review?(bugzilla)
(Assignee)

Updated

9 years ago
Attachment #352606 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 161

9 years ago
Comment on attachment 352606 [details] [diff] [review]
 Add Mac -file arg handling so Spotlight can open mail files from searches   

this looks good, except that I'm not sure that strdup and nsMemory::Free technically go together...I've seen NS_Free used with strdup in other places.

I haven't run with this yet, I will shortly, but I think it's OK to ask josh and bsmedberg for r/sr, maybe with the NS_Free tweak, or not, depending on what bsmedberg says.
Created attachment 352771 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v3

Thanks for your feedback, Benjamin/David.  I've addressed your comments and tried to fix this in a general way vs. doing a 1.9.1 fix.  This has been tested as part of bug 468471.  I'm not sure which of the half-dozen *strdup to call, so please correct me if I'm off track.
Attachment #352606 - Attachment is obsolete: true
Attachment #352771 - Flags: superreview?(benjamin)
Attachment #352771 - Flags: review?(joshmoz)
Comment on attachment 352735 [details] [diff] [review]
[checked in/removed-files.in backed out] mdimporter extra build system changes missed last time + removed-files.in

I'm not totally convinced the removed-files.in change will work, but lets get this in and find out.

r=me, I've already pushed this:

http://hg.mozilla.org/comm-central/rev/8d7c26f297d7
Attachment #352735 - Attachment description: mdimporter extra build system changes missed last time + removed-files.in → [checked in] mdimporter extra build system changes missed last time + removed-files.in
Attachment #352735 - Flags: review?(bugzilla) → review+
Comment on attachment 352735 [details] [diff] [review]
[checked in/removed-files.in backed out] mdimporter extra build system changes missed last time + removed-files.in

I had to back out the removed-files.in change - it wasn't working (I think because its a directory not a file). Not sure we can do much about this.

Changeset http://hg.mozilla.org/comm-central/rev/db12f888c51f
Attachment #352735 - Attachment description: [checked in] mdimporter extra build system changes missed last time + removed-files.in → [checked in/removed-files.in backed out] mdimporter extra build system changes missed last time + removed-files.in

Comment 165

9 years ago
Comment on attachment 352771 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v3

>diff --git a/toolkit/xre/nsCommandLineServiceMac.cpp b/toolkit/xre/nsCommandLineServiceMac.cpp

>+  nsCOMPtr<nsILocalFile> workingDir;

I don't see you actually initializing workingDir anywhere... am I missing something? If not, just pass `nsnull` below and skip the COMPtr.

>+  char *urlPtr = ToNewCString(filePath);
>+  char **argv = new char *[3];
>+  char *arg1 = strdup("-file");
>+  argv[0] = nsnull;
>+  argv[1] = arg1;
>+  argv[2] = urlPtr;

You don't need to malloc any of this... it's all inparams:

const char *argv[3] = {nsnull, "-file", filePath.get()};

>+  if (NS_FAILED(rv))
>+    return errAEEventNotHandled;
>+  return cmdLine->Run();

this last "return" doesn't look right. cmdLine->Run returns an nsresult, and this method returns an OSErr, right?

Does Firefox ever hit this codepath? Do we need to make Firefox handle -file params?
Attachment #352771 - Flags: superreview?(benjamin)
Attachment #352771 - Flags: superreview-
Attachment #352771 - Flags: review?(joshmoz)
Created attachment 357392 [details] [diff] [review]
 Add Mac -file arg handling so Spotlight can open mail files from searches v4

Thanks for your comments, Benjamin.  I've done more testing, and indeed, this needs a -file case for Firefox, which I've added.  I fixed the malloc issues (NOTE: I couldn't use filePath.get() due to const vs. non-const).  After speaking with Josh on irc, I've updated the return type, and also fixed the one above (NOTE: the caller doesn't check the return type, which is why this was working before).  I've also removed the COMPtr for workingDir as suggested.

Hopefully this is closer, if not all the way there.  Thanks for reviewing again.
Attachment #352771 - Attachment is obsolete: true
Attachment #357392 - Flags: superreview?(benjamin)
Attachment #357392 - Flags: review?(joshmoz)
Created attachment 357496 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v5

I forgot to ifdef for mac in the command line handler--fixed.  See other details in comment 166.
Attachment #357392 - Attachment is obsolete: true
Attachment #357496 - Flags: superreview?(benjamin)
Attachment #357496 - Flags: review?(joshmoz)
Attachment #357392 - Flags: superreview?(benjamin)
Attachment #357392 - Flags: review?(joshmoz)

Comment 168

9 years ago
Comment on attachment 357496 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v5

>diff --git a/browser/components/nsBrowserContentHandler.js b/browser/components/nsBrowserContentHandler.js
>--- a/browser/components/nsBrowserContentHandler.js
>+++ b/browser/components/nsBrowserContentHandler.js
>@@ -473,16 +473,28 @@ var nsBrowserContentHandler = {
>       cmdLine.preventDefault = true;
> 
>     var searchParam = cmdLine.handleFlagWithParam("search", false);
>     if (searchParam) {
>       doSearch(searchParam, cmdLine);
>       cmdLine.preventDefault = true;
>     }
> 
>+#ifdef XP_MACOSX
>+    var fileParam = cmdLine.handleFlagWithParam("file", false);
>+    if (fileParam) {
>+      var uri = resolveURIInternal(cmdLine, fileParam);
>+      if (shouldLoadURI(uri))
>+        openWindow(null, this.chromeURL, "_blank",
>+                   "chrome,dialog=no,all" + this.getFeatures(cmdLine),
>+                   uri.spec);
>+      cmdLine.preventDefault = true;
>+    }
>+#endif
>

1) Why make this block mac-only? Seems that people might want to open a file on any OS.
2) I don't understand the codepath. To turn a parameter into a file, use nsICommandLine.resolveFile, and then turn that nsIFile into a nsIURI using the IOService.
Attachment #357496 - Flags: superreview?(benjamin) → superreview-

Updated

9 years ago
Attachment #357496 - Flags: review?(joshmoz)
Created attachment 358412 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v6

Thanks for the feedback.  Here's a new patch:

1) Removed ifdef for osx
2) Switched to using nsICommandLine.resolveFile and the IOService
Attachment #357496 - Attachment is obsolete: true
Attachment #358412 - Flags: superreview?(benjamin)
Attachment #358412 - Flags: review?(joshmoz)

Updated

9 years ago
Attachment #358412 - Flags: superreview?(benjamin) → superreview-

Comment 170

9 years ago
Comment on attachment 358412 [details] [diff] [review]
Add Mac -file arg handling so Spotlight can open mail files from searches v6

>diff --git a/browser/components/nsBrowserContentHandler.js b/browser/components

>+    var fileParam = cmdLine.handleFlagWithParam("file", false);
>+    if (fileParam) {
>+      var file = cmdLine.resolveFile(fileParam);
>+      var ios = Components.classes["@mozilla.org/network/io-service;1"]
>+                          .getService(Components.interfaces.nsIIOService);
>+      var uri = ios.newFileURI(file);
>+      
>+      if (shouldLoadURI(uri))

You can skip the shouldLoadURI check here, since we already know that this isn't a chrome URI.

>diff --git a/toolkit/xre/nsCommandLineServiceMac.cpp b/toolkit/xre/nsCommandLineServiceMac.cpp

>+  char *argv[3] = {nsnull, "-file", ToNewCString(filePath)};
>+  rv = cmdLine->Init(3, argv, nsnull, nsICommandLine::STATE_REMOTE_EXPLICIT);
>+  if (NS_FAILED(rv))
>+    return errAEEventNotHandled;
>+  rv = cmdLine->Run();
>+  return (NS_SUCCEEDED(rv)) ? noErr : errAEEventNotHandled;

You're leaking the ToNewCString(filePath) buffer. I really think you want

const char *argv[3] = {nsnull, "-file", filePath.get()} and then cast it to char** later if you need to (we don't actually modify the pointers).

r=me with those changes
Created attachment 358454 [details] [diff] [review]
[Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7

1) removed shouldLoadURI check
2) fixed leak with ToNewCString(filePath) using bsmedberg's suggested method

Also, in testing this more, I realized that resolveFile is throwing an exception because I was previously not setting a workingDir--the old code path didn't need it, but this new one does.  I've added the working dir, so including bsmedberg again in case there are suggestions about how I do it.
Attachment #358412 - Attachment is obsolete: true
Attachment #358454 - Flags: superreview?(benjamin)
Attachment #358454 - Flags: review?(joshmoz)
Attachment #358412 - Flags: review?(joshmoz)

Updated

9 years ago
Attachment #358454 - Flags: superreview?(benjamin) → superreview+

Updated

9 years ago
Flags: wanted-thunderbird3? → wanted-thunderbird3+
Keywords: helpwanted
Hardware: PowerPC → All

Updated

9 years ago
Attachment #358454 - Flags: review?(joshmoz) → review+
Attachment #358454 - Attachment description: Add Mac -file arg handling so Spotlight can open mail files from searches v7 → [checkin-needed] Add Mac -file arg handling so Spotlight can open mail files from searches v7
Keywords: checkin-needed
Whiteboard: [checkin post gecko 1.9.1 beta 3 freeze restrictions]
Comment on attachment 358454 [details] [diff] [review]
[Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7


http://hg.mozilla.org/mozilla-central/rev/c56446e2e61d
Attachment #358454 - Attachment description: [checkin-needed] Add Mac -file arg handling so Spotlight can open mail files from searches v7 → Add Mac -file arg handling so Spotlight can open mail files from searches v7 [Checkin: Comment 172]
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [checkin post gecko 1.9.1 beta 3 freeze restrictions]
Version: unspecified → Trunk
Attachment #358454 - Flags: approval1.9.1?
Comment on attachment 358454 [details] [diff] [review]
[Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7

I'm wondering if this can make it into 1.9.1 too.  This is needed for TB3 and will also have a positive effect for other XULRunner apps that need/want to use -file handling.

The risk of this patch is low, as it keeps current ff behaviour unchanged.
Re-opening until the other bits get through review and land.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

9 years ago
Blocks: 360488

Comment 175

9 years ago
Is the Spotlight integration testable yet? 

If so, I've seen different instructions for turning it on at different times, and wonder if all it needs is turning it on in Config editor, or whether something else is required now.
(In reply to comment #175)
> Is the Spotlight integration testable yet? 

Yes, I'm pleased to say!  We're just waiting on a mozilla-central 1.9.1 landing, and we can land everything.  Here's what you need to do in order to try this for yourself:

1 - Apply attachment 358454 [details] [diff] [review] from here to your mozilla/ dir (adds -file handling)
2 - Apply attachment 366110 [details] [diff] [review] from bug 468471 to your comm-central tree (hooks -file handling into tb startup for cpp)
3 - Apply attachment 357819 [details] [diff] [review] from bug 468471 to your comm-central tree (fixes spotlight search code to save/read from proper directory)
4 - Set the pref mail.spotlight.enable to true
5 - Get your mail indexed (fastest way is to send yourself a message, but it will happen eventually on idle if you're OK to wait)

I've been running this, Sid has tried, and Standard8 too.  Let us know if you hit any snags.

Comment 177

9 years ago
Thanks - for the instructions, ..... but I'm testing, not developing, I don't have a source tree. Looking forward to testing it.

Comment 178

9 years ago
(In reply to comment #176)
Just step 3 seems to allow spotlight to search for emails. However, on my Intel MacBook 2006, 10.4.11, it seems that when I open the result, OS X tries to handle it with Mail.app (no account set up with Mail.app so it just nags me to create an account). 

Any ideas when the attachments might be applied to the main trunk?

Comment 179

9 years ago
The files that Spotlight will index are located in ~/Library/Caches/Metadata, right? And what will happen if you clean your user cache, maybe with systemtools (Onyx, Cocktail...)?
(In reply to comment #178)
> Any ideas when the attachments might be applied to the main trunk?

As I said above, we are waiting on a 1.9.1 landing for the -file handling code in mozilla-central.  I have no idea on timing, and no guarantee they will take it.

(In reply to comment #179)
> The files that Spotlight will index are located in ~/Library/Caches/Metadata,
> right? And what will happen if you clean your user cache, maybe with
> systemtools (Onyx, Cocktail...)?

The point of ~/Library/Caches/Metadata is that it can be deleted--it's not your mail, just an "index" into it.  If it does get removed, we will re-index it again in time as the app sits idle.  Sid, correct me if I'm wrong, but this is my understanding.
(In reply to comment #180)
> The point of ~/Library/Caches/Metadata is that it can be deleted--it's not your
> mail, just an "index" into it.  If it does get removed, we will re-index it
> again in time as the app sits idle.  Sid, correct me if I'm wrong, but this is
> my understanding.

Actually, no, we won't reindex because we don't check for the presence of the actual file (we keep a property instead, and let that do it's job). Maybe we should check for the presence of the .mozmsgs folder, and reindex if the folder is missing. Could you file a bug?
Depends on: 482724
(In reply to comment #181)
> is missing. Could you file a bug?

Filed bug 482724.

Comment 183

9 years ago
(In reply to comment #180) 
> As I said above, we are waiting on a 1.9.1 landing for the -file handling code
> in mozilla-central.  I have no idea on timing, and no guarantee they will take
> it.

Thanks for the reply. I hope they do. :)
Comment on attachment 358454 [details] [diff] [review]
[Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7

a191=beltzner, humph told me he'd write a litmus test for this.
Attachment #358454 - Flags: approval1.9.1? → approval1.9.1+
Blocks: 482779
(In reply to comment #184)
> (From update of attachment 358454 [details] [diff] [review])
> a191=beltzner, humph told me he'd write a litmus test for this.

Filed bug 482779 for the tests.
No longer blocks: 482779
Comment on attachment 358454 [details] [diff] [review]
[Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7

Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1fa7c792c50c
Attachment #358454 - Attachment description: Add Mac -file arg handling so Spotlight can open mail files from searches v7 [Checkin: Comment 172] → [Checked in c172,c186] Add Mac -file arg handling so Spotlight can open mail files from searches v7
So based on the patches from bug 468471 landing, this is now fixed.  Until we get proper UI, you can enable with mail.spotlight.enable=true.  Please report any related issues in new bugs.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Is there a bug filed on getting proper UI and/or considering making true be the default for this pref?
(In reply to comment #188)
> Is there a bug filed on getting proper UI and/or considering making true be the
> default for this pref?

See bug 468373 and bug 467116.
Having said that, I'd favour turning the pref on by default.  I'll do the patch if it will get accepted.  Thoughts?
Let's discuss in bug 468373.

Comment 192

9 years ago
Also - if this is now "fixed", I'm wondering whether turning it on should automatically start a re-index, I'm noticing that Spotlight only seems to find messages that have arrived since the date I turned on the preference (or should that be a seperate bug?)
(In reply to comment #192)
> Also - if this is now "fixed", I'm wondering whether turning it on should
> automatically start a re-index, I'm noticing that Spotlight only seems to find
> messages that have arrived since the date I turned on the preference (or should
> that be a seperate bug?)

It should index new messages almost immediately, and older messages when left idle.
see also bug 482724

Comment 195

9 years ago
It doesn't seem to be doing the older messages - I mean messages in older folders. When I look in ~/Library/Cache/Thunderbird I find metadata for folders where there has been recent activity, but not for older ones.

Updated

9 years ago
Keywords: fixed1.9.1
For the next time: Please file a bug on SeaMonkey or at least drop a note in #seamonkey on IRC that a new command line flag was introduced on Mac so that we can fix our code (SeaMonkey also has a nsBrowserContentHandler.js file).

Comment 197

7 years ago
Is there any wiki that explanes how the Spotlight integration works? I ask because I was investigating if the Thunderbird.mdimporter is till needed for the functionality, because I couldn't find it on my hard drive. But I find no informations on how the Spotlight integration works.
You need to log in before you can comment on or make changes to this bug.