Last Comment Bug 781650 - gloda should allow search of mail folder types added by extensions
: gloda should allow search of mail folder types added by extensions
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Search (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Kent James (:rkent)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-09 14:05 PDT by Kent James (:rkent)
Modified: 2012-08-14 11:44 PDT (History)
1 user (show)
rkent: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Don't check folder type (1.37 KB, patch)
2012-08-10 15:32 PDT, Kent James (:rkent)
bugmail: review+
Details | Diff | Review

Description Kent James (:rkent) 2012-08-09 14:05:13 PDT

    
Comment 1 Kent James (:rkent) 2012-08-09 14:15:33 PDT
In index_msg.js we have the following code:
  shouldIndexFolder: function(aMsgFolder) {
    let folderFlags = aMsgFolder.flags;
    if (!(folderFlags & Ci.nsMsgFolderFlags.Mail) ||
        (folderFlags & Ci.nsMsgFolderFlags.Virtual))
      return false;
    if (!(aMsgFolder instanceof nsIMsgLocalMailFolder) &&
        !(aMsgFolder instanceof nsIMsgImapMailFolder))
      return false;

This hard-wires gloda to only support existing folder types, local and imap. But mail folder types added by an extension should also be searchable. The real reason for the "instanceof" checks here is to make sure that RSS folders (which derive from nsIMsgLocalFolder) are not searched. I propose to add an explicit check for rss type here rather than prevent other mail folders (who are correctly setting their Ci.nsMsgFolderFlags.Mail flag, unlike rss!) from also being searched.

This is similiar to the check that is added in newMailNotificationService:


169     // If it's not a mail folder we don't count it by default
170     if (!(aFolder.flags & Ci.nsMsgFolderFlags.Mail))
171       shouldCount.data = false;
172 
173     // For whatever reason, RSS folders have the 'Mail' flag
174     else if ((srv = aFolder.server) && (srv.type == "rss"))
175       shouldCount.data = false;
Comment 2 Andrew Sutherland [:asuth] 2012-08-09 15:16:33 PDT
Sounds good.
Comment 3 Kent James (:rkent) 2012-08-09 15:37:33 PDT
After I reread my comment 1, I realized that was incorrect. In fact, rss passes both tests, and gets true from shouldIndexfolder.

After IRC discussions with asuth, we could not establish a case where the instanceof checks catch anything in normal operation, so the patch will simply remove those tests.
Comment 4 Kent James (:rkent) 2012-08-10 15:32:31 PDT
Created attachment 651018 [details] [diff] [review]
Don't check folder type
Comment 5 Kent James (:rkent) 2012-08-14 11:43:37 PDT
Comment on attachment 651018 [details] [diff] [review]
Don't check folder type

Checked in http://hg.mozilla.org/comm-central/rev/6a59dd8ee39f

Note You need to log in before you can comment on or make changes to this bug.