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

filter escapeJs prints empty js object for php objects like strings (with__toString()) #78

Closed
iinfo-dev-mk opened this issue Aug 27, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@iinfo-dev-mk
Copy link

commented Aug 27, 2015

Common problem is Nette\Http\Url, which string like behaviour is supported through whole Nette framework, but if it's set into template and ends in some javascript context, filter prints just '{}', this is serious bug.

Simple test case

 echo Latte\Runtime\Filters::escapeJs(new Nette\Http\Url("http://seznam.cz"));

prints "{}", expected is "http://seznam.cz"

tested with 2.2 branch and latest stable php 5.6

@iinfo-dev-mk iinfo-dev-mk changed the title filter escapeJs prints empty string for objects like strings (with__toString()) filter escapeJs prints empty js object for php objects like strings (with__toString()) Aug 27, 2015

@iinfo-dev-mk

This comment has been minimized.

Copy link
Author

commented Aug 27, 2015

I think this is regression and (BC break). I think it worked fine in some older version of this filter.

@dg

This comment has been minimized.

Copy link
Member

commented Aug 30, 2015

I think that escapeJs never transfered Nette\Http\Url to the string.

It can be solved by adding jsonSerialize() to Nette\Http\Url.

@iinfo-dev-mk

This comment has been minimized.

Copy link
Author

commented Aug 31, 2015

Aha, you are right, there is another tricky situation, with some filtr in template, it worked always e.g.

 <script type="text/javascript">
   dataLayer.push( { test : {$url|someFilter} });
 </script>

but without any filter it worked never

 <script type="text/javascript">
   dataLayer.push( { test : {$url} });
 </script>

I think better solution is to call __toString() in escapeJs for all arguments, which are objects. If somebody passes the object without this functionality, PHP throws error (maybe fatal?), which is correct, because no such object are accepted.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2015

I think better solution is to call __toString() in escapeJs for all arguments

Very strong 👎 That would have a lot of unexpected and unwanted side-effects.

@iinfo-dev-mk

This comment has been minimized.

Copy link
Author

commented Aug 31, 2015

For example? Purpose of escapeJS is escape strings, nothing more. But it doesn't matter, issue is closed, backporting to 2.2 will be fine, as I mentioned in first comment, we use 2.x branch and fix is in master.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2015

Purpose of escapeJS is escape strings

No, it can escape numbers, arrays, objects, booleans and null as well.

@iinfo-dev-mk

This comment has been minimized.

Copy link
Author

commented Aug 31, 2015

Hm, I think escaping and encoding are two different things, but it's true that my suggest can bring BC problems for current cases with php objects in javascript template context.

@dg

This comment has been minimized.

Copy link
Member

commented Aug 31, 2015

Unfortunately, it cannot be backported to 2.3 or 2.2, because the interface jsonSerializable exists since PHP 5.4, so it will be in next major Nette version. You can use filter as workaround.

@iinfo-dev-mk

This comment has been minimized.

Copy link
Author

commented Aug 31, 2015

OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.