Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.I am using views for a site which requires Prototype as well (I'm using the jquery_compability_module, which seems to be working). Prototype (as well as potentially other js libraries) extends the Array object with additional methods / properties. The views js uses the following form to iterate though what are known to be arrays:
for (i in my_array)
for example (in js/dependent.js):
for (i in Drupal.Views.dependent.activeTriggers) {
jQuery(Drupal.Views.dependent.activeTriggers[i]).unbind('change');
}
where Drupal.Views.dependent.activeTriggers is known to be an array. When the array is empty, but overloaded with additional properties due to Prototype, this code will iterate through the Array object's properties (i.e. function names), which causes javascript errors.
I fixed this by moving this form to the more traditional:
for (var i = 0; i < Drupal.Views.dependent.activeTriggers.length; i++)
for all instances where it was an array that was being iterated over.
Here is some more info on the larger issue:
http://stackoverflow.com/questions/500504/javascript-for-in-with-arrays/...
Patch file is attached to ticket.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | views-514128-16.patch | 1.5 KB | sk33lz |
| #13 | views-issue-514128-2.patch | 3.51 KB | pascalduez |
| #11 | views-issue-514128.patch | 392 bytes | pascalduez |
| #5 | views_in_array.patch | 3.75 KB | dawehner |
| #3 | views_in_array.patch | 3.74 KB | dawehner |
Comments
Comment #1
dawehnerWhy not use $.each(). As far as i know its a quite fast version of foreach.
Additional it will not break with Prototype.
Comment #2
athoune commentedit's not a bug with Prototype framework, it's a bug for every one wich add stuff in Array.prototype . The "for a in something" iter over object's attributes, not only array keys.
Comment #3
dawehnerThis code does not ease $.each
Comment #4
merlinofchaos commentedUnfortunately this patch does not apply now. I think at least one of the loops has been changed for this very reason, even though nobody understood why the change. Now that I understand what's going on, I agree with this change.
Can I get a reroll?
Comment #5
dawehnerrerolled the patch
Comment #6
merlinofchaos commentedUnsurprisingly does not apply to D7. Committed to D6.
Comment #7
merlinofchaos commentedReverted this patch.
Not all of the for () loops replaced were for arrays, some were for objects and this syntax broke them.
Comment #8
eugenmayer commentedi would actually set this one on wontfix.
https://developer.mozilla.org/en/JavaScript/Reference/Operators/Special_...
In is used in the correct context here and also operates through _objects_ and not only arrays. This "fix" can probably brake a lot other things ( beside the typo in the last for statement...forgetting the .lenght ).
in is used like in the specifications and if prototype does brake it, its not the fault of views. In any case, use $.each can help here a lot
Comment #9
dawehner@eugenmayer
Does $.each also works with objects?
Comment #10
eugenmayer commentedyes Daniel, it does.
Comment #11
pascalduez commentedHi,
While this issue if more globally related to all "for in" loops I also noticed some missing "var" keywords, resulting in the variable being declared as global. This should really be avoided, especially with variable names like "i".
http://bonsaiden.github.com/JavaScript-Garden/#function.scopes
Patch attached for the base.js file.
As for this issue, there's not much to do to prevent "for in" to iterate over the object / array prototype except the use of object.hasOwnProperty().
Or use jQuery.each()
Comment #12
damien tournoud commentedLast time I checked, jQuery.each() iterated over all the properties, not only the own properties.
Comment #13
pascalduez commentedThis patch does not fix the prototype issue, just make sure "var" is used in the loops. I thought this would not deserve a separate issue ?
I corrected only one loop in the previous patch, but noticed the same problem at several other places.
Re-worked it.
Edit:
@Damien Confirmed, jQuery.each() does loop over all properties, including the inherited ones:
http://bugs.jquery.com/ticket/6132
Comment #14
damien tournoud commentedCorrection to #12: if the object enumerated by jQuery.each() is an array-ish, it is enumerated numerically, as an array. The way I see it, that makes this function potentially dangerous to use: it does what a non Javascript programmer would expect it to do on array-ish structures, but not on objects.
Comment #15
pascalduez commentedI've been trying to secure each "for in" loops with conditional hasOwnProperty().
But there's also some jQuery.each() used, i.e. in the "ajax_views.js". So all in one it's actually adding quite some code...
This issue is more of the decision making type, as extending Object.prototype is considered a bad practice. Moreover, even with views js patched, good chances are it will break on other parts / scripts, including core ones.
In jQuery they made the choice not to support it.
Comment #16
sk33lz commentedRe-rolled patch in #13 for latest dev release.
Comment #17
chris matthews commentedThe Drupal 6 branch is no longer supported, please check with the D6LTS project if you need further support. For more information as to why this issue was closed, please see issue #3030347: Plan to clean process issue queue