Skip to content

Fix IE8 errors. Closes #45 #46

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

Merged
merged 3 commits into from
Aug 27, 2013
Merged

Fix IE8 errors. Closes #45 #46

merged 3 commits into from
Aug 27, 2013

Conversation

wnr
Copy link
Contributor

@wnr wnr commented Aug 19, 2013

No description provided.

'\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028' +
'\u2029\uFEFF';

if (!String.prototype.trim || ws.trim()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm this seems a bit extreme, I'd just do str.replace(/^\s+|\s+$/g, '') with no fallback on the native one, regexps are plenty fast

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay :) Shall I fix this or will you do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced it now with the regex given. To not get a function call overhead I didn't wrap the regex in a trim function.

@tj
Copy link
Member

tj commented Aug 27, 2013

I wouldn't worry about the function overhead, you can typically invoke a function ~5,000,000+ times a second which is plenty haha but good enough :D thanks!

tj added a commit that referenced this pull request Aug 27, 2013
Fix IE8 errors. Closes #45
@tj tj merged commit 88ef602 into reworkcss:master Aug 27, 2013
@wnr
Copy link
Contributor Author

wnr commented Aug 28, 2013

Haha okay! I guess I let some SO answer impact me too much regarding function overheads :p
Glad to help :)

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