Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some updates for newer jQuery. (Sorry about the reformat) #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Some updates for newer jQuery. (Sorry about the reformat) #9

wants to merge 1 commit into from

Conversation

sam159
Copy link

@sam159 sam159 commented Jun 24, 2013

Added error handling to progress ajax.
Updated for jQuery 1.9+ where $.browser has been removed.

Added error handling to progress ajax.
Updated for jQuery 1.9+ where $.browser has been removed.
@clemensg
Copy link

@sam159 It's very hard to see the real differences in such a commit. Reformatting should always be done extra, if necessary.

Also, checking for Webkit or Safari alone is a bad idea, because it matches Chrome too!
Instead we should change $.browser.safari to something like: /safari/.test(navigator.userAgent.toLowerCase()) && !/chrome/.test(navigator.userAgent.toLowerCase())

And regarding error handling I'd prefer the solution from PR #10.

@drogus In case @sam159 does not respond: Should I create a new PR or could you please change the $.browser.safari line? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants