Fixed Inefficient code in PAGE_CONTAINER

[Kirby]

Affected version

2.2.4

<xf:if is="property('metaThemeColor')">
    <meta name="theme-color" content="{{ parse_less_color(property('metaThemeColor')) }}" />
</xf:if>

This code in template PAGE_CONTAINER seems to trigger kinda inefficient code:
\XF\Template\Templater::fnParsLessColor() does instantiate a full CSS renderer which in turn does load lessphp.

public function fnParseLessColor($templater, &$escape, $value)
{
    /** @var \XF\CssRenderer $renderer */
    $rendererClass = $this->app->extendClass('XF\CssRenderer');
    $renderer = new $rendererClass($this->app, $this);
    $renderer->setStyle($this->style);

    return $renderer->parseLessColorValue($value);
}

All in all this does load ~ 20 additional files, which seems a bit much for just converting a CSS color to #Hex format.

IMHO it would be a lot more efficient to prepare the value when saving the style and just using that prepared value at runtime.
This should not cause any side effects as colors could already be in #Hex format anyway.

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.5).

Change log:

Bail out of Less color parsing if we already have a valid-CSS color.

There may be a delay before changes are rolled out to the XenForo Community.

 

[Chris D]

To be clear; we still use the CssRenderer approach but now it is only if a non-valid colour is passed in. In other words, if a CSS compatible colour is passed in, we just use it as-is, otherwise we try and parse it through the CSS renderer.

Points noted on the alternative approach of storing the ready-parsed value. While this is potentially feasible, it requires a system to special case this particular style property, store the value for each style, maintain changes across updates etc. but given that the sheer majority of people will just have a simple value in here (rather than something that requires post-parsing like e.g. xf-intensify(#FF0000, 10%);) this should be more than sufficient.

 

[Kirby]

To be clear; we still use the CssRenderer approach but now it is only if a non-valid colour is passed in. In other words, if a CSS compatible colour is passed in, we just use it as-is, otherwise we try and parse it through the CSS renderer.

That's exactly what I did as well after digging around a bit

 

[dvsDave]

To be clear; we still use the CssRenderer approach but now it is only if a non-valid colour is passed in. In other words, if a CSS compatible colour is passed in, we just use it as-is, otherwise we try and parse it through the CSS renderer.

Points noted on the alternative approach of storing the ready-parsed value. While this is potentially feasible, it requires a system to special case this particular style property, store the value for each style, maintain changes across updates etc. but given that the sheer majority of people will just have a simple value in here (rather than something that requires post-parsing like e.g. xf-intensify(#FF0000, 10%);) this should be more than sufficient.

Is it feasible to handle this xf-intensify(#FF0000, 10%);) by having the system render the correct color when the theme is saved, that way we can avoid having to call the CssRenderer, at least at the child level of the theme?

 

[Chris D]

This is what I commented on already. It is potentially feasible but it requires a whole new system to be implemented to parse the value at save time, store it for each style, a way to access it, a way to maintain it continuously based on various factors including a situation whereby xf-intensify/xf-diminish change their output depending on whether it is a dark or a light style and so on.

It's frankly not worth it and not a very common thing to run into anyway.

It's best to just try and calculate the desired colour yourself, at least for this particular value. There are various calculators online that can help you.