The default bug view has changed. See this FAQ.

Implement Minimal APIs needed for Enigmail.

RESOLVED FIXED in seamonkey2.0

Status

SeaMonkey
MailNews: General
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

(Blocks: 1 bug, {fixed-seamonkey2.0})

Trunk
seamonkey2.0
fixed-seamonkey2.0
Dependency tree / graph
Bug Flags:
blocking-seamonkey2.0 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch in comment 15])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Followup to Bug 516453. Extend gFolderDisplay and gMessageDisplay to support Enigmail.
(Assignee)

Comment 1

8 years ago
Created attachment 401395 [details] [diff] [review]
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.
Assignee: nobody → philip.chee
Attachment #401395 - Flags: superreview?(neil)
Attachment #401395 - Flags: review?(iann_bugzilla)

Comment 2

8 years ago
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.
(Assignee)

Comment 3

8 years ago
Created attachment 401616 [details] [diff] [review]
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. :-?
Attachment #401395 - Attachment is obsolete: true
Attachment #401616 - Flags: superreview?(neil)
Attachment #401616 - Flags: review?(iann_bugzilla)
Attachment #401395 - Flags: superreview?(neil)
Attachment #401395 - Flags: review?(iann_bugzilla)

Updated

8 years ago
Flags: blocking-seamonkey2.0?

Comment 4

8 years ago
(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))];

Updated

8 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

8 years ago
The result is the same, we return a bog standard js array of nsIMsgDBHdrs.
(Assignee)

Comment 6

8 years ago
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]
(Assignee)

Comment 7

8 years ago
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

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

r=me, having discussed queries on IRC.
Attachment #401616 - Flags: review?(iann_bugzilla) → review+

Comment 9

8 years ago
(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.
(Assignee)

Comment 10

8 years ago
Created attachment 401719 [details] [diff] [review]
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)
Attachment #401616 - Attachment is obsolete: true
Attachment #401719 - Flags: superreview?(neil)
Attachment #401719 - Flags: review+
Attachment #401616 - Flags: superreview?(neil)
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
Attachment #401719 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 12

8 years ago
>>+        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

8 years ago
(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.
We can live with the odd 81-char line now and then!
(Assignee)

Comment 15

8 years ago
Created attachment 401800 [details] [diff] [review]
[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.
Attachment #401719 - Attachment is obsolete: true
Attachment #401800 - Flags: superreview+
Attachment #401800 - Flags: review+
Attachment #401800 - Flags: approval-seamonkey2.0?

Updated

8 years ago
Attachment #401800 - Flags: approval-seamonkey2.0? → approval-seamonkey2.0+
(Assignee)

Updated

8 years ago
Attachment #401800 - Attachment description: Patch 1.3 r=iann_bugzilla sr=neil → [for checkin] Patch 1.3 r=iann_bugzilla sr=neil
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Whiteboard: [patch in comment 15]

Updated

8 years ago
Flags: blocking-seamonkey2.0? → blocking-seamonkey2.0+

Updated

8 years ago
Blocks: 517996
http://hg.mozilla.org/comm-central/rev/3dbbd9a979b4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed → fixed-seamonkey2.0
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0

Updated

7 years ago
Blocks: 509324

Updated

5 years ago
Blocks: 786189

Updated

5 years ago
Blocks: 786200
You need to log in before you can comment on or make changes to this bug.