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

Swap BaseHtmlPurifier execute order #11052

Closed
alex-code opened this issue Mar 8, 2016 · 2 comments
Closed

Swap BaseHtmlPurifier execute order #11052

alex-code opened this issue Mar 8, 2016 · 2 comments
Assignees
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Milestone

Comments

@alex-code
Copy link
Contributor

I've got a $classMap extension of HtmlPurifier where I set some defaults in the configure method. e.g.

protected static function configure($config)
{
    $config->set('HTML.SafeIframe', true);
    $config->set('URI.SafeIframeRegexp', '%//(www.youtube.com/embed/|player.vimeo.com/video/)%');
}

I was then trying to add a new element using the closure config,

<?= HtmlPurifier::process($content, function(HTMLPurifier_Config $config) {
    $def = $config->getHTMLDefinition(true);
    $def->addElement('input', 'Inline', 'Empty', [], ['checked' => 'Bool#checked', 'name' => 'CDATA', 'type' => 'Enum#radio']);
}) ?>

This kept failing and the <input type="radio" /> was lost. I eventually tracked this down to $config->set('HTML.SafeIframe', true); in the configure method.
It seems HTMLPurifier was clearing a definition instance so my call to addElement was lost.

Swapping the execution order so the configure method is called first fixed my issue.

From

//....  
if ($config instanceof \Closure) {
    call_user_func($config, $configInstance);
}
static::configure($configInstance);
//....

To

//....
static::configure($configInstance);        
if ($config instanceof \Closure) {
    call_user_func($config, $configInstance);
}
//....

Can anyone else see an issue by swapping the order?

@SilverFire
Copy link
Member

Looks reasonable to me. @samdark what do you think?

@samdark
Copy link
Member

samdark commented Mar 9, 2016

Yes, looks OK to me.

@samdark samdark added the type:bug Bug label Mar 9, 2016
@samdark samdark added this to the 2.0.8 milestone Mar 9, 2016
@samdark samdark added the status:ready for adoption Feel free to implement this issue. label Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

3 participants