Roundcubemail: Nested conditioning skin markup behaves weirdly

Created on 23 Jun 2019  路  5Comments  路  Source: roundcube/roundcubemail

Environment

  • RC: official git repo here, master branch
  • PHP: both 5.6.40 and 7.4.0-dev are tested with the same results

Problem

Since I don't see a note in Wiki that RC's skin markup does not allow doing this common template feature, I consider this as a bug.

image => outputs A C (Good)

image => outputs D (Bad, should be B C)

image => outputs B C D (Bad, should be D)

image => outputs B C D (Bad, should be D)

Reproducer

I replace the whole skins/elastic/templates/login.html with the following codes and visit the login page to do above tests.

<roundcube:if condition="0" />
    <roundcube:if condition="0" />
        A
    <roundcube:else />
        B
    <roundcube:endif />
        C
<roundcube:else />
    D
<roundcube:endif />
Documentation Other bug

All 5 comments

This is indeed a limitation of our template engine. We should document that. Fixing this is not trivial and also not a high prio.

This is a must have feature. Maybe that could be an option, if you should tag your condition with a name.

For example:

<roundcube:if condition="config:something_enabled=='1'" name="something" />
Say something here
    <roundcube:if condition="config:anything_enabled=='1'" name="anything" />
        Say anything here
    <roundcube:endif name="anything" />
    More something
<roundcube:endif name="something" />

It is insane that now I feel this template engine is good for nothing, and I always should use objects, mixing languages and do the HTML output from PHP.

Ok guys, the best way is if you compelety change the roundcube engine. Absolutelly unusable.

Here is my solution.

https://github.com/vaso123/Twiggy/blob/master/twiggy.php

Use twig. My example of course only a skeleton, but you can improve it.

Personally I like Twig's functionality (but not it's syntax) since I have used it in several (Slim/Symfony) projects. But I don't think it would be used in official RC because it will just break every existing skin, which is a big BC break imo.

Fixed.

Was this page helpful?
0 / 5 - 0 ratings