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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost 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 CreditAttribution: gumanist commented
Status: Needs work » Needs review
FileSize
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
CreditAttribution: nod_ commented

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

gumanist’s picture

gumanist CreditAttribution: gumanist commented
FileSize
array-extender-breaks-ajax-responder-1401518-4.patch861 bytes

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

yarroha’s picture

yarroha CreditAttribution: yarroha commented

Works fine for me.

andypost’s picture

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

Makes sense and now looks good

merlinofchaos’s picture

merlinofchaos CreditAttribution: 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.