1326494 - Remove some remaining nsISupportsArray references in mailnews

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[:aceman]

Some misc references to nsISupportsArray.

1.
https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/mailnews/import/test/unit/resources/TestMailImporter.js#77 :

  FindMailboxes: function(location) {
    let result;
    result = Cc["@mozilla.org/supports-array;1"]
             .createInstance(Ci.nsIArray);
    this._collectMailboxesInDirectory(location, 0, result);

    return result;
  },

The nsIArray was converted in bug 945045 (https://hg.mozilla.org/comm-central/diff/117ad84fb83e/mailnews/import/test/unit/resources/TestMailImporter.js) but the "@mozilla.org/supports-array;1" has remained. Probably needs changing to "@mozilla.org/array;1". Actually the array contents is being changed in the functions around so it should probably be nsIMutableArray.

Then, at
https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/mailnews/import/test/unit/resources/TestMailImporter.js#66

result.AppendElement(descriptor, false);
needs to change to
result.appendElement(descriptor, false);

2. https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/suite/browser/pageinfo/pageInfo.js#220
const ARRAY_CONTRACTID          = "@mozilla.org/supports-array;1";
seems unused.

3. https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/suite/common/directory/directory.js#14
const ARRAY_CONTRACTID          = "@mozilla.org/mutable-array;1";
is the only occurence of "@mozilla.org/mutable-array;1" in whole of c-c/m-c. Is it correct or should it be "@mozilla.org/array;1" ?

4. https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/mail/base/content/mailWindowOverlay.js#2809
 // array of isupportsarrays of servers for a particular folder
Comment mentions iSupportsArray but the  pop3DownloadServersArray is an array of arrays (JS arrays).

5. https://dxr.mozilla.org/mozilla-central/rev/6f63f95e28ffc05c0d2f5ef6cd6e05905fe8ea5a/editor/libeditor/HTMLEditRules.cpp#5019
Comment 
" * and insert the inner content into the supplied issupportsarray at offset"
but the argument is a nsTArray.

Comment 1

[Philip Chee]

> 2. https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/suite/browser/pageinfo/pageInfo.js#220
> const ARRAY_CONTRACTID          = "@mozilla.org/supports-array;1";
> seems unused.
Yes, this can be removed.

> 3. https://dxr.mozilla.org/comm-central/rev/72bc94bebbabfed990212cb8959c11fe5c974e05/suite/common/directory/directory.js#14
> const ARRAY_CONTRACTID          = "@mozilla.org/mutable-array;1";
> is the only occurence of "@mozilla.org/mutable-array;1" in whole of c-c/m-c. 
> Is it correct or should it be "@mozilla.org/array;1" ?
Right. Needs to be changed. Looks like a typo introduced in
https://hg.mozilla.org/comm-central/rev/2cb52566a6b7
That no one noticed.

Comment 2

[Eric Rahm [:erahm]]

Just a heads up, I'm landing the |nsISupportsArray| removal patch in m-c next week (March 6, 2017).

Comment 3

[:aceman]

Thanks, yes I'm fixing this right now.

I now noticed point 5. is for m-c. Do you want to take it to  new bug?

Comment 4

[Eric Rahm [:erahm]]

(In reply to :aceman from comment #3)
> Thanks, yes I'm fixing this right now.
> 
> I now noticed point 5. is for m-c. Do you want to take it to  new bug?

Yeah that's fine, I can review and land it (or I can just take care of the patch).

Comment 5

[:aceman]

Attachment: patch for TB

Comment 6

[:aceman]

Attachment: patch for SM

Comment 7

[:aceman]

Attachment: patch for m-c

Comment 8

[Eric Rahm [:erahm]]

Comment on attachment 8842657 [details] [diff] [review]
patch for m-c

Review of attachment 8842657 [details] [diff] [review]:
-----------------------------------------------------------------

r=me Thanks!

Comment 9

[Ian Neal]

Comment on attachment 8842653 [details] [diff] [review]
patch for SM

LGTM r=me thanks

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 8842652 [details] [diff] [review]
patch for TB

I haven't tested it since I assume you have.

I note that bug 1318806 is also in progress.

Comment 11

[:aceman]

When running all xpcshell, it appears to me that the TestMailImporter.js functions are unused. That may be why the got away with the typos.

Comment 12

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #11)
> That may be why the got away with the typos.
Which typos (plural)? *A*ppendElement() and what else?

Comment 13

[:aceman]

The other part was creating a nsIArray from nsISupportsArray and then even adding elements to it as if it were a nsIMutableArray. The .appendElement() should not exist on nsIArray.

Comment 14

[Jorg K (CEST = GMT+2)]

I love working on dead code because you know that it's going to work 100% ;-)

Comment 15

[:aceman]

Yeah. Maybe it was used by some of the importers in the past that got removed. But I would leave it in for now in case we need it again the future.

Comment 16

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/04a1176052ddeab18e98a680dc90b55502c1eaf0
https://hg.mozilla.org/comm-central/rev/e32e7215e0e726eeef616940d4e57b3db70755e6

Dear M-C sheriff, could you please check-in "patch for m-c". It's a one-word comment change only, so really no testing required here. Thank you.

Comment 17

[Jorg K (CEST = GMT+2)]

Dear Sheriff, please fix the commit message, r=erahm (lose the ?). You might consider checking this in with DONTBUILD or combine with other patches.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/77d5a39a4677
Remove an outdated reference to nsISupportsArray in libeditor. r=erahm DONTBUILD a=tomcat