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

Unnecessary escaping in attributes #22

Closed
soukicz opened this issue Apr 15, 2015 · 13 comments
Closed

Unnecessary escaping in attributes #22

soukicz opened this issue Apr 15, 2015 · 13 comments

Comments

@soukicz
Copy link
Contributor

soukicz commented Apr 15, 2015

I am trying to migrate templates from Twig to Twital and there is problem with this code:

(autoescaping is on)

<img src="{%if true%}/a/x.jpg{%endif%}">

Expected result:

<img src="/a/x.jpg">

Actual result:

<img src="%2Fa%2Fx.jpg">

I could rewrite code but it would be much more practical if it could be compatible with Twig.

@soukicz soukicz closed this as completed Apr 15, 2015
@soukicz soukicz reopened this Apr 15, 2015
@soukicz
Copy link
Contributor Author

soukicz commented Apr 15, 2015

This could be solved be enabling FullCompatibilityTwigExtension, but that would enable less strict syntax.

@soukicz
Copy link
Contributor Author

soukicz commented Apr 15, 2015

Is is working correctly with ContextAwareEscapingSubscriber disabled

@goetas
Copy link
Owner

goetas commented Apr 17, 2015

hi! the right syntax for your code should be:

<img t:attr=" true ? src='/a/x.jpg' ">

See http://twital.readthedocs.org/en/latest/tags/attr.html

@goetas
Copy link
Owner

goetas commented Apr 17, 2015

The syntax used by you should work only with FullCompatibilityTwigExtension enabled. Does it work?

@soukicz
Copy link
Contributor Author

soukicz commented Apr 17, 2015

I realize that, but it would require rewrite large amount of template code before migration and FullCompatibilityTwigExtension enables a lot of others things.

It is also inconsistent:

<img src="{%if true%}/a/x.jpg{%endif%}">
{# output <img src="%2Fa%2Fx.jpg"> #}

<img src="{{ true ? '/a/x.jpg' }}">
{# output <img src="/a/x.jpg"> #}

@goetas
Copy link
Owner

goetas commented Apr 17, 2015

It make sense, i Will take a look

@soukicz
Copy link
Contributor Author

soukicz commented Apr 17, 2015

The will be probably somewhere in ContextAwareEscapingSubscriber (esapeUrls?) but I can't find specific place.

goetas added a commit that referenced this issue May 4, 2015
@goetas
Copy link
Owner

goetas commented May 4, 2015

I'm unable to reproduce the issue (see branch escaping test case I22Test).

Feel free to fork it and create a failing test case.

@goetas
Copy link
Owner

goetas commented Aug 8, 2015

any news in this?

@goetas
Copy link
Owner

goetas commented Aug 23, 2015

@Soukiii ping

@soukicz
Copy link
Contributor Author

soukicz commented Aug 23, 2015

I am using this test for my quick&dirty fix:

public function testAttributes() {
        $this->assertHtml('<img src="/a/x.jpg">', '<img src="{{a}}">', ['a' => '/a/x.jpg']);
        $this->assertHtml('<img src="/b/x.jpg">', '<img src="{{ true ? "/b/x.jpg"}}">');
        $this->assertHtml('<img src="/c/x.jpg">', '<img src="{%if true%}/c/x.jpg{%endif%}">');
        $this->assertHtml('<img src="/d/x.jpg">', '<img src="{%if true%}{{a}}{%else%}{{b}}{%endif%}">', ['a' => '/d/x.jpg', 'b' => '/b/x.jpg']);
    }

I will try to look at it later.

goetas added a commit that referenced this issue Aug 24, 2015
goetas added a commit that referenced this issue Aug 24, 2015
goetas added a commit that referenced this issue Aug 24, 2015
@goetas
Copy link
Owner

goetas commented Aug 24, 2015

added test https://github.com/goetas/twital/commits/master no way to reproduce

@soukicz
Copy link
Contributor Author

soukicz commented Aug 24, 2015

These test were definitely failing with master version which means that you fixed it with some other changes.

@soukicz soukicz closed this as completed Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants