Followup to bug 241197 -- or -- feedback from an extension developer

Andrew Sutherland asutherland at asutherland.org
Tue Mar 23 00:26:53 UTC 2010


  On 03/22/2010 02:40 PM, Jonathan Protzenko wrote:
> On 03/19/2010 03:29 AM, Andrew Sutherland wrote:
>> Direct interaction with nsIMsgDBHdr instances is not something I am 
>> trying to facilitate.  This is unfortunately at odds with most 
>> current realities but is a cost/benefit necessity to ever escape the 
>> current realities.
> This is indeed very appealing. I don't have much time right now, but I 
> can imagine submitting some patches to implement marking a message as 
> read, or similar things. Would this be a useful effort or do you plan 
> on tackling this in the near future? I'm asking because the bug is a 
> bit old already, but if there is a hope of landing such patches 
> quickly, I think it might be worth the effort for me.

This would be very useful.  As long as you don't mind writing unit tests 
at the same time, there is hope of landing such patches quickly.

I should probably be explicit that I don't need to be the one to write 
any of this code, and I certainly don't want to stand in the way of 
improving things.  The eternal issue is one of limited resources and 
competing priorities.  The closer the dot product of the direction you 
are headed and the direction I am headed is closer to 1, the greater the 
time I am willing to spend reviewing things, etc.  The better the unit 
tests, the easier the review.

I'm not sure if you were proposing writing a friendly nsIMsgDBHdr 
manipulation library independent of gloda or buying into the gloda 
vision whole hog.  While I'd be super psyched if you want a cauldron of 
gloda kool-aid since gloda would benefit from having working/tested 
nsIMsgDBHdr manipulation code in the tree, I'm also okay if you want to 
just start with a more pragmatic nsIMsgDBHdr helper library too.  (Not 
that I wouldn't write modular code with unit tests and such, I just 
would not be concerned about establishing a documented, stable 
extension-facing API independent of gloda that is nsIMsgDBHdr-centric.)


>> The good news is that there are ways to go from nsIMsgDBHdr instances 
>> to gloda instances, as you are aware.  The bad news is that there is 
>> currently no guarantee that there will be a gloda message for the 
>> message you want and the indexer does nothing to help you out.  The 
>> good news is that we're very close to being able to resolve that.
> That's one of the major pains involved with Gloda, and that accouts 
> for 90% of the bug reports I receive concerning my extension. I'd be 
> glad to hear when it's fixed! Is there any specific bug involved?

I have created the following bug to track this:
https://bugzilla.mozilla.org/show_bug.cgi?id=554190

I am not actively working it at this time; if you are interested in 
working on this bug or others, please feel free to drop a note on the 
bug and I'll be sure to avoid duplicating any work.  I'd also be very 
happy to provide suggestions or answer questions you might have about 
the code.  For example, one could easily start playing around with this 
bug by using GlodaIndexer.killActiveJob(), 
GlodaIndexer.purgeJobsUsingFilter(aFilterFunc), manipulation of the 
GlodaIndexer._indexQueue, and helper methods like 
GlodaMsgIndexer.indexFolder/GlodaMsgIndexer.indexMessages in order to 
get started with this.

My immediate gloda enhancement focuses for 3.1 are on performing 
indexing and search performance which has more to do with inefficient 
queries and tokenizer limitations and much less to do with enhancements, 
so I doubt there would be much contention.


>> This is going to be a big area for future work.  You are doing the 
>> right thing by using the existing code path when involving HTML 
>> messages (as HTML) and probably all other extension authors should 
>> follow your lead here.
> I'm going to post some directions on MDC because I think that really 
> was missing.

This would be fantastic!

>>
>> Our dream is to be able to display messages with extensions being 
>> able to contribute annotations and additional mark-up in an easy 
>> fashion.  Because there are so many security concerns and potential 
>> interaction problems between multiple extensions trying to do the 
>> same thing, I think it's going to take a very concerted effort to 
>> improve this.
> Sure. But at least providing wrappers to display a message simply will 
> already be a big step forward. Of course we're depending on bug 80713 
> here... is there any possible way to lure roc into fixing this ? ;-)

Wrappers would also be fantastic!

I think the core of the point I want to make is just that a lot of the 
code is a painful twisty maze of control flow and that it might not be 
an entirely bad call to just start from near-scratch rather than trying 
to work within the system.  Wrapping the current streaming process in a 
sane abstraction seems like an excellent effort.  Trying to do the blue 
sky dream I suggest above sounds more like a case to consider a 
near-scratch rewrite informed by the existing code.

I'm not suggesting writing an HTML parser in JavaScript, though; more 
like leveraging more modern stuff from the mozilla platform that is at 
least somewhat supported.  For example, I think jcranmer was looking 
into exposing an HTML parsing engine to script which would be much 
better than trying to improve an existing C++ HTML parser-magicker, even 
if it is driven by the same HTML parser.


> How do I "tell" Gloda that all the headers are in the same thread? I'm 
> just calling getMessageCollectionForHeaders with all the selected 
> messages from the dbView. Is that what you meant?

Yes.

>> Then do a single pass to figure out all the distinct conversations 
>> involved and ask those conversations for their messages in a single 
>> query (ex: msgQuery.conversation.apply(msgQuery, 
>> listOfDistinctConversationIdsICalculated);)
> I guess I was not aware that msgQueries has a .conversation property. 
> I know that GlodaMessages have... anyway, I'll try to re-think that, 
> and I know for sure how to do it properly. Once again, the issue is 
> that all I have as a reference is
>
> https://developer.mozilla.org/en/Thunderbird/Gloda_examples
> https://developer.mozilla.org/en/Thunderbird/Creating_a_Gloda_message_query

While I'm not going to claim the existing gloda documentation is the 
be-all and end-all, the creating a gloda message query page does mention 
the conversation property.  In general, any attribute exposed on a gloda 
object should also able to be queried upon.

The unit tests have some potentially useful examples for querying on 
messages:
http://mxr.mozilla.org/comm-central/source/mailnews/db/gloda/test/unit/base_query_messages.js

Probably the best way for me to improve the documentation is for you to 
shoot me an e-mail or ping me on IRC with your question and I'll update 
the documentation in response.  Don't worry about investigating things 
before asking.

> I was thinking this because of:
>
> http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#311
>
> 307  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#307>            let[text  <http://mxr.mozilla.org/comm-central/ident?i=text>,meta  <http://mxr.mozilla.org/comm-central/ident?i=meta>] =mimeMsgToContentSnippetAndMeta  <http://mxr.mozilla.org/comm-central/ident?i=mimeMsgToContentSnippetAndMeta>(aMimeMsg  <http://mxr.mozilla.org/comm-central/ident?i=aMimeMsg>,
> 308  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#308>                                                              aMsgHdr  <http://mxr.mozilla.org/comm-central/ident?i=aMsgHdr>.folder  <http://mxr.mozilla.org/comm-central/ident?i=folder>,
> 309  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#309>                                                              SNIPPET_LENGTH  <http://mxr.mozilla.org/comm-central/ident?i=SNIPPET_LENGTH>);
> 310  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#310>            snippetNode  <http://mxr.mozilla.org/comm-central/ident?i=snippetNode>.textContent  <http://mxr.mozilla.org/comm-central/ident?i=textContent>  =text  <http://mxr.mozilla.org/comm-central/ident?i=text>;
> 311  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#311>            if(meta  <http://mxr.mozilla.org/comm-central/ident?i=meta>.author  <http://mxr.mozilla.org/comm-central/ident?i=author>)
> 312  <http://mxr.mozilla.org/comm-central/source/mail/base/content/selectionsummaries.js#312>              authorNode  <http://mxr.mozilla.org/comm-central/ident?i=authorNode>.textContent  <http://mxr.mozilla.org/comm-central/ident?i=textContent>  =meta  <http://mxr.mozilla.org/comm-central/ident?i=meta>.author  <http://mxr.mozilla.org/comm-central/ident?i=author>;

Ah.  Yeah, that's very misleading.  I think davida was coding explicitly 
against the bugzilla gloda plugin there.


>> Thunderbird 3.1 supports the new element.classList magic and the ship 
>> has sailed on Thunderbird 3.0 for adding that so that extensions can 
>> always assume it is present, so classList is the way to go:
>> https://developer.mozilla.org/en/DOM/element.classList
>
> Wonderful, I wasn't aware of that as well :(. I guess I didn't read 
> the release notes closely enough.

I only knew about it because of a recent planet.mozilla.org blog post 
about Firefox 3.6's support of the feature :)

Andrew
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/tb-planning/attachments/20100322/fcd61cdc/attachment.html>


More information about the tb-planning mailing list