jQuery adds mergenum attribute on matched elements in MSIE

[jf.hovinne]

Using latest revision (r1775), the following code seems to add a 'mergenum' attribute to each matched element in MSIE:

$j(this._doc.body).find("*").bind("mouseup", this.mouseup);

If I comment this line, the attribute isn't added. If I comment the whole mouseup function, the attribute is added, so I guess the problem comes from $.find().

Same problem with the following:

$j(this._doc.body).find("*").show();

Note: this._doc refers to an iframe document.

An example is available  here, when you click on the 'HTML' button to display the editor's HTML content.

The above code is located at line 848 in the file wymeditor/jquery.wymeditor.js.

[brandon]

Interesting. The offending code can be found in the jQuery.unique method. I wonder if changing the name of the expando to $mergeNum or just something else in general would fix this. We use expandos with events also ($events) but I do not see this behavior with those.

Could you try replacing your jquery.js with this one:  http://brandon.jquery.com/testing/1143/jquery.lite.js

Feel free to ping me on the results: brandon . aaron (at) gmail . com

[jf.hovinne]

Replacing the name doesn't fix the issue. On the other hand, if I comment out line 612:

first[i].mergeNum = num;

in jquery.lite.js, function unique(), jQuery works fine, and the mergenum attribute isn't added.

[vmx]

The problem is that Internet Explorer adds every custom property (like "mergeNum") as attribute to a DOM node if you use "innerHTML". The only solution I found was removing the property from the nodes after you found the uniques. Put the following two lines of code in the unique function just before "r" gets returned.

for ( var i = 0, rl = r.length; i < rl; i++ )
	r[i].removeAttribute("mergeNum");

[vmx]

I should have read the discussion about it in google groups before I post that fix. It is undesired to have an extra loop through the array as it would be a major slowdown for almost all selectors. As this is only a problem when you use innerHTML in Internet Explorer a possible solution might be to remove all properties starting with "$" within the "html" function (where innerHTML is used). But this would mean a major slowdown for innerHTML, as you would need to travers recursively through all nodes.

[vmx]

Patch for 1.1.3a

[vmx]

Example for the bugfix

[vmx]

I've thought about a way to add/remove the mergeNum property without an extra loop. This is my solution (it is only needed in onw specific merge() call). It adds a $merge=true propery to all elements that will be tested for uniqueness. If an element in unique() gets added, $merge is removed. This way unique() should work as expected. I'll attach a patch a against 1.1.3a and a small example where you can see the bug fix.

[jf.hovinne]

I've applied the patch and tested with MSIE, FF (Win + Linux), Opera.
Works fine for me!

 Test page.

[vmx]

New version that passes the test suite

[vmx]

The new version of the patch should pass the test suite. Warning: All selector will get slower (about 10%) with that patch.

The better solution to fix that bug will be removing the mergeNum property when you serialize with html(). So there will be only a slowdown on html() when it is used with Internet Explorer.

[vmx]

To replace all mergeNum properties in Internet Explorer, you can replace the html() with this one.

html: function( val ) {
	if  (val == undefined ) {
		if ( this.length ) {
			if ( jQuery.browser.msie ) {
				var nodes = this[0].getElementsByTagName("*");
				for ( var i = 0; nodes[i]; i++)
					nodes[i].removeAttribute("mergeNum");
			}
			return this[0].innerHTML;
		}
	}
	else
		return this.empty().append( val );
},

I hope it can be optimized to a smaller file size, as it currently needs additional 64 bytes (packed).

[brandon]

Patch for html

[brandon]

Thanks vmx for the heavy lifting. I've attached a patch that fixes up html in a little more jQuery-ish way.

Let me know if this works and I'll commit it.

[jf.hovinne]

I think the real problem actually resides in MSIE + designMode + expandos.
A  test page is available, showing that extra DIVs get added while e.g. using execCommand. Messing up the DOM with these extra DIVs is unacceptable, since we try to produce clean strict XHTML. We could try to handle the issue by removing the extra nodes, but I guess other unexpected problems will appear.

Removing mergeNum in html() is an improvement, but I think that the only solution for MSIE + designMode + jQuery to correctly work (if jQuery still uses first[i].mergeNum = num; in unique()) is to avoid using jQuery.find() on the iframe nodes.
Current WYMeditor trunk already uses this workaround, so we can make a release very soon.

[vmx]

Faster version

[vmx]

Brandons patch won't work, as it doesn't travers all node recursively.

I've attached a new version of my uniquebug patch. It is faster than the previous one. Internet Explorer adds custom properties as attrbutes to tags, but only if they aren't an array. This patch takes advantage of that fact and encapsulates mergeNum into an array.

[vmx]

Patching html() seems to be the way to go. Although comment:8 should be a very fast way to get it work it is neither very jQuery-ish, nor small in file size. Here are 2 propositions for a bugfix. The first one is from John, the second one from me. My version is approximately 3 times faster (tested with Firefox, Internet Explorer, Opera).

html: function( val ) { 
	return val == undefined ? 
		( this.length ? $(this[0]).find("*").removeAttr("mergeNum").end()[0].innerHTML : null ) : 
		this.empty().append( val ); 
	},

html: function( val ) { 
	return val == undefined ? 
		( this.length ? $(this[0]).find("*").each(function(){this.removeAttribute("mergeNum")}).end()[0].innerHTML : null ) : 
		this.empty().append( val ); 
	},

[john]

Two possible solutions:

A plugin that you call after you run your selectors to clean everything up:

jQuery.fn.cleanup = function(){
  return this.find("*").removeAttr("mergeNum").end();
};

or extend and overwrite jQuery.unique to force it to clean up:

var _unique = jQuery.unique;
jQuery.unique = function(a){
  var ret = jQuery.unique(a);
  jQuery(a).removeAttr("mergeNum");
  return ret;
};

Simply due to the speed hit on this, we're not planning on fixing this.

[john]

Oops, that should be:

var _unique = jQuery.unique;
jQuery.unique = function(a){
  var ret = jQuery._unique(a);
  jQuery(a).removeAttr("mergeNum");
  return ret;
};

[john]

This has been resolved in jQuery 1.2.