1320549 - iteratorUtils.js in ABSearchDialog.js not found

Status: RESOLVED FIXED

(MailNews Core :: Search, defect)

Description

[:aceman]

In bug 857230 I mistyped iteratorUtils.js instead of iteratorUtils.jsm

This now throws errors in ABSearchDialog.js:

NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import] 1 ABSearchDialog.js:6
	<anonymous> chrome://messenger/content/ABSearchDialog.js:6:1

ReferenceError: reference to undefined property "undefined" 1 ABSearchDialog.js:57:3

TypeError: Components.classes[searchSessionContractID] is undefined 1 ABSearchDialog.js:57:3
	searchOnLoad chrome://messenger/content/ABSearchDialog.js:57:3
	onload chrome://messenger/content/ABSearchDialog.xul:1:1

Comment 1

[:aceman]

Attachment: patch

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8814679 [details] [diff] [review]
patch

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

::: mail/base/content/ABSearchDialog.js
@@ -180,5 @@
>      var searchUri = currentAbURI + "?(";
>  
> -    for (let searchTerm of fixIterator(gSearchSession.searchTerms, nsIMsgSearchTerm)) {
> -      // get the "and" / "or" value from the first term
> -      if (i == 0) {

You didn't spot that 'i' when you introduced the fixIterator, right? How about you just scrap the fixIterator and go back to the original loop controlled by 'i'? Pulling it out before the loop and controlled with a searchTerms.length > 0 appears mildly unelegant ;-)

Comment 3

[Jorg K (CEST = GMT+2)]

Damage done here: https://hg.mozilla.org/comm-central/rev/07171d081841#l1.32

Comment 4

[:aceman]

Pulling it out of the loop seems semantically better per the comment, we want to set it once and be done with it, no need to check i==0 for all the array members.

But yes, I can revert it back :)

Comment 5

[:aceman]

Attachment: patch v2

The beauty of the fixIterator is that it hides away what type of array we are handling. Now we hardcoded calls to .length and queryElementAt() so if it ever stops being a nsIArray we need to change the attributes/methods again.
Had it been fixIterator before bug 857230 we wouldn't even need to touch this loop when switching from nsISupportArray.

That seems to be also the point of implementing .Count() and GetElementAt() on a nsIArray so that the methods are named the same as on nsISupportsArray. The comment in nsIArrayExtensions says so.

But I don't think we have a general plan whether to migrate to fixIterator or away from it, so it doesn't matter much.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8814698 [details] [diff] [review]
patch v2

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

Can we land this now since something is broken right now.

Comment 7

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/1c4780bb98f563027ac46fbe74c25c770c590733
Landed to fix the regression. Since this was merely returning to the pre bug 857230 state, I think we can live without a further review.

Comment 8

[Philip Chee]

Danke!