Wierd IE7 selector issue

[jonleighton]

Hiya,

With an older version of this library I have been getting errors in IE7 due to this selector:

form input[type=submit], form input[type=image], form button[type=submit], form button:not([type])

Specifically, the [type] part. Using the limited debugging tools available in IE it seemed that jQuery was trying to get the type attribute on the document element, which didn't exist, and hence caused an error.

I am not able to reproduce independently from my app code :(

However, I was able to solve it by changing the selector:

form input[type=submit], form input[type=image], form button[type=submit], form button:not(button[type])

I imagine this would be more performant in any case, because it has to match fewer elements.

Cheers

[neerajsingh0101]

Which version of jQuery you used?

[jonleighton]

1.5.2. I tried upgrading but it wasn't straightforward for our app so decided not to at this time.

[akzhan]

I suppose that selector should look like

body input[type=submit], body input[type=image], body button[type=submit], body button:not([type])

Or use body context of $.

Just because inputs not always wrapped by form tag.

[JangoSteve]

@jonleighton, if you'd like to use jquery 1.5.2, you should consider using an older version of jquery-ujs that supports it. 1.5.2 was buggy, so we dropped support for it in 5433841. Anything before that would probably work better.

Can anyone confirm whether or not this is still a problem with 1.6? I'd like to close the ticket if possible.

[akzhan]

Will try to check this later. Need to setup Win/ietester.

[jonleighton]

@JangoSteve to be clear, I am using an older version of jquery-ujs as well, but I noticed that the selector is still present in the current version. AFAIK there would be no harm in changing the :not([type]) to :not(button[type]), and potentially a perf gain if nothing else, which is why I suggested it. But I'm not really fussed, just thought I'd share my findings.

[JangoSteve]

@jonleighton, gotcha. Actually, there may be a performance loss by making that change, due to the way the pseudo-selectors work inside jquery (currently it only has to compare [type], but the new way it'd have to compare [type] and button). However, the performance difference would probably be negligible either way.

I just don't like making changes, especially if they're not necessary, without understanding them. Technically, button:not([type]) works according to jquery docs, and it's cleaner and more readable than button:not(button[type]).

All that being said, I was able to reproduce this in IE7 here in 1.5.2: http://jsfiddle.net/MhCDF/

And it looks like it is still a problem in 1.6.x: http://jsfiddle.net/MhCDF/1

However, changing the selector as you suggested does not appear to fix it: http://jsfiddle.net/MhCDF/2

[JangoSteve]

@akzhan, we don't care about buttons that aren't inside a form. The reason that handler exists is to cache the value of the button before the form is submitted.

[akzhan]

@JangoSteve, easy way to workaround this issue is using of jQuery contexts. Something like

// in outer closure
var $body = $("body");

// convert $(selector) to
$body.find(selector);
// or
$(selector, $body);
// or
$body.delegate(selector, ...);

[JangoSteve]

@akzhan, I'm not sure I understand how that would fix the problem. The problem is not with the context, it's with the fact that the pseudo-selector is being parsed incorrectly by jquery. Using your example, the problem still exists: http://jsfiddle.net/MhCDF/4

[JangoSteve]

@jonleighton, btw, I just realized your original post said you were getting errors. I have not been able to get it to produce any errors, I've only been able to show that the button:not([type]) selector doesn't actually match buttons without the type attribute as it should in IE7. What errors were you getting?

[jonleighton]

@JangoSteve the error seemed to be that for some reason the event was firing on the document element, and jQuery was checking the type attribute of it, but this was causing an error in IE7 because there is no type attribute on the document element.

I've just tried to reproduce it in isolation outside our app and been unsuccessful. I've already spent half a day on this problem yesterday so I am loathe to continue playing with it further unfortunately, so don't worry about the error, but I guess you might want to look into the selector-not-matching issue you found.

(I realise I am being a pretty lame bug reporter and not helping much, sorry, feel free to close this and forget.)

[JangoSteve]

@jonleighton, no prob, I know how it goes. So, that change you suggested in the beginning, that made the error go away? If so, I suppose I'll change it as long as none of the tests fail.

[jonleighton]

Yeah, it did make the error go away.