Last Comment Bug 635311 - iQ(".appTabIcon", this.$appTabTray).each() calls in groupitems.js should return early if possible
: iQ(".appTabIcon", this.$appTabTray).each() calls in groupitems.js should retu...
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P4 normal
: Firefox 6
Assigned To: Adam [:hobophobe]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-18 11:05 PST by Ian Gilman [:iangilman]
Modified: 2016-04-12 14:00 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Allows iQ.each to complete early if callback returns false, updates groupitems.js to do so. (3.83 KB, patch)
2011-04-11 13:32 PDT, Adam [:hobophobe]
ian: review+
ttaubert: feedback+
Details | Diff | Splinter Review
Same patch, but prepared for pushing. (4.14 KB, patch)
2011-04-15 10:59 PDT, Adam [:hobophobe]
no flags Details | Diff | Splinter Review

Description Ian Gilman [:iangilman] 2011-02-18 11:05:33 PST
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 1 Adam [:hobophobe] 2011-04-11 13:32:59 PDT
Created attachment 525149 [details] [diff] [review]
Allows iQ.each to complete early if callback returns false, updates groupitems.js to do so.
Comment 2 Tim Taubert [:ttaubert] 2011-04-12 05:55:05 PDT
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 :)
Comment 3 Ian Gilman [:iangilman] 2011-04-13 17:47:52 PDT
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.
Comment 4 Tim Taubert [:ttaubert] 2011-04-14 15:12:24 PDT
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!
Comment 5 Adam [:hobophobe] 2011-04-15 10:59:57 PDT
Created attachment 526308 [details] [diff] [review]
Same patch, but prepared for pushing.

Note You need to log in before you can comment on or make changes to this bug.