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

Update raven.js to work around IE8 shortcomings #179

Merged
merged 3 commits into from
Jan 11, 2014

Conversation

johnbacon
Copy link
Contributor

Internet Explorer 8 chokes on an undefined or null reference passed into the joinRegExp function:

SCRIPT5007: Unable to get property 'source' of undefined or null reference 
raven.js, line 1762 character 9

Line 1762, character 9 is as follows:

sources[i] = isString(patterns[i]) ?

Checking for the presence of an undefined or null value correctly handles this quirk while still seemingly performing correctly when present.

Internet Explorer 8 chokes on an undefined or null reference passed into this function. Wrapping an if statement around the processing correctly handles this while still performing correctly.
patterns[i].replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1") :
// If it's a regexp already, we want to extract the source
patterns[i].source;
if (typeof patterns[i] != "undefined" && patterns[i]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a helper for isUndefined.

@mattrobenolt
Copy link
Contributor

Looks good to me.

Are you able to add a test to assert this behavior is correct and we don't regress in the future?

Raven throws an error if empty or undefined variables are processed by joinRegExp. Some browsers (such as Internet Explorer 8 and possibly earlier) apparently pass these variables, causing errors.
@johnbacon
Copy link
Contributor Author

Take a look at that and see if it's acceptable.

@mattrobenolt
Copy link
Contributor

So if we're being specific to the problem here, the issue is that it has a variable which doesn't have a source property.

What if we did a check for: isString(sources[i]) || (!isUndefined(sources[i]) && sources[i].source)

Thinking to prevent other edge cases that may pop up. I think this function can just be reworked to handle specifically if it's a string, or if it has a .source. Ignore everything else.

Thoughts?

mattrobenolt added a commit that referenced this pull request Jan 11, 2014
Update raven.js to work around IE8 shortcomings
@mattrobenolt mattrobenolt merged commit 61d5b88 into getsentry:master Jan 11, 2014
@johnbacon
Copy link
Contributor Author

A buddy and I were working on a slightly more robust solution. Expect to see it soon!

johnbacon added a commit to johnbacon/raven-js that referenced this pull request Jan 11, 2014
…rors in joinRegExp)

This slightly more robust solution ignores options that are not strings
or regular expressions in the passed patterns array.

The prior solution still accepted non-string, non-regexp options (e.g.
[], {}, true, etc.).

Credit goes to @soundslocke for the fix.
mattrobenolt added a commit that referenced this pull request Jan 12, 2014
Improved solution to issue #179 (invalid options throwing errors in joinRegExp)
@mattrobenolt
Copy link
Contributor

@johnbacon I've published a 1.1.8, fwiw with this patch. Thanks again. :)

kamilogorek pushed a commit that referenced this pull request Jun 12, 2018
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