-
Notifications
You must be signed in to change notification settings - Fork 219
Fix use of sanitizeHTML #7231
Fix use of sanitizeHTML #7231
Conversation
The release ZIP for this PR is accessible via:
|
Size Change: +60 B (0%) Total Size: 916 kB
ℹ️ View Unchanged
|
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.
Thanks @hsingyuc! This looks good to me, though two thoughts:
- Since I'm less familiar with the
woocommerce-blocks
, I'd like to get a second reviewer on this for another pair of eyes. Maybe @nielslange, since I see you worked on related code recently? - There's a lot of changes to
package-lock.json
here for what seems like a fairly small dev dependency change. I wonder if maybe yours was generated with a different version of npm than this repo normally uses?
Thank you for the feedbacks, @brianyu28!
Good idea.
I |
@brianyu28 I tried to remove |
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.
Thanks for working on this, @hsingyuc. I left one question, regarding removing dangerouslySetInnerHTML()
, and one suggestions, regarding removing the object keys, based on your comment in #7147 (comment).
assets/js/base/context/providers/store-notices/components/store-notices-container.js
Show resolved
Hide resolved
@hsingyuc I just want to let you know that @alexflorisca is currently merging #6612. To avoid inconsistencies, please do not merge this PR until @alexflorisca is done with the other PR and is giving green light. Thanks in advance! 🙌 |
You can merge now, but you may have some conflicts. If you need a hand fixing them just let me know. |
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the |
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 ran the linting jobs locally and they are both fine. E2E failures seem unrelated so I will approve and merge. Thanks @hsingyuc for working on this! 🙌🏼
@opr Thank you! |
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
* Remove object from sanitizeHTML return value * Import sanitizeHTML from utils * Fix dangerously set inner HTML format * Update package-lock * Update package-lock * Update package-lock * Update @types/dompurify version Co-authored-by: Thomas Roberts <[email protected]>
This PR fixes the
sanitizeHTML
return value data type and imports to thestore-notices-container
so we don't have two same methods in one repo.Fixes #7230
Screenshots
Testing
In src/Checkout/PaymentsHandler.php, comment or remove the if around the exception:
WooCommerce Visibility
Changelog