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

Correct the regexp for kewordish css property which hold hex values #165

Merged
merged 3 commits into from
Apr 23, 2019
Merged

Correct the regexp for kewordish css property which hold hex values #165

merged 3 commits into from
Apr 23, 2019

Conversation

asok
Copy link
Contributor

@asok asok commented Apr 18, 2019

Loofah::HTML5::Scrub.scrub_css("background: #0000A") # => ""

Should behave the same as

Loofah::HTML5::Scrub.scrub_css("background: #0000a") # => "background:#0000a;"

@flavorjones
Copy link
Owner

THanks for submitting this PR! I'll take a look as soon as I can, this is a super-busy week for me.

@flavorjones
Copy link
Owner

@asok I noticed that this change doesn't have any test coverage. Would you mind adding a unit test to prevent regressions?

@asok
Copy link
Contributor Author

asok commented Apr 18, 2019

I've added one, I hope this is something you've hoped for.

@flavorjones
Copy link
Owner

@asok Thanks for writing up some tests. Though the ones in your commit were using assert instead of assert_equal, I was able to easily fix them and they're running through CI now.

@flavorjones flavorjones added this to the v2.3.0 milestone Apr 23, 2019
@flavorjones flavorjones merged commit 863806b into flavorjones:master Apr 23, 2019
flavorjones added a commit that referenced this pull request Apr 23, 2019
@flavorjones
Copy link
Owner

CI went green, I've merged and this will be in v2.3.0 when it ships.

@asok asok deleted the correct-css-keywordish-regexp branch April 23, 2019 08:41
@flavorjones
Copy link
Owner

v2.3.0 has been released.

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.

2 participants