Xinha messing with global arrays, thus breaking other JS in the same page

[mso@…]

I noticed, that Xinha registers 2 function, namely contains() and indexOf() to all arrays in a document, not just the ones it uses itself. This completely breaks every other Javascript routne on the page using arrays. Isn't there any way to prevent this? I consider messing with the global namesapce bad behaviour.

[anonymous]

contains() and indexOf() are pretty standard things we add to the prototype of Array() if they do not already exist.

I'd be surprised if any other script used those exact method names for something other than the purpose we use them for!

If instead you are complaining because other scripts break because we add *anything* to the prototype of Array. Well technically the other scripts are the ones in the wrong here, Javascript is *designed* for prototype manipulation, I would go pester those script authors because it won't just be Xinha that breaks them.

Leaving open for now incase some other person wants to comment.

[mharrisonline]

Could you post a sample of the code that is being broken? I use Xinha with content that contains JavaScript?, but to prevent document.writes from executing I immobilize the script while it is in Xinha. I change the word javascript to freezescript, and then back again.

[mso@…]

Quite hard to extract something, which breaks without losing context. This is one excerpt, which doesn't work with xinha:

function adb_query_cache(value) {

for (var i in search_adb_queried_words) {

if (search_adb_queried_words[i].toLowerCase().indexOf(value.toLowerCase()) != -1) {

return true;

}

} return false;

}

The variable search_adb_queried_words is usually an array, which holds AJAX search results. This code breaks when using xinha, since the for loop stumbles across your indexOf() and contains() entries, which cannot be treated as strings.

Maybe I am wrong, if you say so. But basically something meant to be included into bigger things like xinha surely is should not tinker with the global namespace, IMHO.

[mharrisonline]

When I paste:

<script>
function adb_query_cache(value) { 

for (var i in search_adb_queried_words) { 

if (search_adb_queried_words[i].toLowerCase().indexOf(value.toLowerCase()) != -1) { 

return true; 

} 

} return false; } 


</script>

into the HTML in Xinha with the GetHtml plugin, go to WYSIWYG mode, and then return to HTML view, if there was other HTML the code is not altered. If this is the only code, it is deleted.

This worked for me in both IE and Firefox.

[mokhet]

This ticket is imo invalid except for two little things we can easily fix.

Mozilla 1.8 has native implementation of indexOf and JScript 7 (IE) have it also. But Array.prototype.indexOf should returns -1 when nothing is found. http://developer.mozilla.org/en/docs/Core_JavaScript_1.5_Reference:Global_Objects:Array:indexOf http://msdn.microsoft.com/library/default.asp?url=/library/en-us/jscript7/html/jsmthindexof.asp

But with the actual indexOf implementation, the function returns null when nothing is found. Which lead to inconsistency between native indexOf and Xinha implementation.

Another problem, is the fact that native implementation of indexOf in mozilla 1.8+ and JScript 7 accepts 2 parameters argument (searchElement and fromIndex)

So basically, Xinha is creating an indexOf function which not behave the same way on every browsers. And worst of all, Xinha is creating a function that is not aware of the fromIndex argument.

I, for one, encounter the issue when including my file implementing array compatibility after including Xinha files but not if my file i including before Xinha. When I include Xinha in first place, all my scripts using indexOf are broke on some browsers (more or less outdated but still used)

To remove this inconsistency i propose the following patch (also in attached file):

Index: htmlarea.js
===================================================================
--- htmlarea.js	(revision 471)
+++ htmlarea.js	(working copy)
@@ -2532,31 +2532,36 @@
   this.updateToolbar();
 };
 
-if(!Array.prototype.contains)
+if (!Array.prototype.contains)
 {
   Array.prototype.contains = function(needle)
   {
-   var haystack = this;
-   for(var i = 0; i < haystack.length; i++)
-    {
-      if(needle == haystack[i]) return true;
-    }
-
-    return false;
+    return this.indexOf(needle) != -1;
   };
 }
 
-if(!Array.prototype.indexOf)
+if (!Array.prototype.indexOf)
 {
-  Array.prototype.indexOf = function(needle)
+  Array.prototype.indexOf = function(needle, fromIndex)
   {
-    var haystack = this;
-    for(var i = 0; i < haystack.length; i++)
+    if (!fromIndex)
     {
-      if(needle == haystack[i]) return i;
+      fromIndex = 0;
     }
+    else if (fromIndex < 0)
+    {
+      fromIndex = Math.max(0, this.length + fromIndex);
+    }
 
-    return null;
+    for (var i=fromIndex, ilen=this.length; i<ilen; i++)
+    {
+      if (this[i] === needle)
+      {
+        return i;
+      }
+    }
+
+    return -1;
   };
 }

[ray]

fixed in the distant past