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

RESOLVED FIXED in Firefox 6

Status

Firefox Graveyard
Panorama
P4
normal
RESOLVED FIXED
7 years ago
a year ago

People

(Reporter: iangilman, Assigned: hobophobe)

Tracking

Trunk
Firefox 6

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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).
(Assignee)

Comment 1

6 years ago
Created attachment 525149 [details] [diff] [review]
Allows iQ.each to complete early if callback returns false, updates groupitems.js to do so.
Attachment #525149 - Flags: review?(ian)
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+
(Reporter)

Comment 3

6 years ago
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!
(Assignee)

Comment 5

6 years ago
Created attachment 526308 [details] [diff] [review]
Same patch, but prepared for pushing.
Attachment #525149 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e58b0ab494db
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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.