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

Added DefaultHtmlEncoder property to allow specifying the HtmlEncoder… #50

Closed
wants to merge 2 commits into from

Conversation

couellet
Copy link

… to use when creating a new instance of HtmlSanitizer.

By default HtmlEncoders.Default was used, to override it you had to specify a new Formatter on each .Sanitize call.

… to use when creating a new instance of HtmlSanitizer.

By default `HtmlEncoders.Default` was used, to override it you had to specify a new Formatter on each `.Sanitize` call.
@304NotModified
Copy link
Contributor

Nice idea!

Why it's called DefaultHtmlEncoder and not HtmlEncoder?

@304NotModified
Copy link
Contributor

PS: IHtmlEncoder is from CsQuery and so not compatible with HtmlSanitizer v3.

@couellet
Copy link
Author

Good question ;) First thing that came in mind, can update it to HtmlEncoder instead. I also think it's a better name.

@304NotModified
Copy link
Contributor

Also I would do this in the constructor or the getter, what do you think @mganss ?

if (DefaultHtmlEncoder == null)
             DefaultHtmlEncoder = HtmlEncoders.Default;

@mganss
Copy link
Owner

mganss commented Nov 29, 2015

Good idea, but how about adding an OutputFormatter and a static DefaultOutputFormatter property instead?

@couellet
Copy link
Author

Yep that could be more flexible, however I am not sure about the static property. In my case sometimes I need to encode all the HTML characters and sometimes I don't. I'll make the adjustments to create a property for the OutputFormatter instead of the IHtmlEncoder.

@304NotModified
Copy link
Contributor

I think this PR should be fixed or closed.

@mganss mganss closed this in 3c02cec Aug 1, 2016
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.

3 participants