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

Fix issue#498 Cannot read property 'firstRun' of undefined #499

Merged
merged 3 commits into from Oct 15, 2017
Merged

Fix issue#498 Cannot read property 'firstRun' of undefined #499

merged 3 commits into from Oct 15, 2017

Conversation

shaunjohansen
Copy link
Contributor

  • fix in location identified by stack trace
  • audit code and protect other instance of deeply dereferenced variables in settings
  • Release v3.5.15 (may be a little presumptuous, but we're using this version in our product)

Shaun Johansen added 3 commits June 27, 2017 14:08
- fix in location identified by stack trace
- audit code and protect other instance of deeply dereferenced variables in `settings`
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 94.1% when pulling a1496d5 on velo-industries:fix-issue-498-cannot-read-property-firstRun-of-undefined into 1237597 on davidjbradshaw:master.

@@ -201,7 +201,7 @@
}

function checkSingle(){
var remoteHost = settings[iframeId].remoteHost;
var remoteHost = settings[iframeId] && settings[iframeId].remoteHost;
Copy link

@grrrian grrrian Jun 27, 2017

Choose a reason for hiding this comment

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

Doesn't this make remoteHost a boolean? Wouldn't that break the lines below, e.g.: return origin === remoteHost

You seem to do this in many places. It's fine when the variable is a boolean, but it makes me nervous in other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, in JavaScript the && operator returns the last value or the first non-truthy value. Essentially this simply protects from accessing .remoteHost when settings[iframeId] is undefined. So, when settings[iframeId] is undefined, remoteHost is assigned undefined; and when settings[iframeId] is an object, remoteHost is assigned the value settings[iframeId].remoteHost.

@shaunjohansen
Copy link
Contributor Author

shaunjohansen commented Jun 27, 2017

Drat, the code I added decreased coverage by 0.07%!

@davidjbradshaw
Copy link
Owner

davidjbradshaw commented Jul 12, 2017 via email

@shaunjohansen
Copy link
Contributor Author

No worries at all. FYI we are deploying this updated version in our product. Just thought we'd make it available to you also.

Enjoy the vacation!

@davidjbradshaw davidjbradshaw merged commit a1496d5 into davidjbradshaw:master Oct 15, 2017
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.

4 participants