Skip to content
This repository was archived by the owner on May 18, 2024. It is now read-only.

Conversation

@JohnColvin
Copy link

jQuery 1.9 removed $.browser. I just copied the coffee file from the main repo and it fixes the gem for apps using 1.9.

@dnagir
Copy link
Owner

dnagir commented Feb 6, 2013

What's this fix about?
I am pretty hesitant modifying the jQuery object.

@JohnColvin
Copy link
Author

I didn't write it. I just copied it from tdreyno/iphone-style-checkboxes. jQuery.browser has been deprecated for a long time and its been removed as of 1.9. So, the author of that JS library applied this fix.

The commit for the coffeescript is here: tdreyno/iphone-style-checkboxes@3c61765

@dnagir
Copy link
Owner

dnagir commented Feb 7, 2013

I see, I see...

From the jQuery upgrade guide:

The jQuery.browser() method has been deprecated since jQuery 1.3 and is removed in 1.9. If needed, it is available as part of the jQuery Migrate plugin. We recommend using feature detection with a library such as Modernizr.

I don't know, but that commit is BAD IMO. The ios-checkboxes should either depend on the jQuery Migrate or deal with it some other way.

"Fixing deprecation" by undeprecating against the library is a bad way of dealing with it, especially when the library actually provides a migration path for that.

Maybe @tdreyno can suggest?

Not sure I want to merge it in.

@tdreyno
Copy link

tdreyno commented Feb 7, 2013

I don't think jQuery 2.0 will have a migrate plugin, right? Basically, the original plugin needs to work around some IE spacing issues which are not feature detectable. It's only in known versions of IE so we use simple user agent detection.

The code could probably do this internally without re-estabilishing the $.browser object, but I was just getting the fastest possible solution out for the folks who were reporting the breakage.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants