513319 - add a message tag event notification for add-ons

Status: RESOLVED DUPLICATE of bug 1627604

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

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

Attachment: convert OnTagsChange to observer notification

OnTagsChange seems to be the function called when a tag is added/removed on a message.  It would be nice if this was converted into an event notification such that add-ons could listen for tag changes as well.

http://mxr.mozilla.org/comm-central/ident?i=OnTagsChange

I created a first pass at a patch for making this change.  However I'm not the right person to undertake this kind of patch work so I'm adding dmose in hopes that he could take a look at the patch.

Comment 1

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

Just as an experience note.  I tried using ToggleMessageTag(key, addKey) [1] however it didn't work for me.  Likely it was just some poor programming but for the case of removing a tag from a single message being displayed it was easier to write my own tag remove function and then send the MsgTagsChanged notification out.

[1] http://mxr.mozilla.org/comm-central/source/mail/base/content/mailWindowOverlay.js#575

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I don't think we'd block on this if it were the last bug standing.  Marking wanted so that if I get bandwidth, I can come back and have a look at this.

Comment 3

[Günter]

Don't understand the problem with adding/removing tags.
We use that service with ReminderFox and it works with the standard code base.
And AFAIR those routines are also used with Postbox, also no problem there.

So what's the point with tagService??

Comment 4

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

(In reply to comment #3)
> Don't understand the problem with adding/removing tags.

The problem right now is that if a user presses one of the keyboard commands to add a tag there is no way for your extension to notice.  With the compact header extension it doesn't show tags in the compact views so this isn't a problem.  The detailed header view uses function calls to itself to be alerted of tags being added.  When you add or remove tags from your own function calls you can make sure to refresh your own display but you would have to also catch the keyboard commands: 1,2,3,4,5, and 0 in order to also intercept tagging via the keyboard.

I would think that the the tagService would provide this kind of notification support but it doesn't appear to handle that.

Comment 5

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

Phil or Magnus, do you have time to take a look at this?  I'm not even sure it's the right direction so if you have some comments on a good approach I could try coding that up.

Comment 6

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

Comment on attachment 397320 [details] [diff] [review]
convert OnTagsChange to observer notification

Would you have a chance to review this?

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 397320 [details] [diff] [review]
convert OnTagsChange to observer notification

Basically looks ok but it's bitrotted.

>+  // Add a message tag observer so we can update tag changes in the header 

Trailing space.

>+  observe: function tags_change_observe(aSubject, aTopic, aData) {

camelCase for the the function name would be more common i think.

Comment 8

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

Attachment: updated tag observer patch

(In reply to comment #7)
> (From update of attachment 397320 [details] [diff] [review])

Thanks for the review!  Sorry for the slow turnaround.

> Basically looks ok but it's bitrotted.

fixed and updated.

> >+  // Add a message tag observer so we can update tag changes in the header 
> 
> Trailing space.

removed in both mail and suite locations

> >+  observe: function tags_change_observe(aSubject, aTopic, aData) {
> 
> camelCase for the the function name would be more common i think.

changed to msgTagChangeObserve

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 406474 [details] [diff] [review]
updated tag observer patch

>+  // Add a message tag observer so we can update tag changes in the header

Nit: dot at the end of the sentence.

>+      // now update the expanded header view to rebuild the tags,
>+      // and then show or hide the tag header box.

Nit: capitalize.

r=me for the mail/ bits

Comment 10

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

Attachment: patch - version 3 with nits addressed

(In reply to comment #9)
> (From update of attachment 406474 [details] [diff] [review])
> >+  // Add a message tag observer so we can update tag changes in the header
> 
> Nit: dot at the end of the sentence.

done in both mail and suite

> >+      // now update the expanded header view to rebuild the tags,
> >+      // and then show or hide the tag header box.
> 
> Nit: capitalize.

done

> r=me for the mail/ bits

Thanks!  carrying forward r+

Comment 11

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

Comment on attachment 406725 [details] [diff] [review]
patch - version 3 with nits addressed

Neil, can I get your sr on the suite piece?

I hope I'm doing this request correctly.

Comment 12

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

If we can aim for the rc1 this will help out the header extensions quite a bit

Comment 13

[[e-mail]]

Comment on attachment 406725 [details] [diff] [review]
patch - version 3 with nits addressed

The parameters to notifyObservers are nsISupports aSubject, string aTopic and wstring aData. Unfortunately for aSubject you pass a JS array (either of nsIMsgHdr objects or strings, depending on the app), and for aData you pass an nsIMutableArray. (And that array does not even contain all of the messages when you are working on a cross-folder virtual folder.)

The other potential issue with this patch is that when you have multiple windows open then they will all update themselves when you dispatch the notification, and there's no way for the observer to know which window actually generated the notification, except by assuming it's the active window.

Comment 14

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

I shouldn't be the assignee for these bugs.  Filter against clarkbfilter to delete all these from your emails.

Comment 15

[nightalf.iq]

So is this issue fixed? I am developing an extension for thunderbird and I totally need to execute my own function when a tag is set on an email. But I have no idea how do I listen to such event. I would appreciate any hints.

Comment 16

[Magnus Melin [:mkmelin]]

No it's not fixed, still NEW.

Comment 17

[John Bieling (:TbSync)]

Provided by bug 1627604.