517238 - Implement Minimal APIs needed for Enigmail.

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Philip Chee]

Followup to Bug 516453. Extend gFolderDisplay and gMessageDisplay to support Enigmail.

Comment 1

[Philip Chee]

Attachment: Patch eV1.0 APIs for Enigmail (MQ patch)

Note: this patch applies on top of attachment 401393 [details] [diff] [review] from Bug 516453.

What Enigmail apparently needs:

> gFolderDisplay.selectedCount
> gFolderDisplay.selectedMessages
> gFolderDisplay.selectedMessage
> gFolderDisplay.selectedMessageUris
> gFolderDisplay.messageDisplay.visible
> gFolderDisplay.selectedMessageIsNews
> gFolderDisplay.selectedMessageIsImap

I also did:

gFolderDisplay.selectedMessageIsFeed
gFolderDisplay.selectedMessageIsExternal
gFolderDisplay.selectedIndices

Not tested with Enigmail but I exercised all these methods on the 3pane tab and they all returned what looked like reasonable data without throwing.

Tobias, do you have the necessary setup to test these patches? Otherwise I can send you two files (messenger.xul and folderDisplay.js) that you can insert into messenger.jar from the current 2.0pre nightly.

Comment 2

[[e-mail]]

Comment on attachment 401395 [details] [diff] [review]
Patch eV1.0 APIs for Enigmail (MQ patch)

>+var nsMsgFolderFlags = Components.interfaces.nsMsgFolderFlags;
Ugh. For once, I don't like polluting the global scope!

>+  get messageDisplay()
>+  {
>+    return gMessageDisplay;
>+  },
If gMessageDisplay was declared first, you could write this as
messageDisplay: gMessageDisplay,
Or you could set it as a property at the end of the file.

>+    if (!gDBView)
>+      return 0;
>+    return gDBView.numSelected;
Nit: could use gDBView ? gDBView.numSelected : 0;

>     if (!gDBView.QueryInterface(Components.interfaces.nsITreeView).selection.count)
Hmm, should this be numSelected?

>+    return Boolean(message && message.folder &&
Bah, is some idiot actually going to compare this === to "false"?

>+    message.folder.server.type == "rss");
Nit: indentation wrong.

>+    if (!gDBView)
>+      return [];
>+    return gDBView.getIndicesForSelection({});
?: again.

>+      var enum = gDBView.getMsgHdrsForSelection().enumerate();
getMsgHdrsForSelection returns an nsIMutableArray, so use a for loop and queryElementAt. (Sadly fixIterator hasn't been fixed to work with an nsIMutableArray.)

>+    return messageArray.length ? messageArray : null;
:-)

>+  set visible()
>+  {
>+    return; // Fake setter for the time being.
Nit: setter should return the value being set.

Comment 3

[Philip Chee]

Attachment: Patch v1.1 Fix Nits.

Also in this patch, I changed all top level lets to vars.

> >+var nsMsgFolderFlags = Components.interfaces.nsMsgFolderFlags;
> Ugh. For once, I don't like polluting the global scope!

Change it to a property of gMessageDisplay.

> >+  get messageDisplay()
> >+  {
> >+    return gMessageDisplay;
> >+  },
> If gMessageDisplay was declared first, you could write this as
> messageDisplay: gMessageDisplay,
> Or you could set it as a property at the end of the file.

Since this is all rather temporary I set the property at the end of the file.

> >+    if (!gDBView)
> >+      return 0;
> >+    return gDBView.numSelected;
> Nit: could use gDBView ? gDBView.numSelected : 0;

Fixed.

> >     if (!gDBView.QueryInterface(Components.interfaces.nsITreeView).selection.count)
> Hmm, should this be numSelected?

I don't think so. numSelected is affected by |mail.operate_on_msgs_in_collapsed_threads| which I don't think is what we want here.

> >+    return Boolean(message && message.folder &&
> Bah, is some idiot actually going to compare this === to "false"?

Removed Boolean() globally.

> >+    message.folder.server.type == "rss");
> Nit: indentation wrong.

Fixed.

> >+    if (!gDBView)
> >+      return [];
> >+    return gDBView.getIndicesForSelection({});
> ?: again.

Fixed.

> >+      var enum = gDBView.getMsgHdrsForSelection().enumerate();
> getMsgHdrsForSelection returns an nsIMutableArray, so use a for loop and
> queryElementAt. (Sadly fixIterator hasn't been fixed to work with an
> nsIMutableArray.)

Fixed.

> >+  set visible()
> >+  {
> >+    return; // Fake setter for the time being.
> Nit: setter should return the value being set.

Fixed. Note Thunderbird returns null, when it bothers to return anything at all. :-?

Comment 4

[Ian Neal]

(In reply to comment #2)
> (From update of attachment 401395 [details] [diff] [review])
> >+      var enum = gDBView.getMsgHdrsForSelection().enumerate();
> getMsgHdrsForSelection returns an nsIMutableArray, so use a for loop and
> queryElementAt. (Sadly fixIterator hasn't been fixed to work with an
> nsIMutableArray.)
Hmmm, TB has:
// getMsgHdrsForSelection returns an nsIMutableArray.  We want our callers
//  to have a user-friendly JS array and not have to worry about
//  QueryInterfacing the values (or needing to know to use fixIterator).
return [msgHdr for each
          (msgHdr in fixIterator(
                      this.view.dbView.getMsgHdrsForSelection().enumerate(),
                      Components.interfaces.nsIMsgDBHdr))];

Comment 5

[Philip Chee]

The result is the same, we return a bog standard js array of nsIMsgDBHdrs.

Comment 6

[Philip Chee]

gFolderDisplay: Properties for object:
[object Object]
nsMsgFolderFlags: nsMsgFolderFlags
selectedCount: 4
selectedMessage: [xpconnect wrapped nsIMsgDBHdr]
selectedMessageIsFeed: false
selectedMessageIsImap: false
selectedMessageIsNews: true
selectedMessageIsExternal: false
selectedIndices: 8,9,10,11
selectedMessages: [xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr]
selectedMessageUris: news-message://news.mozilla.org/mozilla.test#3762,news-message://news.mozilla.org/mozilla.test#3755,news-message://news.mozilla.org/mozilla.test#3761,news-message://news.mozilla.org/mozilla.test#3760
messageDisplay: [object Object]

gFolderDisplay.selectedMessages: Properties for object:
[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr]
0: [xpconnect wrapped nsIMsgDBHdr]
1: [xpconnect wrapped nsIMsgDBHdr]
2: [xpconnect wrapped nsIMsgDBHdr]
3: [xpconnect wrapped nsIMsgDBHdr]

Comment 7

[Philip Chee]

On Shredder.

gFolderDisplay.selectedMessages: Properties for object:
[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr],[xpconnect wrapped nsIMsgDBHdr]
0: [xpconnect wrapped nsIMsgDBHdr]
1: [xpconnect wrapped nsIMsgDBHdr]
2: [xpconnect wrapped nsIMsgDBHdr]
3: [xpconnect wrapped nsIMsgDBHdr]
4: [xpconnect wrapped nsIMsgDBHdr]

Comment 8

[Ian Neal]

Comment on attachment 401616 [details] [diff] [review]
Patch v1.1 Fix Nits.

r=me, having discussed queries on IRC.

Comment 9

[[e-mail]]

(In reply to comment #3)
> > >     if (!gDBView.QueryInterface(Components.interfaces.nsITreeView).selection.count)
> > Hmm, should this be numSelected?
> I don't think so. numSelected is affected by
> |mail.operate_on_msgs_in_collapsed_threads| which I don't think is what we want
> here.
I don't suppose it matters much since you're only worried about whether or not
a selection exists.

Comment 10

[Philip Chee]

Attachment: Patch v1.2 Use numSelected (via this.selectedCount) r=iann

Carrying forward r=iann

>>> Hmm, should this be numSelected?
>> I don't think so. numSelected is affected by
>> |mail.operate_on_msgs_in_collapsed_threads| which I don't think is what we want
>> here.
> I don't suppose it matters much since you're only worried about whether or not
> a selection exists.
OK. Fixed to use numSelected (via this.selectedCount)

Comment 11

[[e-mail]]

Comment on attachment 401719 [details] [diff] [review]
Patch v1.2 Use numSelected (via this.selectedCount) r=iann

>+           (message.folder.flags & this.nsMsgFolderFlags.ImapBox) > 0;
Nit: != (both times)

>+        let hdr = array.queryElementAt(i, Components.interfaces.nsIMsgDBHdr);
>+        msgHdrs.push(hdr);
Nit: not worth a separate variable

Comment 12

[Philip Chee]

>>+        let hdr = array.queryElementAt(i, Components.interfaces.nsIMsgDBHdr);
>>+        msgHdrs.push(hdr);
> Nit: not worth a separate variable
How best to wrap the resulting line @80 then?

Comment 13

[Ian Neal]

(In reply to comment #12)
> >>+        let hdr = array.queryElementAt(i, Components.interfaces.nsIMsgDBHdr);
> >>+        msgHdrs.push(hdr);
> > Nit: not worth a separate variable
> How best to wrap the resulting line @80 then?

+        msgHdrs.push(array.queryElementAt(i,
                                           Components.interfaces.nsIMsgDBHdr));
The above fits, but looks strange, sometimes things end up being more than 80.

Comment 14

[[e-mail]]

We can live with the odd 81-char line now and then!

Comment 15

[Philip Chee]

Attachment: [for checkin] Patch 1.3 r=iann_bugzilla sr=neil

Carrying forward r=iann_bugzilla, sr=neil

> (From update of attachment 401719 [details] [diff] [review])
> >+           (message.folder.flags & this.nsMsgFolderFlags.ImapBox) > 0;
> Nit: != (both times)
Fixed. Fixed.

> >+        let hdr = array.queryElementAt(i, Components.interfaces.nsIMsgDBHdr);
> >+        msgHdrs.push(hdr);
> Nit: not worth a separate variable
Fixed.

Comment 16

[Frank Wein [:mcsmurf]]

http://hg.mozilla.org/comm-central/rev/3dbbd9a979b4