search

Exercise / HTMLPurifierBundle

HTML Purifier is a standards-compliant HTML filter library written in PHP.
http://htmlpurifier.org/
Other
275 stars 56 forks source link

[3.0] Fixed warming up custom configs and added support for custom config definition #52

Closed HeahDude closed 4 years ago

HeahDude commented 6 years ago

Definitely fixes https://github.com/Exercise/HTMLPurifierBundle/issues/22, while closing https://github.com/Exercise/HTMLPurifierBundle/issues/26 and https://github.com/Exercise/HTMLPurifierBundle/issues/34.

TODO:

HeahDude commented 6 years ago

ping @XWB @stof

stof commented 5 years ago

As 2.0 has already been tagged, a BC break on the configuration is not really possible anymore.

spolischook commented 5 years ago

@stof should we keep this until version 3.0?

XWB commented 5 years ago

@HeahDude Another solution would be adding the new configuration, and deprecating the old one.

stof commented 5 years ago

@XWB the hard part with that is that the existing config has prototyped node for profiles as the root node itself, making it hard to introduce anything else new.

Btw, that's one of the reason why having a prototyped node as the root is a bad idea (the other reason is that it makes it impossible to support XML config files)

HeahDude commented 5 years ago

@XWB, not sure it is worth it, we should think about a 3.0 release instead, if we really want this new config, what I would vote to. I don't think we should add complexity here because we made a tag too soon.

We did such thing in the FrameworkExtraBundle, the 4.0 has never really had the time to be required somewhere. Here some months have passed, but I don't think it's an issue actually.

jon-ht commented 5 years ago

Hi,

I'd like to use your PR but I'm facing a problem with cache warmup. It gives me this error

User Warning: Base directory (...)\var\cache\dev/htmlpurifier does not exist, please create or change using %Cache.SerializerPath

image

My config is really simple

exercise_html_purifier:
    html_profiles:
        # full configuration reference: http://htmlpurifier.org/live/configdoc/plain.html
#        default:
#            config:
#                Cache.SerializerPermissions: 777
        notice:
            config:
                HTML.Allowed: 'strong,p[class]'
            elements:
                hide:
                    - Inline
                    - Inline
                    - Common

This error is thrown by this line in ezyang/htmlpurifier

It should be a simple warning but Symfony CacheWarmer breaks and fails to build the rest of cache files. I'm not having any trouble if I use master branch

bobvandevijver commented 5 years ago

@HeahDude Is there any progress on this feature? I would really like to use this, instead of overwriting the config factory myself.

bastos71 commented 5 years ago

@stof will this PR be merged soon ? More and more we are facing problems regarding rights problems in cache directory, creating issues during deployments and during dev steps ...

stof commented 5 years ago

@bastos71 no idea. I'm not the maintainer.

bastos71 commented 5 years ago

oh sorry, then @spolischook ? or @HeahDude ?

spolischook commented 4 years ago

@HeahDude could you please resolve conflicts

HeahDude commented 4 years ago

The PR has BC breaks, we need a v3 now that v2 has been tagged. I'll try to update the PR this week end.

HeahDude commented 4 years ago

Ready for review again (ping @stof), I've fixed @jon-ht bug report. @jon-ht could you confirm please?

jon-ht commented 4 years ago

@HeahDude I'm sorry but I'm not on the project using this bundle anymore. But I guess applying my config above might give you an idea on the resolution

HeahDude commented 4 years ago

Test are green 🎉, I try to get a better tests coverage then I merge.