-
Notifications
You must be signed in to change notification settings - Fork 9.4k
2.3 devel 14172 set timeout #14173
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
2.3 devel 14172 set timeout #14173
Conversation
|
@jonathanKingston thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @jonathanKingston. You don't have to use wrapper function. For that, you should put method "close" to setTimeout without execution. (setTimeout(self.close, 1000))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't clear where self.close is from, executing it this way prevents it being a string also. This would also make linting simpler in future if you wanted to prevent this in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I understood the problem, the solution is ok. Also, you can use "bind" function to wrapping self.case method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool will do that as I have seen this elsewhere. I'm just checking over all other uses of setTimeout now.
33614da to
6ca2f30
Compare
6ca2f30 to
0684b13
Compare
|
@VladimirZaets not included in this patch are setTimeout calls from tiny-mce and swagger libs: Questions:
Summary: Thanks for the reviews. |
|
@jonathanKingston We will make the forward port to Magento 2.3-develop, or if you want you can do it :) How do these dependencies get changed? |
|
@VladimirZaets cool I think we could do the updating of these libs in other issues as mentioned. I'm not sure why travis is failing but it seems unrelated. I added in your previous comment about bind, is there anything else you would like me to change? |
Description
Restrict usage of
setTimeoutevaluation to allow the usage of a CSP and restrict evaluation exploits.Fixed Issues
Manual testing scenarios
Contribution checklist