Got an issue on working ctools with the others external script, which extends Array object. For example:

Array.prototype.contains=function(obj) {
  for(var i=0; i<this.length; i++) {
    if(this[i] === obj) return i;
  }
  return -1;
};

the problem is with 

Drupal.CTools.AJAX.commands.scripts = function(data) {
...
  for (i in data.argument) {

we are processing an array as an object and as the result it have "function contains()" in values, when it passed to Drupal.CTools.AJAX.getPath it breaks javascript when it tries get indexOf('?') from function.

there are two possible solutions how to fix it:
1. add validation of value in Drupal.CTools.AJAX.getPath
2. replace for ... in with for loop

both of the fixes added to patch.

CommentFileSizeAuthor
#4 array-extender-breaks-ajax-responder-1401518-4.patch861 bytesgumanist
#2 array-extender-breaks-ajax-responder-1401518-2.patch853 bytesgumanist
array-extender-breaks-ajax-responder.patch652 bytesgumanist

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented

for..in iteration should be used only for Objects!!!

+++ b/js/ajax-responder.js
@@ -342,7 +342,7 @@
-    if (!link) {
+    if (!link || (typeof(link) != 'string')) {

This is over-engineering

+++ b/js/ajax-responder.js
@@ -458,7 +458,7 @@
-    for (i in data.argument) {
+    for(i = 0; i < data.argument.length; i++) {

same should be applied for css!

gumanist’s picture

gumanist commented
Status: Needs work » Needs review
StatusFileSize
new array-extender-breaks-ajax-responder-1401518-2.patch853 bytes

Removed redundant validation for link, updated iterator for css.

nod_’s picture

nod_
Primary language French
Location Lille
commented

Please be careful of implicit global variables, add var to variables declaration, for (var i = 0....

gumanist’s picture

gumanist commented
StatusFileSize
new array-extender-breaks-ajax-responder-1401518-4.patch861 bytes

Thanks for your suggession. You are totally right. Patch updated.

yarroha’s picture

yarroha commented

Works fine for me.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Makes sense and now looks good

merlinofchaos’s picture

merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.