[Form] Add escape_label option #4543

Dinduks · 2012-06-09T21:31:05Z

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: symfony/symfony-docs#1442

This option simply tells Twig not to escape a form's label and is useful when wanting to use a HTML tag inside a label.

->add('acceptRules', 'checkbox', array(
    'property_path' => false,
    'label'         => '<strong>I accept <br />the rules</strong>',
    'escape_label'  => false,
));

It is set to false by default, and thus doesn't break BC.

I'm aware that this option may lead to writing dirty code sometimes, but I believe the choice is up to the developer ; it can't hurt if one knows what he's doing. Besides, it'd save time when writing simple forms.

stof · 2012-06-09T21:34:19Z

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_label.html.php

@@ -1,3 +1,3 @@
 <?php if ($required) { $label_attr['class'] = trim((isset($label_attr['class']) ? $label_attr['class'] : '').' required'); } ?>
 <?php if (!$compound) { $label_attr['for'] = $id; } ?>
-<label <?php foreach ($label_attr as $k => $v) { printf('%s="%s" ', $view->escape($k), $view->escape($v)); } ?>><?php echo $view->escape($view['translator']->trans($label, array(), $translation_domain)) ?></label>
+<label <?php foreach ($label_attr as $k => $v) { printf('%s="%s" ', $view->escape($k), $view->escape($v)); } ?>><?php if ($escape_label) ?><?php echo $view->escape($view['translator']->trans($label, array(), $translation_domain)) ?><?php else ?><?php echo $view['translator']->trans($label, array(), $translation_domain) ?><?php endif ?></label>


why closing the PHP tag and opening again just after ? It seems weird

They were on different lines before I noticed that everything needs to be on a single one to not break the tests, so I just joined the lines without paying attention to that detail. I will fix that.

lyrixx · 2012-06-09T22:32:01Z

src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

@@ -225,7 +225,13 @@
    {% if required %}
        {% set label_attr = label_attr|merge({'class': (label_attr.class|default('') ~ ' required')|trim}) %}
    {% endif %}
-    <label{% for attrname, attrvalue in label_attr %} {{ attrname }}="{{ attrvalue }}"{% endfor %}>{{ label|trans({}, translation_domain) }}</label>
+    <label{% for attrname, attrvalue in label_attr %} {{ attrname }}="{{ attrvalue }}"{% endfor %}>
+        {%- if escape_label -%}


this code is in a spaceless tag. So you don't need to use {%-, {% is enougth
it's the same for other lines.

It seems that that block isn't that spaceless. It still renders spaces, and makes tests fail.

@lyrixx spaceless removes spaces between tags, not between a tag and some text

@stof , @Dinduks ok thanks.

travisbot · 2012-06-10T05:32:00Z

This pull request passes (merged 1c26835 into 6266b72).

travisbot · 2012-06-10T07:06:36Z

This pull request passes (merged 7cf0073 into 6266b72).

webmozart · 2012-06-10T07:28:43Z

@fabpot What do you think about this?

mvrhov · 2012-06-10T10:06:09Z

I've had so many cases where the label had to contain the html that I rather removed raw from the template itself.

stof · 2012-06-10T10:24:15Z

@mvrhov I don't understand what you mean. If you remove raw from the template, you have the current behavior where the label is always escaped

mvrhov · 2012-06-10T14:20:18Z

It seems that since last form refactoring unescaped values are default. Before that I had redefine the label template with added |raw. I was just to quick in answering this.

stof · 2012-06-10T15:57:31Z

labels are escaped in the core theme, unless you disabled the auto-escaping in Twig: https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig#L220

hvt · 2012-06-20T15:03:31Z

What about having this same option for the items inside choice fields? Sometimes it is convenient to allow € for example...?

Dinduks · 2012-07-07T16:24:30Z

Ping @fabpot.

willdurand · 2012-07-07T17:10:32Z

Overriding the label template is so easy, I don't think it's a good idea to start adding custom options like that one.

stof · 2012-07-07T17:17:12Z

@willdurand what you need to override for this is the rendering of the label.

henrikbjorn · 2012-07-10T14:08:41Z

👎 cant you turn the autoescape of when calling the form_widget ? or something.

stof · 2012-07-10T14:59:47Z

@henrikbjorn no. Autoescape is in the template. It cannot be changed when calling the template.

stof · 2012-10-13T17:03:33Z

@fabpot what do you think about it ?

webmozart · 2012-10-22T07:17:26Z

I agree with @willdurand that this should be solved by overriding the label template.

henrikbjorn · 2012-10-22T07:23:06Z

@stof isnt it possible to wrap the calls to the template with {% autoescape false %} ?

stof · 2012-10-22T08:06:04Z

@henrikbjorn the autoescaping is done when compiling the template. The way you use this template cannot influence it. You would have to modify the source of the template itself, not to put an autoescape tag in the caller template

Dinduks added 2 commits June 9, 2012 17:46

Add escape_label option for FormType

b1827be

Add tests for the escape_label option

1c26835

stof reviewed Jun 9, 2012
View reviewed changes

Remove useless PHP tags

7cf0073

lyrixx reviewed Jun 9, 2012
View reviewed changes

webmozart closed this Oct 22, 2012

Uh oh!

[Form] Add escape_label option #4543

[Form] Add escape_label option #4543

Uh oh!

Conversation

Dinduks commented Jun 9, 2012

Uh oh!

stof Jun 9, 2012

Choose a reason for hiding this comment

Uh oh!

Dinduks Jun 9, 2012

Choose a reason for hiding this comment

Uh oh!

lyrixx Jun 9, 2012

Choose a reason for hiding this comment

Uh oh!

Dinduks Jun 9, 2012

Choose a reason for hiding this comment

Uh oh!

stof Jun 10, 2012

Choose a reason for hiding this comment

Uh oh!

lyrixx Jun 10, 2012

Choose a reason for hiding this comment

Uh oh!

travisbot commented Jun 10, 2012

Uh oh!

travisbot commented Jun 10, 2012

Uh oh!

webmozart commented Jun 10, 2012

Uh oh!

mvrhov commented Jun 10, 2012

Uh oh!

stof commented Jun 10, 2012

Uh oh!

mvrhov commented Jun 10, 2012

Uh oh!

stof commented Jun 10, 2012

Uh oh!

hvt commented Jun 20, 2012

Uh oh!

Dinduks commented Jul 7, 2012

Uh oh!

willdurand commented Jul 7, 2012

Uh oh!

stof commented Jul 7, 2012

Uh oh!

henrikbjorn commented Jul 10, 2012

Uh oh!

stof commented Jul 10, 2012

Uh oh!

stof commented Oct 13, 2012

Uh oh!

webmozart commented Oct 22, 2012

Uh oh!

henrikbjorn commented Oct 22, 2012

Uh oh!

stof commented Oct 22, 2012

Uh oh!

Uh oh!