Closed Bug 635311 Opened 11 years ago Closed 10 years ago

iQ(".appTabIcon", this.$appTabTray).each() calls in groupitems.js should return early if possible

Categories

(Firefox Graveyard :: Panorama, defect, P4)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 6

People

(Reporter: iangilman, Assigned: hobophobe)

Details

Attachments

(1 file, 1 obsolete file)

There are 3 such loops, and they're all looking for a single entry. Once they've found that entry, they should return early. This is currently not possible, because iQ.each doesn't have a mechanism for early return. It should return early if the callback returns false which is what jQuery does:

http://api.jquery.com/each/

Note that in order to avoid JavaScript strict errors, the other code path in the loop has to return true (rather than nothing).
Comment on attachment 525149 [details] [diff] [review]
Allows iQ.each to complete early if callback returns false, updates groupitems.js to do so.

Looks good, although that "for (...) {}" looks a bit strange to me :)
Attachment #525149 - Flags: feedback+
Comment on attachment 525149 [details] [diff] [review]
Allows iQ.each to complete early if callback returns false, updates groupitems.js to do so.

I agree the for(...) {} construct is a bit unusual, but it seems fine.
Attachment #525149 - Flags: review?(ian) → review+
Assignee: nobody → unusualtears
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Hey Adam, it would be great to get this fix into the tree. Here is how to prepare the patch:

http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Thank you!
Attachment #525149 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e58b0ab494db
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: Future → Firefox 6
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.