Button Initialization Slow

[marcrohloff]

Initializing a button (or buttonset) is very slow with a large DOM. I tracked down the issue to the following lines in the _determineButtonType function:

this.buttonElement = this.element.parents().last()
   .find( "[for=" + this.element.attr("id") + "]" );

This does an attribute based search on the whole document (parents().last() which is very slow and also seems unnecessary. IMO, This can be sped up in two ways (1) Only search under the immediate parent, (2) limit the search to labels. i.e.

this.buttonElement = this.element.parent()
   .find( "label[for=" + this.element.attr("id") + "]" );

[marcrohloff]

Similarly, the radioGroup function also searches the whole DOM:

form = radio.form,
...
radios = $( form ).find( "[name='" + name + "']" );
} else {
radios = $( "[name='" + name + "']", ...

The following is much faster

container = $(radio).parent();
...
radios = container.find( "input[name='" + name + "']" );

Additioally, buttonset._create calls _init which results in _init running twice and

[AzaToth]

this wont work if you are for example in a table, and have the label in one row and the input in an other (though it might fail otherwise then as well, so it's a question what's to be allowed or not)

[bungle]

Great catch! I hope that this will be fixed in a upcoming release. This is especially bad on Internet Explorer. In my sample page, IE loads (and freezes) for about 6 seconds while Chrome does it in less than a second.

[BStramke]

For me it looks like the part described here looks like it should only be executed when the type is "checkbox" or "radio". Otherwise its using "this.buttonElement = this.element;"

(Sourcecode:)

if ( this.type === "checkbox" || this.type === "radio" ) {
   // we don't search against the document in case the element
   // is disconnected from the DOM
   this.buttonElement = this.element.parents().last()
	.find( "[for=" + this.element.attr("id") + "]" );
	this.element.addClass( "ui-helper-hidden-accessible" );

	var checked = this.element.is( ":checked" );
	if ( checked ) {
		this.buttonElement.addClass( "ui-state-active" );
	}
	this.buttonElement.attr( "aria-pressed", checked );
} else {
	this.buttonElement = this.element;
}

I found a Problem which might be very related to the one described, but at another location (line 6377), which, if you don't use a form tag, would parse the complete DOM...

_create: function() {
	this.element.closest( "form" )
		.unbind( "reset.button" )
		.bind( "reset.button", formResetHandler );

Try to add an empty <form/> Tag at the beginning of your huge DOM and report back if there was a major speedup. (this can only be a workaround)

[Scott González]

Replying to BStramke:

I found a Problem which might be very related to the one described, but at another location (line 6377), which, if you don't use a form tag, would parse the complete DOM...

_create: function() {
	this.element.closest( "form" )
		.unbind( "reset.button" )
		.bind( "reset.button", formResetHandler );

No, .closest('form') will return an empty set if you're not contained in a form. Using .closest() is extremely efficient here.

[egret230]

This just took me days to figure out.

I've got a large DOM, with lots of buttons, and initializing them froze all browsers. Now I know why!

Here's the problem code:

if(this.type==="checkbox"||this.type==="radio"){
var b=this.element.parents().filter(":last"),c="label[for="+this.element.attr("id")+"]";this.buttonElement=b.find(c);

I changed:

var b=this.parent();

If the owners of jquery ui feel that it's important to accomodate a label for a checkbox where-ever it may be in the dom, then I'd suggest doing it in a multi-step process:

1. first look for it as an immediate sibling to the check box
2. then look in a parent table (if not found as a sibling)
3. then look in a parent form (if not found in table, or no table)
4. then go to the top of the dom as a last resort

Thank you for your consideration.

[egret230]

Another option would be to add an option to the button widget.

example: labelLocation (default = null, unspecified = assumes label is a sibling element, where it "should" be)

you could then set labelLocation to:

• tr (and it looks for the label in .closest("tr")
• tbody (and it looks for the label in .closest("tbody")
• table (and it looks for the label in .closest("table")
• form (and it looks for the label in .closest("form")
• body (and it looks for the label in .closest("body")

Or, per my previous suggestion, make the code smart and have it look for it in these places until it finds it.

But as things stand now, this is a CRITICAL FLAW for large DOMs with lots of buttons - the web page will crash having to traverse to the body and then back down to style every single button...

[egret230]

By the way - I'm experiencing this bug in jQuery UI 1.8.12

[ingeborgsjon]

If the function have to search the whole DOM I think this should be an optional parameter (or vice versa). As it is today vanilla jQuery UI becomes completely useless for many of us. However if you tweak the code a bit it can be used, but this also means that you have to modify the code at any upgrade.

[mvandiest]

The usage of '.closest("form")' in my very large page is killing render times in IE. Every button instantiation walks the entire DOM to find the one form element thousands of elements above.

A default option of form: 'form' could be used to support this, but currently the use of .closest has a very specific behavior for the button and doesn't follow the selector pattern similar to the autocomplete 'appendTo' option where it is a simple selector.

Going to continue to try and find a way to supply a 'form' selector option and support the current .closest behavior by default.

[Scott González]

@mvandiest There should never be a form option. This is extremely different than autocomplete's appendTo option. You cannot optionally change the behavior of an HTML form.

[Scott González]

We can easily improve .closest( "form" ) with [0].form.

[Jörn Zaefferer]

This look like we should address is ASAP. The rewrite will likely use the same approach, so no need to wait for that.

[fontzter]

tried applying the following code to check prev and next elements first. It had negligible benefit.

ancestor = this.element.prev().add( this.element.next() );
labelSelector = "label[for='" + this.element.attr("id") + "']";

this.buttonElement = ancestor.filter( labelSelector );

if ( !this.buttonElement.length ) {
	ancestor = this.element.parents().last();
	this.buttonElement = ancestor.filter( labelSelector );
	if ( !this.buttonElement.length ) {
		ancestor = ancestor.length ? ancestor.siblings() : this.element.siblings();
		this.buttonElement = ancestor.filter( labelSelector );
		if ( !this.buttonElement.length ) {
			this.buttonElement = ancestor.find( labelSelector );
		}
	}
}

i created a fiddle to test this here: ​http://jsfiddle.net/fontzter/7jgvW/2/

[djQuery]

​http://jsperf.com/label-selector-test

After running the above test I don't see that making any changes would improve the performance significantly. $('#container').find('label').filter('[for="rb-250-2"]'); Runs fastest on most modern browsers but slower in others.