-
Notifications
You must be signed in to change notification settings - Fork 17
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
Security fix: Security check on email template preview page #220
Conversation
@bahiirwa it's a good idea to explain what was tested/reviewed, we shouldn't just check the "approved" box with no context. See also #214 (comment) - we should probably make sure we agree on the approach we are using to review and approve these changes. |
Testing
|
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.
I compared the changes here against woocommerce/woocommerce@60148d7 and the end result looks good.
The changes themselves are different, because there was another previous commit to this line that we haven't specifically included yet: woocommerce/woocommerce@14d9678
Since this commit also includes escaping changes in other lines I think we should include it in this PR.
Backport from woocommerce/woocommerce@60148d7 See also #214
Woo commit #14d9678
Whats the way forward on this PR? |
Awaiting approval from @nylen. |
As discussed and agreed with @nylen. An exercise in continuity planning. Reduces single-person dependency.
Backport from woocommerce/woocommerce@60148d7
See also #214
File affected:
includes/admin/class-wc-admin.php