586055 - Eliminate users of browsers.item

Status: VERIFIED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Robert Kaiser]

In bug 585511, I eliminated browser.item, but we have a few callers left that should really die:
robert@robert:/mnt/mozilla/hg/comm-central> grep -rin "browsers.item" suite/
suite/browser/navigator.js:552:    for (var i = 0; browsers.item(i); i++)
suite/common/bindings/notification.xml:50:                this._activeBrowser = browsers.item(0);
suite/common/bindings/notification.xml:53:                  if (browsers.item(i).docShell) {
suite/common/bindings/notification.xml:54:                    this._activeBrowser = browsers.item(i);

Comment 1

[[e-mail]]

(In reply to comment #0)
The notification.xml hits are false positives.

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> suite/browser/navigator.js:552:    for (var i = 0; browsers.item(i); i++)

{
372   isTabContentWindow: function isTabContentWindow(aWindow) {
373     var browsers = gBrowser.browsers;
374     for (var i = 0; browsers.item(i); i++)
375       if (browsers[i].contentWindow == aWindow)
376         return true;
377     return false;
378   }
}

This code should be replaceable using "Array.some()", right?

Comment 3

[Robert Kaiser]

Serge, feel free to do a patch using this, I would have done it with something I better understand myself, but your idea might be better overall. ;-)

Comment 4

[Robert Kaiser]

Oh, if you intend to do this, please do it within the next few hours though, as this is something that should go in for the already-frozen 2.1a3, btw.

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Copy Firefox usage of "Array.some()" [Checkin: Comment 7]

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4669
already has the intended code ;->

Requesting review from Callek, as in bug 585511.

Comment 6

[Robert Kaiser]

Comment on attachment 464867 [details] [diff] [review]
(Av1) Copy Firefox usage of "Array.some()"
[Checkin: Comment 7]

Stealing this one. Please get it landed ASAP, this is probably the reason why our alive-tests on leak test boxes are hanging and need to be manually stopped. We also shouldn't ship an alpha with this.

Thanks for looking into that!

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 464867 [details] [diff] [review]
(Av1) Copy Firefox usage of "Array.some()"
[Checkin: Comment 7]

http://hg.mozilla.org/comm-central/rev/55314c861a36

Comment 8

[Robert Kaiser]

status-seamonkey2.1 is only for wanted nominations right now, as we haven't branched, no need to set it to any value, target milestone does fine atm. :)

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)
> this is probably the reason why
> our alive-tests on leak test boxes are hanging and need to be manually stopped.

Indeed, this checkin fixed
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1281643601.1281645719.10743.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/08/12 13:06:41

23733 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_bug426646.html | Test timed out.
TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug391777.html | application timed out after 330 seconds with no output
PROCESS-CRASH | /tests/content/html/document/test/test_bug391777.html | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | missing output line for total leaks!
}
on all 3 platforms :-)

(In reply to comment #8)
> status-seamonkey2.1 is only for wanted nominations right now, as we haven't
> branched

Ah, I'm used to getting unsure/wrong in this specific situation ;-<

Comment 12

[Serge Gautherie (:sgautherie)]

V.Fixed, per the duplicates, then.