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.

Sign up for GitHub

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

Jump to bottom

Notification is escaped but allows anchor tags #39

Merged
merged 5 commits into from
Apr 20, 2018
Merged

Notification is escaped but allows anchor tags #39

merged 5 commits into from
Apr 20, 2018

Conversation

nateinaction
Copy link
Member

@nateinaction
Copy link
Member Author

I see the whitespace issue. Will fix.

@nateinaction
Copy link
Member Author

@ryanshoover ready for review

ryanshoover
ryanshoover reviewed Feb 27, 2018
View reviewed changes
Copy link
Contributor

@ryanshoover ryanshoover left a comment

Choose a reason for hiding this comment

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

Should we allow more tags in notices? Since a notice could theoretically be anything, make this a bit more future proof?
<em>
<strong>
<span>

Or would this unnecessarily bloat the code?

I lean toward future proofing personally

@nateinaction nateinaction force-pushed the fix/notification-escaping branch from 229ef3d to bee3d24 Compare February 27, 2018 16:18
@nateinaction
Copy link
Member Author

@ryanshoover Added support for those tags.

@nateinaction nateinaction force-pushed the fix/notification-escaping branch from bee3d24 to 5fb8ba6 Compare February 27, 2018 21:09
nateinaction added a commit to nateinaction/geoip that referenced this pull request Mar 22, 2018
@nateinaction
Adding test for wpengine#39
6ae2050
@nateinaction nateinaction force-pushed the fix/notification-escaping branch from 4094c96 to 5fb8ba6 Compare April 20, 2018 05:29
nateinaction added a commit to nateinaction/geoip that referenced this pull request Apr 20, 2018
@nateinaction
Test no longer cares about additional spacing in wpengine#39
700ae92
@stevenkword
Copy link
Contributor

This looks good to me.

@stevenkword stevenkword merged commit c14deb6 into wpengine:master Apr 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanshoover ryanshoover ryanshoover left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nateinaction @stevenkword @ryanshoover